Skip to content

Commit 2c156e5

Browse files
dashpoledashpole
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 2c156e5

2 files changed

Lines changed: 275 additions & 3 deletions

File tree

‎expfmt/openmetrics_2_0_create.go‎

Lines changed: 91 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) || containsRawNewline(name) {
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,14 @@ 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 metric == nil {
165+
return written, fmt.Errorf("expected non-nil metric in MetricFamily %s", name)
166+
}
167+
if err := validateMetric20(name, metricType, metric); err != nil {
168+
return written, err
169+
}
153170
switch metricType {
154171
case dto.MetricType_COUNTER:
155-
if metric.Counter == nil {
156-
return written, fmt.Errorf("expected counter in metric %s %s", name, metric)
157-
}
158172
n, err = writeOpenMetrics20Sample(w, name, metric, metric.Counter.GetValue(), 0, false, metric.Counter.Exemplar)
159173
case dto.MetricType_GAUGE:
160174
if metric.Gauge == nil {
@@ -334,3 +348,77 @@ func writeCompositeHistogram(w enhancedWriter, name string, metric *dto.Metric,
334348
_ = isGauge
335349
return 0, errors.New("histogram not implemented yet")
336350
}
351+
352+
func validateMetric20(name string, metricType dto.MetricType, metric *dto.Metric) error {
353+
if err := validateLabels20(metric.Label); err != nil {
354+
return fmt.Errorf("invalid label in metric %s: %w", name, err)
355+
}
356+
357+
switch metricType {
358+
case dto.MetricType_COUNTER:
359+
if metric.Counter == nil {
360+
return fmt.Errorf("expected counter in metric %s %s", name, metric)
361+
}
362+
val := metric.Counter.GetValue()
363+
if math.IsNaN(val) {
364+
return fmt.Errorf("counter value cannot be NaN in metric %s", name)
365+
}
366+
if val < 0 {
367+
return fmt.Errorf("counter value cannot be negative (%g) in metric %s", val, name)
368+
}
369+
if metric.Counter.CreatedTimestamp != nil {
370+
if err := metric.Counter.CreatedTimestamp.CheckValid(); err != nil {
371+
return fmt.Errorf("invalid created timestamp in metric %s: %w", name, err)
372+
}
373+
}
374+
if ex := metric.Counter.Exemplar; ex != nil && ex.Timestamp != nil {
375+
if err := validateExemplar20(ex); err != nil {
376+
return fmt.Errorf("invalid exemplar in metric %s: %w", name, err)
377+
}
378+
}
379+
case dto.MetricType_GAUGE:
380+
if metric.Gauge == nil {
381+
return fmt.Errorf("expected gauge in metric %s %s", name, metric)
382+
}
383+
case dto.MetricType_UNTYPED:
384+
if metric.Untyped == nil {
385+
return fmt.Errorf("expected untyped in metric %s %s", name, metric)
386+
}
387+
}
388+
return nil
389+
}
390+
391+
func validateLabels20(labels []*dto.LabelPair) error {
392+
for i, lp := range labels {
393+
if lp == nil {
394+
return errors.New("expected non-nil label pair")
395+
}
396+
lname := lp.GetName()
397+
if lname == "" {
398+
return errors.New("label name cannot be empty")
399+
}
400+
if !utf8.ValidString(lname) || containsRawNewline(lname) {
401+
return fmt.Errorf("label name %q is not valid UTF-8 or contains raw newlines", lname)
402+
}
403+
if !utf8.ValidString(lp.GetValue()) {
404+
return fmt.Errorf("label value %q is not valid UTF-8", lp.GetValue())
405+
}
406+
for j := i + 1; j < len(labels); j++ {
407+
if labels[j] != nil && labels[j].GetName() == lname {
408+
return fmt.Errorf("duplicate label name %q", lname)
409+
}
410+
}
411+
}
412+
return nil
413+
}
414+
415+
func containsRawNewline(s string) bool {
416+
return strings.IndexByte(s, '\n') >= 0 || strings.IndexByte(s, '\r') >= 0
417+
}
418+
419+
func validateExemplar20(e *dto.Exemplar) error {
420+
if err := e.Timestamp.CheckValid(); err != nil {
421+
return err
422+
}
423+
return validateLabels20(e.Label)
424+
}

‎expfmt/openmetrics_2_0_create_test.go‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,30 @@ http_requests_total 1027 1234567891 st@1234567890 # {trace_id="1234"} 1 12345678
183183
},
184184
out: `# TYPE http_requests_total counter
185185
http_requests_total 1027
186+
`,
187+
},
188+
{
189+
name: "CounterWithNaNExemplar",
190+
in: &dto.MetricFamily{
191+
Name: proto.String("http_requests_total"),
192+
Type: dto.MetricType_COUNTER.Enum(),
193+
Metric: []*dto.Metric{
194+
{
195+
Counter: &dto.Counter{
196+
Value: proto.Float64(1027),
197+
Exemplar: &dto.Exemplar{
198+
Label: []*dto.LabelPair{
199+
{Name: proto.String("trace_id"), Value: proto.String("1234")},
200+
},
201+
Value: proto.Float64(math.NaN()),
202+
Timestamp: &timestamppb.Timestamp{Seconds: 1234567890},
203+
},
204+
},
205+
},
206+
},
207+
},
208+
out: `# TYPE http_requests_total counter
209+
http_requests_total 1027 # {trace_id="1234"} NaN 1234567890
186210
`,
187211
},
188212
{
@@ -379,6 +403,98 @@ func TestCreateOpenMetrics20_Errors(t *testing.T) {
379403
},
380404
},
381405
},
406+
{
407+
name: "CounterValueNaN",
408+
in: &dto.MetricFamily{
409+
Name: proto.String("test_counter_total"),
410+
Type: dto.MetricType_COUNTER.Enum(),
411+
Metric: []*dto.Metric{
412+
{Counter: &dto.Counter{Value: proto.Float64(math.NaN())}},
413+
},
414+
},
415+
},
416+
{
417+
name: "CounterValueNegative",
418+
in: &dto.MetricFamily{
419+
Name: proto.String("test_counter_total"),
420+
Type: dto.MetricType_COUNTER.Enum(),
421+
Metric: []*dto.Metric{
422+
{Counter: &dto.Counter{Value: proto.Float64(-5.0)}},
423+
},
424+
},
425+
},
426+
{
427+
name: "DuplicateLabelName",
428+
in: &dto.MetricFamily{
429+
Name: proto.String("test_counter_total"),
430+
Type: dto.MetricType_COUNTER.Enum(),
431+
Metric: []*dto.Metric{
432+
{
433+
Label: []*dto.LabelPair{
434+
{Name: proto.String("foo"), Value: proto.String("bar")},
435+
{Name: proto.String("foo"), Value: proto.String("baz")},
436+
},
437+
Counter: &dto.Counter{Value: proto.Float64(1.0)},
438+
},
439+
},
440+
},
441+
},
442+
{
443+
name: "EmptyLabelName",
444+
in: &dto.MetricFamily{
445+
Name: proto.String("test_counter_total"),
446+
Type: dto.MetricType_COUNTER.Enum(),
447+
Metric: []*dto.Metric{
448+
{
449+
Label: []*dto.LabelPair{
450+
{Name: proto.String(""), Value: proto.String("bar")},
451+
},
452+
Counter: &dto.Counter{Value: proto.Float64(1.0)},
453+
},
454+
},
455+
},
456+
},
457+
{
458+
name: "NewlineInMetricName",
459+
in: &dto.MetricFamily{
460+
Name: proto.String("test_counter\ntotal"),
461+
Type: dto.MetricType_COUNTER.Enum(),
462+
Metric: []*dto.Metric{
463+
{Counter: &dto.Counter{Value: proto.Float64(1.0)}},
464+
},
465+
},
466+
},
467+
{
468+
name: "InvalidUTF8MetricName",
469+
in: &dto.MetricFamily{
470+
Name: proto.String("test_\xff_total"),
471+
Type: dto.MetricType_COUNTER.Enum(),
472+
Metric: []*dto.Metric{
473+
{Counter: &dto.Counter{Value: proto.Float64(1.0)}},
474+
},
475+
},
476+
},
477+
{
478+
name: "NilMetric",
479+
in: &dto.MetricFamily{
480+
Name: proto.String("test_counter_total"),
481+
Type: dto.MetricType_COUNTER.Enum(),
482+
Metric: []*dto.Metric{nil},
483+
},
484+
},
485+
{
486+
name: "NilLabelPair",
487+
in: &dto.MetricFamily{
488+
Name: proto.String("test_counter_total"),
489+
Type: dto.MetricType_COUNTER.Enum(),
490+
Metric: []*dto.Metric{
491+
{
492+
Label: []*dto.LabelPair{nil},
493+
Counter: &dto.Counter{Value: proto.Float64(1.0)},
494+
},
495+
},
496+
},
497+
},
382498
}
383499

