Skip to content

Commit 6b80960

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 6b80960

2 files changed

Lines changed: 213 additions & 3 deletions

File tree

‎expfmt/openmetrics_2_0_create.go‎

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

‎expfmt/openmetrics_2_0_create_test.go‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,113 @@ 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+
},
453+
{
454+
name: "DuplicateLabelSet",
455+
in: &dto.MetricFamily{
456+
Name: proto.String("test_counter_total"),
457+
Type: dto.MetricType_COUNTER.Enum(),
458+
Metric: []*dto.Metric{
459+
{
460+
Label: []*dto.LabelPair{{Name: proto.String("foo"), Value: proto.String("bar")}},
461+
Counter: &dto.Counter{Value: proto.Float64(1.0)},
462+
},
463+
{
464+
Label: []*dto.LabelPair{{Name: proto.String("foo"), Value: proto.String("bar")}},
465+
Counter: &dto.Counter{Value: proto.Float64(2.0)},
466+
},
467+
},
468+
},
469+
},
470+
{
471+
name: "ExemplarValueNaN",
472+
in: &dto.MetricFamily{
473+
Name: proto.String("test_counter_total"),
474+
Type: dto.MetricType_COUNTER.Enum(),
475+
Metric: []*dto.Metric{
476+
{
477+
Counter: &dto.Counter{
478+
Value: proto.Float64(10.0),
479+
Exemplar: &dto.Exemplar{
480+
Label: []*dto.LabelPair{{Name: proto.String("trace_id"), Value: proto.String("abc")}},
481+
Value: proto.Float64(math.NaN()),
482+
Timestamp: &timestamppb.Timestamp{Seconds: 12345},
483+
},
484+
},
485+
},
486+
},
487+
},
488+
},
382489
}
383490

384491
for _, tc := range tests {

0 commit comments

Comments
 (0)