Metric name validation (#2975)

storage/remote/read.go

@@ -14,6 +14,7 @@
 package remote
 
 import (
+	"fmt"
 	"sync"
 	"time"
 
@@ -93,11 +94,33 @@ func (q *querier) read(ctx context.Context, from, through model.Time, matchers m
 	if err != nil {
 		return nil, err
 	}
-
 	removeLabels(res, added)
+	err = validateLabelsAndMetricName(res)
+	if err != nil {
+		return nil, err
+	}
+
 	return res, err
 }
 
+// validateLabelsAndMetricName validates the label names/values and metric names returned from remote read.
+func validateLabelsAndMetricName(res model.Matrix) error {
+	for _, r := range res {
+		if !model.IsValidMetricName(r.Metric[model.MetricNameLabel]) {
+			return fmt.Errorf("Invalid metric name: %v", r.Metric[model.MetricNameLabel])
+		}
+		for name, value := range r.Metric {
+			if !name.IsValid() {
+				return fmt.Errorf("Invalid label name: %v", name)
+			}
+			if !value.IsValid() {
+				return fmt.Errorf("Invalid label value: %v", value)
+			}
+		}
+	}
+	return nil
+}
+
 // addExternalLabels adds matchers for each external label. External labels
 // that already have a corresponding user-supplied matcher are skipped, as we
 // assume that the user explicitly wants to select a different value for them.

storage/remote/read_test.go

@@ -22,6 +22,115 @@ import (
 	"github.com/prometheus/prometheus/storage/metric"
 )
 
+func TestValidateLabelsAndMetricName(t *testing.T) {
+	tests := []struct {
+		result      model.Matrix
+		expectedErr string
+		shouldPass  bool
+	}{
+		{
+			result: model.Matrix{
+				&model.SampleStream{
+					Metric: model.Metric{
+						"__name__":  "name",
+						"labelName": "labelValue",
+					},
+				},
+			},
+			expectedErr: "",
+			shouldPass:  true,
+		},
+		{
+			result: model.Matrix{
+				&model.SampleStream{
+					Metric: model.Metric{
+						"__name__":   "name",
+						"_labelName": "labelValue",
+					},
+				},
+			},
+			expectedErr: "",
+			shouldPass:  true,
+		},
+		{
+			result: model.Matrix{
+				&model.SampleStream{
+					Metric: model.Metric{
+						"__name__":   "name",
+						"@labelName": "labelValue",
+					},
+				},
+			},
+			expectedErr: "Invalid label name: @labelName",
+			shouldPass:  false,
+		},
+		{
+			result: model.Matrix{
+				&model.SampleStream{
+					Metric: model.Metric{
+						"__name__":     "name",
+						"123labelName": "labelValue",
+					},
+				},
+			},
+			expectedErr: "Invalid label name: 123labelName",
+			shouldPass:  false,
+		},
+		{
+			result: model.Matrix{
+				&model.SampleStream{
+					Metric: model.Metric{
+						"__name__": "name",
+						"":         "labelValue",
+					},
+				},
+			},
+			expectedErr: "Invalid label name: ",
+			shouldPass:  false,
+		},
+		{
+			result: model.Matrix{
+				&model.SampleStream{
+					Metric: model.Metric{
+						"__name__":  "name",
+						"labelName": model.LabelValue([]byte{0xff}),
+					},
+				},
+			},
+			expectedErr: "Invalid label value: " + string([]byte{0xff}),
+			shouldPass:  false,
+		},
+		{
+			result: model.Matrix{
+				&model.SampleStream{
+					Metric: model.Metric{
+						"__name__": "@invalid_name",
+					},
+				},
+			},
+			expectedErr: "Invalid metric name: @invalid_name",
+			shouldPass:  false,
+		},
+	}
+
+	for _, test := range tests {
+		err := validateLabelsAndMetricName(test.result)
+		if test.shouldPass {
+			if err != nil {
+				t.Fatalf("Test should pass, got unexpected error: %v", err)
+			}
+			continue
+		}
+		if err != nil {
+			if err.Error() != test.expectedErr {
+				t.Fatalf("Unexpected error, got: %v, expected: %v", err, test.expectedErr)
+			}
+		} else {
+			t.Fatalf("Expected error, got none")
+		}
+	}
+}
+
 func mustNewLabelMatcher(mt metric.MatchType, name model.LabelName, val model.LabelValue) *metric.LabelMatcher {
 	m, err := metric.NewLabelMatcher(mt, name, val)
 	if err != nil {