diff --git a/model/histogram/generic.go b/model/histogram/generic.go index 9ec9e9cd4b..287d914d64 100644 --- a/model/histogram/generic.go +++ b/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 { diff --git a/model/histogram/histogram_test.go b/model/histogram/histogram_test.go index a2b4c7c0a8..421227ff48 100644 --- a/model/histogram/histogram_test.go +++ b/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{ diff --git a/model/textparse/protobufparse.go b/model/textparse/protobufparse.go index 62f1d64431..f0537b212f 100644 --- a/model/textparse/protobufparse.go +++ b/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 +} diff --git a/model/textparse/protobufparse_test.go b/model/textparse/protobufparse_test.go index 7eff1e7387..a6c6cced50 100644 --- a/model/textparse/protobufparse_test.go +++ b/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 {