Merge pull request #18414 from roidelapluie/roidelapluie/fix-proto-histogram-panics

histogram, textparse: fix two panics in compactBuckets for malformed input

model/histogram/generic.go

@@ -332,10 +332,18 @@ func compactBuckets[IBC InternalBucketCount](
 	spans = spans[:iSpan]
 	iSpan = 0
 
+	// If all spans were zero-length, no buckets remain valid.
+	if len(spans) == 0 {
+		if compensationBuckets != nil {
+			compensationBuckets = compensationBuckets[:0]
+		}
+		return primaryBuckets[:0], compensationBuckets, spans
+	}
+
 	// Cut out empty buckets from start and end of spans, no matter
 	// what. Also cut out empty buckets from the middle of a span but only
 	// if there are more than maxEmptyBuckets consecutive empty buckets.
-	for iBucket < len(primaryBuckets) {
+	for iBucket < len(primaryBuckets) && iSpan < len(spans) {
 		if deltaBuckets {
 			currentBucketAbsolute += primaryBuckets[iBucket]
 		} else {

model/histogram/histogram_test.go

@@ -1338,6 +1338,41 @@ func TestHistogramCompact(t *testing.T) {
 				CustomValues:    []float64{5, 10, 15, 20},
 			},
 		},
+		{
+			"all zero-length spans with non-empty buckets",
+			&Histogram{
+				PositiveSpans:   []Span{{0, 0}, {2, 0}},
+				PositiveBuckets: []int64{1, 3},
+				NegativeSpans:   []Span{{1, 0}},
+				NegativeBuckets: []int64{2},
+			},
+			0,
+			&Histogram{
+				PositiveSpans:   []Span{},
+				PositiveBuckets: []int64{},
+				NegativeSpans:   []Span{},
+				NegativeBuckets: []int64{},
+			},
+		},
+		{
+			// The loop guard prevents a panic; extra buckets beyond span
+			// coverage are preserved in the output. Callers must validate
+			// span/bucket consistency before invoking Compact.
+			"more buckets than spans account for",
+			&Histogram{
+				PositiveSpans:   []Span{{0, 1}, {2, 1}},
+				PositiveBuckets: []int64{1, 2, 3, 4},
+				NegativeSpans:   []Span{{0, 1}},
+				NegativeBuckets: []int64{5, 6},
+			},
+			0,
+			&Histogram{
+				PositiveSpans:   []Span{{0, 1}, {2, 1}},
+				PositiveBuckets: []int64{1, 2, 3, 4},
+				NegativeSpans:   []Span{{0, 1}},
+				NegativeBuckets: []int64{5, 6},
+			},
+		},
 		{
 			"eliminate multiple zero length spans with custom buckets",
 			&Histogram{

model/textparse/protobufparse.go

@@ -484,6 +484,9 @@ func (p *ProtobufParser) Next() (Entry, error) {
 				p.fieldPos = -3 // We have not returned anything, let p.Next() increment it to -2.
 				return p.Next()
 			}
+			if err := checkNativeHistogramConsistency(p.dec.GetHistogram()); err != nil {
+				return EntryInvalid, fmt.Errorf("histogram %q: %w", p.dec.GetName(), err)
+			}
 			p.state = EntryHistogram
 		} else {
 			p.state = EntrySeries
@@ -527,6 +530,9 @@ func (p *ProtobufParser) Next() (Entry, error) {
 			// it means we might need to do NHCB conversion.
 			if t == dto.MetricType_HISTOGRAM || t == dto.MetricType_GAUGE_HISTOGRAM {
 				if !isClassicHistogram {
+					if err := checkNativeHistogramConsistency(p.dec.GetHistogram()); err != nil {
+						return EntryInvalid, fmt.Errorf("histogram %q: %w", p.dec.GetName(), err)
+					}
 					p.state = EntryHistogram
 				} else if p.convertClassicHistogramsToNHCB {
 					// We still need to spit out the NHCB.
@@ -748,3 +754,36 @@ func (p *ProtobufParser) convertToNHCB(t dto.MetricType) (*histogram.Histogram,
 	}
 	return ch, cfh, nil
 }
+
+// checkNativeHistogramConsistency returns an error if the span bucket counts
+// do not match the number of bucket values in a native histogram protobuf
+// message. It catches malformed input before it reaches compactBuckets, where
+// a mismatch would cause a panic.
+func checkNativeHistogramConsistency(h *dto.Histogram) error {
+	isFloat := h.GetSampleCountFloat() > 0 || h.GetZeroCountFloat() > 0
+	var positiveBuckets, negativeBuckets int
+	if isFloat {
+		positiveBuckets = len(h.GetPositiveCount())
+		negativeBuckets = len(h.GetNegativeCount())
+	} else {
+		positiveBuckets = len(h.GetPositiveDelta())
+		negativeBuckets = len(h.GetNegativeDelta())
+	}
+	if err := checkProtoSpanBucketConsistency("positive", h.GetPositiveSpan(), positiveBuckets); err != nil {
+		return err
+	}
+	return checkProtoSpanBucketConsistency("negative", h.GetNegativeSpan(), negativeBuckets)
+}
+
+// checkProtoSpanBucketConsistency returns an error when the total length
+// described by spans does not match numBuckets.
+func checkProtoSpanBucketConsistency(side string, spans []dto.BucketSpan, numBuckets int) error {
+	var total int
+	for _, s := range spans {
+		total += int(s.GetLength())
+	}
+	if total != numBuckets {
+		return fmt.Errorf("%s side: spans require %d buckets, have %d", side, total, numBuckets)
+	}
+	return nil
+}

model/textparse/protobufparse_test.go

@@ -5937,6 +5937,85 @@ metric: <
 	})
 }
 
+func TestProtobufParseHistogramSpanBucketMismatch(t *testing.T) {
+	for _, tc := range []struct {
+		name  string
+		input string
+	}{
+		{
+			name: "all zero-length positive spans with non-empty positive deltas",
+			input: `name: "test_histogram"
+help: "Test."
+type: HISTOGRAM
+metric: <
+  histogram: <
+    sample_count: 2
+    sample_sum: 1.0
+    schema: 1
+    zero_threshold: 0
+    zero_count: 0
+    positive_span: <
+      offset: 0
+      length: 0
+    >
+    positive_span: <
+      offset: 2
+      length: 0
+    >
+    positive_delta: 1
+    positive_delta: 3
+  >
+>
+`,
+		},
+		{
+			name: "more positive deltas than positive spans account for",
+			input: `name: "test_histogram"
+help: "Test."
+type: HISTOGRAM
+metric: <
+  histogram: <
+    sample_count: 10
+    sample_sum: 1.0
+    schema: 1
+    zero_threshold: 0
+    zero_count: 0
+    positive_span: <
+      offset: 0
+      length: 1
+    >
+    positive_span: <
+      offset: 2
+      length: 1
+    >
+    positive_delta: 1
+    positive_delta: 2
+    positive_delta: 3
+    positive_delta: 4
+  >
+>
+`,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			buf := metricFamiliesToProtobuf(t, []string{tc.input})
+			p := NewProtobufParser(buf.Bytes(), false, false, false, false, labels.NewSymbolTable())
+			var gotErr error
+			for {
+				_, err := p.Next()
+				if errors.Is(err, io.EOF) {
+					break
+				}
+				if err != nil {
+					gotErr = err
+					break
+				}
+			}
+			require.ErrorContains(t, gotErr, "positive side")
+		})
+	}
+}
+
 func generateString(r *rand.Rand, firstRunes, restRunes []rune) string {
 	result := make([]rune, 1+r.Intn(20))
 	for i := range result {