fix(volume): reject a negative suffix length in Range headers (#12269)

* fix(volume): reject a negative suffix length in Range headers

parseRange accepted "bytes=--5" as a suffix range of -5 bytes and
returned {start: size+5, length: -5}. The volume server then answered
206 with an empty body and Content-Range "bytes 15-9/10", and the
filer's inline-content reader sliced entry.Content[15:10] and panicked.

Restore the check Go's net/http parseRange has: a suffix-length must be
a non-negative integer (RFC 7233 2.1), so a leading '-' or a negative
value is an invalid range. Apply the same check to the Rust volume
server's parse_range_header.

* test: negative-suffix manifest read is now rejected with 416

The parser change makes a negative suffix an invalid-range error, so
the duplicate-range case in this test can no longer reach the "empty
200" path. Assert the new rejection and that nothing is read.

* volume: treat a zero-length suffix range as unsatisfiable

"bytes=-0" resolved to {start: size, length: 0}, and the range handler
then answered 206 with "Content-Range: bytes 10-9/10", a value clients
must not recombine (and a first-byte-pos RFC 9110 cannot express).
Classify the zero-length suffix with the existing no-overlap case —
its resolved first-byte-pos sits at the content size — so the request
gets a 416 "bytes */size" unless another range in the set overlaps.
Applies to the Go and Rust volume servers alike.

---------

Co-authored-by: chrislusf <chrislusf@gmail.com>
Co-authored-by: user.email <chrislusf@users.noreply.github.com>

seaweed-volume/src/server/handlers.rs

@@ -2114,11 +2114,23 @@ fn parse_range_header(s: &str, size: i64) -> Result<Vec<HttpRange>, &'static str
             length: 0,
         };
         if start_str.is_empty() {
+            // suffix-length must be a non-negative integer (RFC 7233 Section 2.1)
+            if end_str.is_empty() || end_str.starts_with('-') {
+                return Err("invalid range");
+            }
             let mut i = end_str.parse::<i64>().map_err(|_| "invalid range")?;
+            if i < 0 {
+                return Err("invalid range");
+            }
             if i > size {
                 i = size;
             }
             r.start = size - i;
+            if r.start >= size {
+                // a zero-length suffix selects no bytes, so it does not overlap
+                no_overlap = true;
+                continue;
+            }
             r.length = size - r.start;
         } else {
             let i = start_str.parse::<i64>().map_err(|_| "invalid range")?;
@@ -5080,15 +5092,34 @@ mod tests {
             parse_range_header("bytes=100-", 10).unwrap_err(),
             RANGE_NO_OVERLAP
         );
+        // a zero-length suffix selects no bytes; same unsatisfiable class
+        assert_eq!(
+            parse_range_header("bytes=-0", 10).unwrap_err(),
+            RANGE_NO_OVERLAP
+        );
         // 416 only when every range fails to overlap
         let ranges = parse_range_header("bytes=10-,0-1", 10).unwrap();
         assert_eq!(ranges.len(), 1);
         assert_eq!((ranges[0].start, ranges[0].length), (0, 2));
+        let ranges = parse_range_header("bytes=-0,0-1", 10).unwrap();
+        assert_eq!(ranges.len(), 1);
+        assert_eq!((ranges[0].start, ranges[0].length), (0, 2));
         // an end past the size is clamped, still satisfiable
         let ranges = parse_range_header("bytes=5-100", 10).unwrap();
         assert_eq!((ranges[0].start, ranges[0].length), (5, 5));
     }
 
