mirror of
https://github.com/rclone/rclone.git
synced 2026-07-09 07:15:27 -04:00
fs: fix negative offset when a suffix Range request exceeds object size
A Range header requesting a suffix longer than the object (e.g. "bytes=-90407" against a 5 byte object) caused RangeOption.Decode to compute a negative offset (size - End), which serve.Object then used directly as a slice/seek offset and panicked with "slice bounds out of range". FixRangeOption (used by backends like OneDrive/Box that lack native suffix-range support) had the same root cause: it produced a RangeOption with a negative Start, which Header() silently dropped, turning the request into the wrong byte range instead of erroring or serving the whole object. Per RFC 7233 section 2.1, when the suffix-length exceeds the representation size, the entire representation should be served. Clamp the computed offset/start to 0 in both places. Fixes #6310
This commit is contained in:
committed by
Nick Craig-Wood
parent
b12251f07f
commit
cb41e42d04
@@ -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
|
||||
|
||||
@@ -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{
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user