Skip to content

Commit 1a2e1e4

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 1a2e1e4

2 files changed

Lines changed: 241 additions & 3 deletions

File tree

‎expfmt/openmetrics_2_0_create.go‎

Lines changed: 108 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ import (
1919
"fmt"
2020
"io"
2121
"math"
22+
"sort"
2223
"strconv"
24+
"strings"
25+
"unicode/utf8"
2326

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

4153
// Try the interface upgrade. If it doesn't work, we'll use a
4254
// bufio.Writer from the sync.Pool.
@@ -149,12 +161,21 @@ func MetricFamilyToOpenMetrics20(out io.Writer, in *dto.MetricFamily, options ..
149161
}
150162

151163
// Finally the samples, one line for each.
164+
seenLabelSets := make(map[string]struct{}, len(in.Metric))
152165
for _, metric := range in.Metric {
166+
if metric == nil {
167+
return written, fmt.Errorf("expected non-nil metric in MetricFamily %s", name)
168+
}
169+
if err := validateMetric20(name, metricType, metric); err != nil {
170+
return written, err
171+
}
172+
key := labelSetKey(metric.Label)
173+
if _, ok := seenLabelSets[key]; ok {
174+
return written, fmt.Errorf("duplicate label set in MetricFamily %s", name)
175+
}
176+
seenLabelSets[key] = struct{}{}
153177
switch metricType {
154178
case dto.MetricType_COUNTER:
155-
if metric.Counter == nil {
156-
return written, fmt.Errorf("expected counter in metric %s %s", name, metric)
157-
}
158179
n, err = writeOpenMetrics20Sample(w, name, metric, metric.Counter.GetValue(), 0, false, metric.Counter.Exemplar)
159180
case dto.MetricType_GAUGE:
160181
if metric.Gauge == nil {
@@ -334,3 +355,87 @@ func writeCompositeHistogram(w enhancedWriter, name string, metric *dto.Metric,
334355
_ = isGauge
335356
return 0, errors.New("histogram not implemented yet")
336357
}
358+
359+
func validateMetric20(name string, metricType dto.MetricType, metric *dto.Metric) error {
360+
if err := validateLabels20(metric.Label); err != nil {
361+
return fmt.Errorf("invalid label in metric %s: %w", name, err)
362+
}
363+
364+
switch metricType {
365+
case dto.MetricType_COUNTER:
366+
if metric.Counter == nil {
367+
return fmt.Errorf("expected counter in metric %s %s", name, metric)
368+
}
369+
val := metric.Counter.GetValue()
370+
if math.IsNaN(val) {
371+
return fmt.Errorf("counter value cannot be NaN in metric %s", name)
372+
}
373+
if val < 0 {
374+
return fmt.Errorf("counter value cannot be negative (%g) in metric %s", val, name)
375+
}
376+
if metric.Counter.CreatedTimestamp != nil {
377+
if err := metric.Counter.CreatedTimestamp.CheckValid(); err != nil {
378+
return fmt.Errorf("invalid created timestamp in metric %s: %w", name, err)
379+
}
380+
}
381+
if ex := metric.Counter.Exemplar; ex != nil && ex.Timestamp != nil {
382+
if err := validateExemplar20(ex); err != nil {
383+
return fmt.Errorf("invalid exemplar in metric %s: %w", name, err)
384+
}
385+
}
386+
case dto.MetricType_GAUGE:
387+
if metric.Gauge == nil {
388+
return fmt.Errorf("expected gauge in metric %s %s", name, metric)
389+
}
390+
case dto.MetricType_UNTYPED:
391+
if metric.Untyped == nil {
392+
return fmt.Errorf("expected untyped in metric %s %s", name, metric)
393+
}
394+
}
395+
return nil
396+
}
397+
398+
func validateLabels20(labels []*dto.LabelPair) error {
399+
seen := make(map[string]struct{}, len(labels))
400+
for _, lp := range labels {
401+
if lp == nil {
402+
return errors.New("expected non-nil label pair")
403+
}
404+
lname := lp.GetName()
405+
if lname == "" {
406+
return errors.New("label name cannot be empty")
407+
}
408+
if !utf8.ValidString(lname) || strings.ContainsAny(lname, "\n\r") {
409+
return fmt.Errorf("label name %q is not valid UTF-8 or contains raw newlines", lname)
410+
}
411+
if !utf8.ValidString(lp.GetValue()) {
412+
return fmt.Errorf("label value %q is not valid UTF-8", lp.GetValue())
413+
}
414+
if _, ok := seen[lname]; ok {
415+
return fmt.Errorf("duplicate label name %q", lname)
416+
}
417+
seen[lname] = struct{}{}
418+
}
419+
return nil
420+
}
421+
422+
func validateExemplar20(e *dto.Exemplar) error {
423+
if err := e.Timestamp.CheckValid(); err != nil {
424+
return err
425+
}
426+
return validateLabels20(e.Label)
427+
}
428+
429+
func labelSetKey(labels []*dto.LabelPair) string {
430+
if len(labels) == 0 {
431+
return ""
432+
}
433+
pairs := make([]string, len(labels))
434+
for i, lp := range labels {
435+
if lp != nil {
436+
pairs[i] = lp.GetName() + "\x00" + lp.GetValue()
437+
}
438+
}
439+
sort.Strings(pairs)
440+
return strings.Join(pairs, "\x00\x00")
441+
}

‎expfmt/openmetrics_2_0_create_test.go‎

Lines changed: 133 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,115 @@ 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: "DuplicateLabelSet",
479+
in: &dto.MetricFamily{
480+
Name: proto.String("test_counter_total"),
481+
Type: dto.MetricType_COUNTER.Enum(),
482+
Metric: []*dto.Metric{
483+
{
484+
Label: []*dto.LabelPair{{Name: proto.String("foo"), Value: proto.String("bar")}},
485+
Counter: &dto.Counter{Value: proto.Float64(1.0)},
486+
},
487+
{
488+
Label: []*dto.LabelPair{{Name: proto.String("foo"), Value: proto.String("bar")}},
489+
Counter: &dto.Counter{Value: proto.Float64(2.0)},
490+
},
491+
},
492+
},
493+
},
494+
{
495+
name: "NilMetric",
496+
in: &dto.MetricFamily{
497+
Name: proto.String("test_counter_total"),
498+
Type: dto.MetricType_COUNTER.Enum(),
499+
Metric: []*dto.Metric{nil},
500+
},
501+
},
502+
{
503+
name: "NilLabelPair",
504+
in: &dto.MetricFamily{
505+
Name: proto.String("test_counter_total"),
506+
Type: dto.MetricType_COUNTER.Enum(),
507+
Metric: []*dto.Metric{
508+
{
509+
Label: []*dto.LabelPair{nil},
510+
Counter: &dto.Counter{Value: proto.Float64(1.0)},
511+
},
512+
},
513+
},
514+
},
382515
}
383516

384517
for _, tc := range tests {

0 commit comments

Comments
 (0)