From edfe07834905809d687b30632ccb849b84ebd4f2 Mon Sep 17 00:00:00 2001 From: Jorropo Date: Wed, 31 Aug 2022 22:25:51 +0200 Subject: [PATCH] net/http: ignore ranges if the content is empty in serveContent Fixes #54794 Change-Id: I6f2b7b86b82ea27b9d53cf989daa21cb8ace13da Reviewed-on: https://go-review.googlesource.com/c/go/+/427195 Run-TryBot: Damien Neil TryBot-Result: Gopher Robot Reviewed-by: Damien Neil Reviewed-by: Bryan Mills --- src/net/http/fs.go | 146 ++++++++++++++++++++++------------------ src/net/http/fs_test.go | 21 ++++++ 2 files changed, 101 insertions(+), 66 deletions(-) diff --git a/src/net/http/fs.go b/src/net/http/fs.go index d007e763c3..b17542ecc9 100644 --- a/src/net/http/fs.go +++ b/src/net/http/fs.go @@ -254,81 +254,95 @@ func serveContent(w ResponseWriter, r *Request, name string, modtime time.Time, Error(w, err.Error(), StatusInternalServerError) return } + if size < 0 { + // Should never happen but just to be sure + Error(w, "negative content size computed", StatusInternalServerError) + return + } // handle Content-Range header. sendSize := size var sendContent io.Reader = content - if size >= 0 { - ranges, err := parseRange(rangeReq, size) - if err != nil { - if err == errNoOverlap { - w.Header().Set("Content-Range", fmt.Sprintf("bytes */%d", size)) - } + ranges, err := parseRange(rangeReq, size) + switch err { + case nil: + case errNoOverlap: + if size == 0 { + // Some clients add a Range header to all requests to + // limit the size of the response. If the file is empty, + // ignore the range header and respond with a 200 rather + // than a 416. + ranges = nil + break + } + w.Header().Set("Content-Range", fmt.Sprintf("bytes */%d", size)) + fallthrough + default: + Error(w, err.Error(), StatusRequestedRangeNotSatisfiable) + return + } + + if sumRangesSize(ranges) > size { + // The total number of bytes in all the ranges + // is larger than the size of the file by + // itself, so this is probably an attack, or a + // dumb client. Ignore the range request. + ranges = nil + } + switch { + case len(ranges) == 1: + // RFC 7233, Section 4.1: + // "If a single part is being transferred, the server + // generating the 206 response MUST generate a + // Content-Range header field, describing what range + // of the selected representation is enclosed, and a + // payload consisting of the range. + // ... + // A server MUST NOT generate a multipart response to + // a request for a single range, since a client that + // does not request multiple parts might not support + // multipart responses." + ra := ranges[0] + if _, err := content.Seek(ra.start, io.SeekStart); err != nil { Error(w, err.Error(), StatusRequestedRangeNotSatisfiable) return } - if sumRangesSize(ranges) > size { - // The total number of bytes in all the ranges - // is larger than the size of the file by - // itself, so this is probably an attack, or a - // dumb client. Ignore the range request. - ranges = nil - } - switch { - case len(ranges) == 1: - // RFC 7233, Section 4.1: - // "If a single part is being transferred, the server - // generating the 206 response MUST generate a - // Content-Range header field, describing what range - // of the selected representation is enclosed, and a - // payload consisting of the range. - // ... - // A server MUST NOT generate a multipart response to - // a request for a single range, since a client that - // does not request multiple parts might not support - // multipart responses." - ra := ranges[0] - if _, err := content.Seek(ra.start, io.SeekStart); err != nil { - Error(w, err.Error(), StatusRequestedRangeNotSatisfiable) - return - } - sendSize = ra.length - code = StatusPartialContent - w.Header().Set("Content-Range", ra.contentRange(size)) - case len(ranges) > 1: - sendSize = rangesMIMESize(ranges, ctype, size) - code = StatusPartialContent - - pr, pw := io.Pipe() - mw := multipart.NewWriter(pw) - w.Header().Set("Content-Type", "multipart/byteranges; boundary="+mw.Boundary()) - sendContent = pr - defer pr.Close() // cause writing goroutine to fail and exit if CopyN doesn't finish. - go func() { - for _, ra := range ranges { - part, err := mw.CreatePart(ra.mimeHeader(ctype, size)) - if err != nil { - pw.CloseWithError(err) - return - } - if _, err := content.Seek(ra.start, io.SeekStart); err != nil { - pw.CloseWithError(err) - return - } - if _, err := io.CopyN(part, content, ra.length); err != nil { - pw.CloseWithError(err) - return - } + sendSize = ra.length + code = StatusPartialContent + w.Header().Set("Content-Range", ra.contentRange(size)) + case len(ranges) > 1: + sendSize = rangesMIMESize(ranges, ctype, size) + code = StatusPartialContent + + pr, pw := io.Pipe() + mw := multipart.NewWriter(pw) + w.Header().Set("Content-Type", "multipart/byteranges; boundary="+mw.Boundary()) + sendContent = pr + defer pr.Close() // cause writing goroutine to fail and exit if CopyN doesn't finish. + go func() { + for _, ra := range ranges { + part, err := mw.CreatePart(ra.mimeHeader(ctype, size)) + if err != nil { + pw.CloseWithError(err) + return } - mw.Close() - pw.Close() - }() - } + if _, err := content.Seek(ra.start, io.SeekStart); err != nil { + pw.CloseWithError(err) + return + } + if _, err := io.CopyN(part, content, ra.length); err != nil { + pw.CloseWithError(err) + return + } + } + mw.Close() + pw.Close() + }() + } - w.Header().Set("Accept-Ranges", "bytes") - if w.Header().Get("Content-Encoding") == "" { - w.Header().Set("Content-Length", strconv.FormatInt(sendSize, 10)) - } + w.Header().Set("Accept-Ranges", "bytes") + if w.Header().Get("Content-Encoding") == "" { + w.Header().Set("Content-Length", strconv.FormatInt(sendSize, 10)) } w.WriteHeader(code) diff --git a/src/net/http/fs_test.go b/src/net/http/fs_test.go index 47526152b3..14f26cc50f 100644 --- a/src/net/http/fs_test.go +++ b/src/net/http/fs_test.go @@ -218,6 +218,27 @@ func TestServeFileDirPanicEmptyPath(t *testing.T) { } } +// Tests that ranges are ignored with serving empty content. (Issue 54794) +func TestServeContentWithEmptyContentIgnoreRanges(t *testing.T) { + for _, r := range []string{ + "bytes=0-128", + "bytes=1-", + } { + rec := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/", nil) + req.Header.Set("Range", r) + ServeContent(rec, req, "nothing", time.Now(), bytes.NewReader(nil)) + res := rec.Result() + if res.StatusCode != 200 { + t.Errorf("code = %v; want 200", res.Status) + } + bodyLen := rec.Body.Len() + if bodyLen != 0 { + t.Errorf("body.Len() = %v; want 0", res.Status) + } + } +} + var fsRedirectTestData = []struct { original, redirect string }{ -- 2.50.0