]> Cypherpunks.ru repositories - gostls13.git/commitdiff
net/http: ignore ranges if the content is empty in serveContent
authorJorropo <jorropo.pgm@gmail.com>
Wed, 31 Aug 2022 20:25:51 +0000 (22:25 +0200)
committerDamien Neil <dneil@google.com>
Fri, 4 Nov 2022 15:03:32 +0000 (15:03 +0000)
Fixes #54794

Change-Id: I6f2b7b86b82ea27b9d53cf989daa21cb8ace13da
Reviewed-on: https://go-review.googlesource.com/c/go/+/427195
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
src/net/http/fs.go
src/net/http/fs_test.go

index d007e763c3e294abcf866774794ed86f1f6a0824..b17542ecc9f6a6acace3ff3f0fa0c6fbe7a78034 100644 (file)
@@ -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)
index 47526152b30e842754e73932c8e2a138c0614d8e..14f26cc50f2a2244d68544ead3bbf91e8ea0d431 100644 (file)
@@ -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
 }{