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 <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Damien Neil <[email protected]> Reviewed-by: Bryan Mills <[email protected]>
golang · Nov 4, 2022 · edfe078 · edfe078
1 parent 7ddbadb
commit edfe078
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 66 deletions.
diff --git 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
@@ -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
 }{