384500
for _, tc := range tests {
@@ -437,3 +553,71 @@ http_requests_total 1027
437553
t.Errorf("expected %q, got %q", expected, buf.String())
438554
}
439555
}
556+
557+
func benchmarkCounterMetricFamily() *dto.MetricFamily {
558+
return &dto.MetricFamily{
559+
Name: proto.String("http_requests_total"),
560+
Help: proto.String("Total number of HTTP requests."),
561+
Type: dto.MetricType_COUNTER.Enum(),
562+
Unit: proto.String("requests"),
563+
Metric: []*dto.Metric{
564+
{
565+
Label: []*dto.LabelPair{
566+
{Name: proto.String("method"), Value: proto.String("GET")},
567+
{Name: proto.String("handler"), Value: proto.String("/api/v1/query")},
568+
{Name: proto.String("status"), Value: proto.String("200")},
569+
},
570+
Counter: &dto.Counter{
571+
Value: proto.Float64(1027),
572+
CreatedTimestamp: &timestamppb.Timestamp{Seconds: 1234567890},
573+
Exemplar: &dto.Exemplar{
574+
Label: []*dto.LabelPair{
575+
{Name: proto.String("trace_id"), Value: proto.String("1234567890abcdef")},
576+
},
577+
Value: proto.Float64(1),
578+
Timestamp: &timestamppb.Timestamp{Seconds: 1234567890, Nanos: 500000000},
579+
},
580+
},
581+
TimestampMs: proto.Int64(1234567891000),
582+
},
583+
{
584+
Label: []*dto.LabelPair{
585+
{Name: proto.String("method"), Value: proto.String("POST")},
586+
{Name: proto.String("handler"), Value: proto.String("/api/v1/query")},
587+
{Name: proto.String("status"), Value: proto.String("500")},
588+
},
589+
Counter: &dto.Counter{
590+
Value: proto.Float64(3),
591+
CreatedTimestamp: &timestamppb.Timestamp{Seconds: 1234567890},
592+
},
593+
TimestampMs: proto.Int64(1234567891000),
594+
},
595+
},
596+
}
597+
}
598+
599+
func BenchmarkOpenMetrics20Create(b *testing.B) {
600+
mf := benchmarkCounterMetricFamily()
601+
out := bytes.NewBuffer(make([]byte, 0, 1024))
602+
b.ResetTimer()
603+
for i := 0; i < b.N; i++ {
604+
_, err := MetricFamilyToOpenMetrics20(out, mf)
605+
if err != nil {
606+
b.Fatal(err)
607+
}
608+
out.Reset()
609+
}
610+
}
611+
612+
func BenchmarkOpenMetricsCreate_Counter(b *testing.B) {
613+
mf := benchmarkCounterMetricFamily()
614+
out := bytes.NewBuffer(make([]byte, 0, 1024))
615+
b.ResetTimer()
616+
for i := 0; i < b.N; i++ {
617+
_, err := MetricFamilyToOpenMetrics(out, mf)
618+
if err != nil {
619+
b.Fatal(err)
620+
}
621+
out.Reset()
622+
}
623+
}

0 commit comments

Comments
 (0)