diff --git a/fs/open_options.go b/fs/open_options.go index 0e761461f..448cdd02a 100644 --- a/fs/open_options.go +++ b/fs/open_options.go @@ -126,6 +126,12 @@ func (o *RangeOption) Decode(size int64) (offset, limit int64) { } else { if o.End >= 0 { offset = size - o.End + if offset < 0 { + // RFC 7233 section 2.1: if the suffix-length is + // larger than the representation, use the entire + // representation. + offset = 0 + } } else { offset = 0 } @@ -162,7 +168,14 @@ func FixRangeOption(options []OpenOption, size int64) { case *RangeOption: // If start is < 0 then fetch from the end if x.Start < 0 { - x = &RangeOption{Start: size - x.End, End: -1} + start := size - x.End + if start < 0 { + // RFC 7233 section 2.1: if the suffix-length is + // larger than the representation, use the entire + // representation (#6310). + start = 0 + } + x = &RangeOption{Start: start, End: -1} options[i] = x } // If end is too big or undefined, fetch to the end diff --git a/fs/open_options_test.go b/fs/open_options_test.go index 37463adef..a463679f7 100644 --- a/fs/open_options_test.go +++ b/fs/open_options_test.go @@ -53,6 +53,13 @@ func TestRangeOptionDecode(t *testing.T) { {in: RangeOption{Start: 1, End: -1}, size: 100, wantOffset: 1, wantLimit: -1}, {in: RangeOption{Start: -1, End: 90}, size: 100, wantOffset: 10, wantLimit: -1}, {in: RangeOption{Start: -1, End: -1}, size: 100, wantOffset: 0, wantLimit: -1}, + // Regression test for #6310: a suffix range whose length exceeds + // the size of the object must not produce a negative offset - + // RFC 7233 section 2.1 says the entire representation should be + // used instead. "bytes=-90407" against a 5 byte object panicked + // downstream with "slice bounds out of range [-90402:]". + {in: RangeOption{Start: -1, End: 90407}, size: 5, wantOffset: 0, wantLimit: -1}, + {in: RangeOption{Start: -1, End: 5}, size: 5, wantOffset: 0, wantLimit: -1}, } { gotOffset, gotLimit := test.in.Decode(test.size) what := fmt.Sprintf("%+v size=%d", test.in, test.size) @@ -227,6 +234,20 @@ func TestFixRangeOptions(t *testing.T) { }, size: 100, }, + { + // Regression test for #6310: a suffix range longer than the + // object must clamp to the start of the file (Start: 0), + // not produce a negative Start which would corrupt the + // Range header sent to the remote. + name: "Fetch the last N bytes where N is bigger than size", + in: []OpenOption{ + &RangeOption{Start: -1, End: 90407}, + }, + want: []OpenOption{ + &RangeOption{Start: 0, End: 99}, + }, + size: 100, + }, { name: "SeekOption", in: []OpenOption{ diff --git a/lib/http/serve/serve_test.go b/lib/http/serve/serve_test.go index ed716cad9..b945fa28b 100644 --- a/lib/http/serve/serve_test.go +++ b/lib/http/serve/serve_test.go @@ -70,6 +70,24 @@ func TestObjectRange(t *testing.T) { assert.Equal(t, "345", string(body)) } +func TestObjectRangeSuffixLongerThanObject(t *testing.T) { + // Regression test for #6310: a suffix range request (e.g. "bytes=-90407") + // asking for more bytes than the object contains used to compute a + // negative offset and panic with "slice bounds out of range". Per + // RFC 7233 section 2.1, the entire object should be served instead. + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "http://example.com/aFile", nil) + r.Header.Add("Range", "bytes=-90407") + o := mockobject.New("aFile").WithContent([]byte("hello"), mockobject.SeekModeNone) + Object(w, r, o) + resp := w.Result() + assert.Equal(t, http.StatusPartialContent, resp.StatusCode) + assert.Equal(t, "5", resp.Header.Get("Content-Length")) + assert.Equal(t, "bytes 0-4/5", resp.Header.Get("Content-Range")) + body, _ := io.ReadAll(resp.Body) + assert.Equal(t, "hello", string(body)) +} + func TestObjectBadRange(t *testing.T) { w := httptest.NewRecorder() r := httptest.NewRequest("GET", "http://example.com/aFile", nil)