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)