From 64aeaed6b7a920657b41f387901760324ef302a0 Mon Sep 17 00:00:00 2001
From: JordanRushing <rushing.jordan@gmail.com>
Date: Mon, 7 Apr 2025 11:38:18 -0500
Subject: [PATCH] fix: prevent panic when ContentLength is negative in
 Distributor RequestBodyTooLarge metrics (#17054)

(cherry picked from commit 9e9f534460527f25d28fa3f3e17cd1eb4148b39d)
---
 pkg/distributor/http.go      |  7 +++++-
 pkg/distributor/http_test.go | 45 ++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/pkg/distributor/http.go b/pkg/distributor/http.go
index df4a9959c44d7..4d700ae7b9929 100644
--- a/pkg/distributor/http.go
+++ b/pkg/distributor/http.go
@@ -63,7 +63,12 @@ func (d *Distributor) pushHandler(w http.ResponseWriter, r *http.Request, pushRe
 			// and thus we don't know the uncompressed size.
 			// In addition we don't add the metric label values for
 			// `retention_hours` and `policy` because we don't know the labels.
-			validation.DiscardedBytes.WithLabelValues(validation.RequestBodyTooLarge, tenantID).Add(float64(r.ContentLength))
+			// Ensure ContentLength is positive to avoid counter panic
+			if r.ContentLength > 0 {
+				// Add empty values for retention_hours and policy labels since we don't have
+				// that information for request body too large errors
+				validation.DiscardedBytes.WithLabelValues(validation.RequestBodyTooLarge, tenantID, "", "").Add(float64(r.ContentLength))
+			}
 			errorWriter(w, err.Error(), http.StatusRequestEntityTooLarge, logger)
 			return
 
diff --git a/pkg/distributor/http_test.go b/pkg/distributor/http_test.go
index f263efef429fc..2ea863ef01cbb 100644
--- a/pkg/distributor/http_test.go
+++ b/pkg/distributor/http_test.go
@@ -112,6 +112,51 @@ func TestRequestParserWrapping(t *testing.T) {
 		require.True(t, called)
 		require.Equal(t, http.StatusNoContent, rec.Code)
 	})
+
+	t.Run("it handles request body too large error with positive content length", func(t *testing.T) {
+		limits := &validation.Limits{}
+		flagext.DefaultValues(limits)
+		limits.RejectOldSamples = false
+		distributors, _ := prepare(t, 1, 3, limits, nil)
+
+		ctx := user.InjectOrgID(context.Background(), "test-user")
+		req, err := http.NewRequestWithContext(ctx, http.MethodPost, "fake-path", nil)
+		require.NoError(t, err)
+
+		// Set a positive content length
+		req.ContentLength = 1000
+
+		parser := newFakeParser()
+		parser.parseErr = push.ErrRequestBodyTooLarge
+
+		rec := httptest.NewRecorder()
+		distributors[0].pushHandler(rec, req, parser.parseRequest, push.HTTPError)
+
+		require.Equal(t, http.StatusRequestEntityTooLarge, rec.Code)
+	})
+
+	t.Run("it handles request body too large error with negative content length", func(t *testing.T) {
+		limits := &validation.Limits{}
+		flagext.DefaultValues(limits)
+		limits.RejectOldSamples = false
+		distributors, _ := prepare(t, 1, 3, limits, nil)
+
+		ctx := user.InjectOrgID(context.Background(), "test-user")
+		req, err := http.NewRequestWithContext(ctx, http.MethodPost, "fake-path", nil)
+		require.NoError(t, err)
+
+		// Set a negative content length to test our guard clause
+		req.ContentLength = -1
+
+		parser := newFakeParser()
+		parser.parseErr = push.ErrRequestBodyTooLarge
+
+		rec := httptest.NewRecorder()
+		distributors[0].pushHandler(rec, req, parser.parseRequest, push.HTTPError)
+
+		require.Equal(t, http.StatusRequestEntityTooLarge, rec.Code)
+		// The test should complete without panicking
+	})
 }
 
 type fakeParser struct {