From 7ff77a4f3b22fb79c06d7925137c391c3ce319a7 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Wed, 21 Oct 2026 20:43:32 -0700 Subject: [PATCH] s3api: skip error response body when http forbids it (#11931) WriteResponse wrote the payload even for HEAD requests and statuses that disallow a body (1xx, 204, 304), which made net/http return ErrBodyNotAllowed and logged 'write err: http: request method or response status code does not allow body' for ordinary requests. Generated with [Devin](https://devin.ai) Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- weed/s3api/s3err/error_handler.go | 18 +++++++++++++++-- weed/s3api/s3err/error_handler_test.go | 28 ++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/weed/s3api/s3err/error_handler.go b/weed/s3api/s3err/error_handler.go index 72e1bd1661..d8f6b745c6 100644 --- a/weed/s3api/s3err/error_handler.go +++ b/weed/s3api/s3err/error_handler.go @@ -137,16 +137,30 @@ func setCommonHeaders(w http.ResponseWriter, r *http.Request) { } } +// bodyAllowedForStatus is a copy of http.bodyAllowedForStatus non-exported function. +func bodyAllowedForStatus(status int) bool { + switch { + case status >= 100 && status <= 199: + return false + case status == http.StatusNoContent: + return false + case status == http.StatusNotModified: + return false + } + return true +} + func WriteResponse(w http.ResponseWriter, r *http.Request, statusCode int, response []byte, mType mimeType) { setCommonHeaders(w, r) - if response != nil { + canHaveBody := bodyAllowedForStatus(statusCode) + if response != nil && canHaveBody { w.Header().Set("Content-Length", strconv.Itoa(len(response))) } if mType != mimeNone { w.Header().Set("Content-Type", string(mType)) } w.WriteHeader(statusCode) - if response != nil { + if response != nil && canHaveBody && r.Method != http.MethodHead { glog.V(4).Infof("status %d %s: %s", statusCode, mType, string(response)) _, err := w.Write(response) if err != nil { diff --git a/weed/s3api/s3err/error_handler_test.go b/weed/s3api/s3err/error_handler_test.go index f20b8719f2..2e611dfe89 100644 --- a/weed/s3api/s3err/error_handler_test.go +++ b/weed/s3api/s3err/error_handler_test.go @@ -6,6 +6,7 @@ import ( "net/http" "net/http/httptest" "regexp" + "strconv" "testing" "time" @@ -54,6 +55,33 @@ func TestWriteErrorResponseDrainsRequestBodyBeforeWriting(t *testing.T) { assert.True(t, rr.readDeadlines[1].IsZero()) } +func TestWriteResponseOmitsBodyWhenNotAllowed(t *testing.T) { + payload := []byte("NoSuchKey") + for _, tc := range []struct { + name string + method string + status int + wantContentLength string + }{ + {"head request", http.MethodHead, http.StatusNotFound, strconv.Itoa(len(payload))}, + {"no content", http.MethodGet, http.StatusNoContent, ""}, + {"not modified", http.MethodGet, http.StatusNotModified, ""}, + } { + t.Run(tc.name, func(t *testing.T) { + req := httptest.NewRequest(tc.method, "/bucket/object", nil) + rr := httptest.NewRecorder() + WriteResponse(rr, req, tc.status, payload, MimeXML) + assert.Empty(t, rr.Body.String()) + assert.Equal(t, tc.wantContentLength, rr.Header().Get("Content-Length")) + }) + } + + req := httptest.NewRequest(http.MethodGet, "/bucket/object", nil) + rr := httptest.NewRecorder() + WriteResponse(rr, req, http.StatusOK, payload, MimeXML) + assert.Equal(t, string(payload), rr.Body.String()) +} + func extractRequestIDFromBody(body string) string { re := regexp.MustCompile(`([^<]+)`) matches := re.FindStringSubmatch(body)