+    #[test]
+    fn test_parse_range_header_rejects_negative_suffix() {
+        // a suffix-length is a non-negative integer; "--5" must not become
+        // the range {start: 15, length: -5}
+        for spec in ["bytes=--5", "bytes=--0", "bytes=-"] {
+            assert_eq!(parse_range_header(spec, 10).unwrap_err(), "invalid range");
+        }
+        let ranges = parse_range_header("bytes=-3", 10).unwrap();
+        assert_eq!((ranges[0].start, ranges[0].length), (7, 3));
+    }
+
     #[test]
     fn test_extract_jwt_bearer() {
         let mut headers = HeaderMap::new();
@@ -5136,6 +5167,8 @@ mod tests {
         let headers = HeaderMap::new();
         let resp = handle_range_request("bytes=999-1000", data, headers, None);
         assert_eq!(resp.status(), StatusCode::RANGE_NOT_SATISFIABLE);
+        let resp = handle_range_request("bytes=-0", data, HeaderMap::new(), None);
+        assert_eq!(resp.status(), StatusCode::RANGE_NOT_SATISFIABLE);
     }
 
     #[tokio::test]
@@ -7135,10 +7168,10 @@ mod tests {
         assert_eq!(taken(&reads), vec![0]);
     }
 
-    /// A negative suffix must not shrink the covered length under the sum.
-    /// Duplicate ranges stay an empty 200 and read nothing.
+    /// A negative suffix makes the whole header invalid, so the read is
+    /// rejected before any chunk is fetched.
     #[tokio::test]
-    async fn test_chunk_manifest_negative_suffix_stays_empty() {
+    async fn test_chunk_manifest_negative_suffix_is_rejected() {
         let tmp = tempfile::TempDir::new().unwrap();
         let state = volume_test_state(&tmp);
         let ten = put_test_needle(&state, 0x6e7a_0c41, b"0123456789");
@@ -7148,16 +7181,14 @@ mod tests {
             &ten[1..]
         );
         let path = put_manifest(&state, 0x6e7a_0c42, &json);
-        let (status, headers, body) = send_read(
+        let (status, _, _) = send_read(
             &state,
             Method::GET,
             &path,
             Some(b"bytes=0-7,0-7,--1073741824"),
         )
         .await;
-        assert_eq!(status, StatusCode::OK);
-        assert!(body.is_empty());
-        assert_eq!(headers[header::ACCEPT_RANGES], "bytes");
+        assert_eq!(status, StatusCode::RANGE_NOT_SATISFIABLE);
         assert_eq!(taken(&reads), vec![0]);
     }
 

weed/server/common_test.go

@@ -325,6 +325,9 @@ func TestProcessRangeRequestRanges(t *testing.T) {
 		{"bytes=10-", http.StatusRequestedRangeNotSatisfiable, "bytes */10", ""},
 		{"bytes=100-", http.StatusRequestedRangeNotSatisfiable, "bytes */10", ""},
 		{"bytes=10-,0-1", http.StatusPartialContent, "bytes 0-1/10", "01"},
+		{"bytes=--5", http.StatusRequestedRangeNotSatisfiable, "", ""},
+		{"bytes=-0", http.StatusRequestedRangeNotSatisfiable, "bytes */10", ""},
+		{"bytes=-0,0-1", http.StatusPartialContent, "bytes 0-1/10", "01"},
 	}
 	for _, tt := range tests {
 		w, err := serve(tt.rangeHeader)

weed/server/volume_server_handlers_helper.go

@@ -55,15 +55,28 @@ func parseRange(s string, size int64) ([]httpRange, error) {
 		var r httpRange
 		if start == "" {
 			// If no start is specified, end specifies the
-			// range start relative to the end of the file.
+			// range start relative to the end of the file,
+			// and we are dealing with <suffix-length>
+			// which has to be a non-negative integer as per
+			// RFC 7233 Section 2.1 "Byte-Ranges".
+			if end == "" || end[0] == '-' {
+				return nil, errors.New("invalid range")
+			}
 			i, err := strconv.ParseInt(end, 10, 64)
-			if err != nil {
+			if i < 0 || err != nil {
 				return nil, errors.New("invalid range")
 			}
 			if i > size {
 				i = size
 			}
 			r.start = size - i
+			if r.start >= size {
+				// a zero-length suffix starts at the content size, so it
+				// cannot overlap (RFC 9110 14.1.2: a suffix-range needs a
+				// non-zero suffix-length to be satisfiable)
+				noOverlap = true
+				continue
+			}
 			r.length = size - r.start
 		} else {
 			i, err := strconv.ParseInt(start, 10, 64)

weed/server/volume_server_handlers_helper_test.go

@@ -19,7 +19,12 @@ func TestParseRange(t *testing.T) {
 		{name: "start with explicit end within bounds", rangeSpec: "bytes=2-5", wantStart: 2, wantLen: 4},
 		{name: "end clamped to the last byte", rangeSpec: "bytes=2-100", wantStart: 2, wantLen: 8},
 		{name: "suffix range within bounds", rangeSpec: "bytes=-3", wantStart: 7, wantLen: 3},
+		{name: "zero-length suffix range is not satisfiable", rangeSpec: "bytes=-0", wantErr: true},
+		{name: "zero-length suffix beside a satisfiable range is skipped", rangeSpec: "bytes=-0,0-1", wantStart: 0, wantLen: 2},
 		{name: "suffix range larger than size is clamped, not rejected", rangeSpec: "bytes=-100", wantStart: 0, wantLen: 10},
+		{name: "negative suffix length is invalid", rangeSpec: "bytes=--5", wantErr: true},
+		{name: "negative zero suffix length is invalid", rangeSpec: "bytes=--0", wantErr: true},
+		{name: "empty suffix length is invalid", rangeSpec: "bytes=-", wantErr: true},
 	}
 
 	for _, tt := range tests {
@@ -52,4 +57,10 @@ func TestParseRangeOnEmptyFile(t *testing.T) {
 	if _, err := parseRange("bytes=0-", 0); err == nil {
 		t.Fatal("parseRange(\"bytes=0-\", 0) = nil error, want an error")
 	}
+	if _, err := parseRange("bytes=-0", 0); err == nil {
+		t.Fatal("parseRange(\"bytes=-0\", 0) = nil error, want an error")
+	}
+	if _, err := parseRange("bytes=-5", 0); err == nil {
+		t.Fatal("parseRange(\"bytes=-5\", 0) = nil error, want an error")
+	}
 }