From 957bba6f68bad79e4d710a0c0cfa9035b0b4a469 Mon Sep 17 00:00:00 2001
From: Marco Jantke <marco.jantke@gmail.com>
Date: Sat, 19 Aug 2017 22:12:52 +0200
Subject: [PATCH] add label value validation to GetMetricWith and friends

---
 prometheus/counter_test.go | 61 ++++++++++++++++++++++++++++++++++++++
 prometheus/label_values.go |  1 +
 prometheus/vec.go          | 39 +++++++++++++++++++++---
 3 files changed, 97 insertions(+), 4 deletions(-)
 create mode 100644 prometheus/label_values.go

diff --git a/prometheus/counter_test.go b/prometheus/counter_test.go
index 67391a2..fb2868a 100644
--- a/prometheus/counter_test.go
+++ b/prometheus/counter_test.go
@@ -14,6 +14,7 @@
 package prometheus
 
 import (
+	"fmt"
 	"math"
 	"testing"
 
@@ -56,3 +57,63 @@ func decreaseCounter(c *counter) (err error) {
 	c.Add(-1)
 	return nil
 }
+
+func TestCounterVecGetMetricWithInvalidLabelValues(t *testing.T) {
+	testCases := []struct {
+		desc   string
+		labels Labels
+	}{
+		{
+			desc:   "non utf8 label value",
+			labels: Labels{"a": "\xFF"},
+		},
+		{
+			desc:   "not enough label values",
+			labels: Labels{},
+		},
+		{
+			desc:   "too many label values",
+			labels: Labels{"a": "1", "b": "2"},
+		},
+	}
+
+	for _, test := range testCases {
+		test := test
+		t.Run(test.desc, func(t *testing.T) {
+			t.Parallel()
+
+			counterVec := NewCounterVec(CounterOpts{
+				Name: "test",
+			}, []string{"a"})
+
+			labelValues := make([]string, len(test.labels))
+			for _, val := range test.labels {
+				labelValues = append(labelValues, val)
+			}
+
+			expectPanic(t, func() {
+				counterVec.WithLabelValues(labelValues...)
+			}, fmt.Sprintf("WithLabelValues: expected panic because: %s", test.desc))
+			expectPanic(t, func() {
+				counterVec.With(test.labels)
+			}, fmt.Sprintf("WithLabelValues: expected panic because: %s", test.desc))
+
+			if _, err := counterVec.GetMetricWithLabelValues(labelValues...); err == nil {
+				t.Errorf("GetMetricWithLabelValues: expected error because: %s", test.desc)
+			}
+			if _, err := counterVec.GetMetricWith(test.labels); err == nil {
+				t.Errorf("GetMetricWith: expected error because: %s", test.desc)
+			}
+		})
+	}
+}
+
+func expectPanic(t *testing.T, op func(), errorMsg string) {
+	defer func() {
+		if err := recover(); err == nil {
+			t.Error(errorMsg)
+		}
+	}()
+
+	op()
+}
diff --git a/prometheus/label_values.go b/prometheus/label_values.go
new file mode 100644
index 0000000..7b1b4c0
--- /dev/null
+++ b/prometheus/label_values.go
@@ -0,0 +1 @@
+package prometheus
diff --git a/prometheus/vec.go b/prometheus/vec.go
index 40d92b7..1d1b395 100644
--- a/prometheus/vec.go
+++ b/prometheus/vec.go
@@ -14,8 +14,10 @@
 package prometheus
 
 import (
+	"errors"
 	"fmt"
 	"sync"
+	"unicode/utf8"
 
 	"github.com/prometheus/common/model"
 )
@@ -206,10 +208,24 @@ func (m *metricVec) Reset() {
 	}
 }
 
-func (m *metricVec) hashLabelValues(vals []string) (uint64, error) {
+func (m *metricVec) validateLabelValues(vals []string) error {
 	if len(vals) != len(m.desc.variableLabels) {
-		return 0, errInconsistentCardinality
+		return errInconsistentCardinality
 	}
+	for _, val := range vals {
+		if !utf8.ValidString(val) {
+			return errors.New(fmt.Sprintf("label %q is not valid utf8", val))
+		}
+	}
+
+	return nil
+}
+
+func (m *metricVec) hashLabelValues(vals []string) (uint64, error) {
+	if err := m.validateLabelValues(vals); err != nil {
+		return 0, err
+	}
+
 	h := hashNew()
 	for _, val := range vals {
 		h = m.hashAdd(h, val)
@@ -218,10 +234,25 @@ func (m *metricVec) hashLabelValues(vals []string) (uint64, error) {
 	return h, nil
 }
 
-func (m *metricVec) hashLabels(labels Labels) (uint64, error) {
+func (m *metricVec) validateLabels(labels Labels) error {
 	if len(labels) != len(m.desc.variableLabels) {
-		return 0, errInconsistentCardinality
+		return errInconsistentCardinality
 	}
+
+	for name, val := range labels {
+		if !utf8.ValidString(val) {
+			return errors.New(fmt.Sprintf("label %s: %q is not valid utf8", name, val))
+		}
+	}
+
+	return nil
+}
+
+func (m *metricVec) hashLabels(labels Labels) (uint64, error) {
+	if err := m.validateLabels(labels); err != nil {
+		return 0, err
+	}
+
 	h := hashNew()
 	for _, label := range m.desc.variableLabels {
 		val, ok := labels[label]