Skip to content

Commit 69d1006

Browse files
committed
Add strict validation to OpenMetrics 2.0 encoder to reject invalid input
Signed-off-by: David Ashpole <dashpole@google.com>
1 parent 677d544 commit 69d1006

2 files changed

Lines changed: 152 additions & 3 deletions

File tree

expfmt/openmetrics_2_0_create.go

Lines changed: 81 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import (
2020
"io"
2121
"math"
2222
"strconv"
23+
"strings"
24+
"unicode/utf8"
2325

2426
dto "github.com/prometheus/client_model/go"
2527
)
@@ -37,6 +39,15 @@ func MetricFamilyToOpenMetrics20(out io.Writer, in *dto.MetricFamily, options ..
3739
if name == "" {
3840
return 0, fmt.Errorf("MetricFamily has no name: %s", in)
3941
}
42+
if !utf8.ValidString(name) || strings.ContainsAny(name, "\n\r") {
43+
return 0, fmt.Errorf("MetricFamily name %q is not valid UTF-8 or contains raw newlines", name)
44+
}
45+
if in.Help != nil && !utf8.ValidString(*in.Help) {
46+
return 0, fmt.Errorf("MetricFamily help %q is not valid UTF-8", *in.Help)
47+
}
48+
if in.Unit != nil && !utf8.ValidString(*in.Unit) {
49+
return 0, fmt.Errorf("MetricFamily unit %q is not valid UTF-8", *in.Unit)
50+
}
4051

4152
// Try the interface upgrade. If it doesn't work, we'll use a
4253
// bufio.Writer from the sync.Pool.
@@ -150,11 +161,11 @@ func MetricFamilyToOpenMetrics20(out io.Writer, in *dto.MetricFamily, options ..
150161

151162
// Finally the samples, one line for each.
152163
for _, metric := range in.Metric {
164+
if err := validateMetric20(name, metricType, metric); err != nil {
165+
return written, err
166+
}
153167
switch metricType {
154168
case dto.MetricType_COUNTER:
155-
if metric.Counter == nil {
156-
return written, fmt.Errorf("expected counter in metric %s %s", name, metric)
157-
}
158169
n, err = writeOpenMetrics20Sample(w, name, metric, metric.Counter.GetValue(), 0, false, metric.Counter.Exemplar)
159170
case dto.MetricType_GAUGE:
160171
if metric.Gauge == nil {
@@ -334,3 +345,70 @@ func writeCompositeHistogram(w enhancedWriter, name string, metric *dto.Metric,
334345
_ = isGauge
335346
return 0, errors.New("histogram not implemented yet")
336347
}
348+
349+
func validateMetric20(name string, metricType dto.MetricType, metric *dto.Metric) error {
350+
if err := validateLabels20(metric.Label); err != nil {
351+
return fmt.Errorf("invalid label in metric %s: %w", name, err)
352+
}
353+
354+
switch metricType {
355+
case dto.MetricType_COUNTER:
356+
if metric.Counter == nil {
357+
return fmt.Errorf("expected counter in metric %s %s", name, metric)
358+
}
359+
val := metric.Counter.GetValue()
360+
if math.IsNaN(val) {
361+
return fmt.Errorf("counter value cannot be NaN in metric %s", name)
362+
}
363+
if val < 0 {
364+
return fmt.Errorf("counter value cannot be negative (%g) in metric %s", val, name)
365+
}
366+
if metric.Counter.CreatedTimestamp != nil {
367+
if err := metric.Counter.CreatedTimestamp.CheckValid(); err != nil {
368+
return fmt.Errorf("invalid created timestamp in metric %s: %w", name, err)
369+
}
370+
}
371+
if ex := metric.Counter.Exemplar; ex != nil && ex.Timestamp != nil {
372+
if err := validateExemplar20(ex); err != nil {
373+
return fmt.Errorf("invalid exemplar in metric %s: %w", name, err)
374+
}
375+
}
376+
case dto.MetricType_GAUGE:
377+
if metric.Gauge == nil {
378+
return fmt.Errorf("expected gauge in metric %s %s", name, metric)
379+
}
380+
case dto.MetricType_UNTYPED:
381+
if metric.Untyped == nil {
382+
return fmt.Errorf("expected untyped in metric %s %s", name, metric)
383+
}
384+
}
385+
return nil
386+
}
387+
388+
func validateLabels20(labels []*dto.LabelPair) error {
389+
seen := make(map[string]struct{}, len(labels))
390+
for _, lp := range labels {
391+
lname := lp.GetName()
392+
if lname == "" {
393+
return errors.New("label name cannot be empty")
394+
}
395+
if !utf8.ValidString(lname) || strings.ContainsAny(lname, "\n\r") {
396+
return fmt.Errorf("label name %q is not valid UTF-8 or contains raw newlines", lname)
397+
}
398+
if !utf8.ValidString(lp.GetValue()) {
399+
return fmt.Errorf("label value %q is not valid UTF-8", lp.GetValue())
400+
}
401+
if _, ok := seen[lname]; ok {
402+
return fmt.Errorf("duplicate label name %q", lname)
403+
}
404+
seen[lname] = struct{}{}
405+
}
406+
return nil
407+
}
408+
409+
func validateExemplar20(e *dto.Exemplar) error {
410+
if err := e.Timestamp.CheckValid(); err != nil {
411+
return err
412+
}
413+
return validateLabels20(e.Label)
414+
}

expfmt/openmetrics_2_0_create_test.go

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,77 @@ func TestCreateOpenMetrics20_Errors(t *testing.T) {
379379
},
380380
},
381381
},
382+
{
383+
name: "CounterValueNaN",
384+
in: &dto.MetricFamily{
385+
Name: proto.String("test_counter_total"),
386+
Type: dto.MetricType_COUNTER.Enum(),
387+
Metric: []*dto.Metric{
388+
{Counter: &dto.Counter{Value: proto.Float64(math.NaN())}},
389+
},
390+
},
391+
},
392+
{
393+
name: "CounterValueNegative",
394+
in: &dto.MetricFamily{
395+
Name: proto.String("test_counter_total"),
396+
Type: dto.MetricType_COUNTER.Enum(),
397+
Metric: []*dto.Metric{
398+
{Counter: &dto.Counter{Value: proto.Float64(-5.0)}},
399+
},
400+
},
401+
},
402+
{
403+
name: "DuplicateLabelName",
404+
in: &dto.MetricFamily{
405+
Name: proto.String("test_counter_total"),
406+
Type: dto.MetricType_COUNTER.Enum(),
407+
Metric: []*dto.Metric{
408+
{
409+
Label: []*dto.LabelPair{
410+
{Name: proto.String("foo"), Value: proto.String("bar")},
411+
{Name: proto.String("foo"), Value: proto.String("baz")},
412+
},
413+
Counter: &dto.Counter{Value: proto.Float64(1.0)},
414+
},
415+
},
416+
},
417+
},
418+
{
419+
name: "EmptyLabelName",
420+
in: &dto.MetricFamily{
421+
Name: proto.String("test_counter_total"),
422+
Type: dto.MetricType_COUNTER.Enum(),
423+
Metric: []*dto.Metric{
424+
{
425+
Label: []*dto.LabelPair{
426+
{Name: proto.String(""), Value: proto.String("bar")},
427+
},
428+
Counter: &dto.Counter{Value: proto.Float64(1.0)},
429+
},
430+
},
431+
},
432+
},
433+
{
434+
name: "NewlineInMetricName",
435+
in: &dto.MetricFamily{
436+
Name: proto.String("test_counter\ntotal"),
437+
Type: dto.MetricType_COUNTER.Enum(),
438+
Metric: []*dto.Metric{
439+
{Counter: &dto.Counter{Value: proto.Float64(1.0)}},
440+
},
441+
},
442+
},
443+
{
444+
name: "InvalidUTF8MetricName",
445+
in: &dto.MetricFamily{
446+
Name: proto.String("test_\xff_total"),
447+
Type: dto.MetricType_COUNTER.Enum(),
448+
Metric: []*dto.Metric{
449+
{Counter: &dto.Counter{Value: proto.Float64(1.0)}},
450+
},
451+
},
452+
},
382453
}
383454

384455
for _, tc := range tests {

0 commit comments

Comments
 (0)