diff --git a/seaweed-volume/src/server/handlers.rs b/seaweed-volume/src/server/handlers.rs index 0985ed9438..3c0c3218fe 100644 --- a/seaweed-volume/src/server/handlers.rs +++ b/seaweed-volume/src/server/handlers.rs @@ -2114,11 +2114,23 @@ fn parse_range_header(s: &str, size: i64) -> Result, &'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::().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::().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]); } diff --git a/weed/server/common_test.go b/weed/server/common_test.go index 2d60a89326..cdd4be5d21 100644 --- a/weed/server/common_test.go +++ b/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) diff --git a/weed/server/volume_server_handlers_helper.go b/weed/server/volume_server_handlers_helper.go index b161b1eca5..c8a9fad5ca 100644 --- a/weed/server/volume_server_handlers_helper.go +++ b/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 + // 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) diff --git a/weed/server/volume_server_handlers_helper_test.go b/weed/server/volume_server_handlers_helper_test.go index 29b38ac9d0..e899a461c1 100644 --- a/weed/server/volume_server_handlers_helper_test.go +++ b/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") + } }