fs: fix FixRangeOption so it doesn't add HTTPOptions in place of bad Ranges
Before this fix, FixRangeOption would substitute RangeOptions it wanted to get rid of with with empty HTTPOption. This caused a problem now that backends interpret HTTPOptions. This fix subsitutes those with NullOptions which aren't interpreted as HTTPOptions. This patch also improves the unit tests.
This commit is contained in:
Nick Craig-Wood
parent
c4572ebc91
commit
75fc3fe389
|
|
@ -42,8 +42,8 @@ type OpenOption interface {
|
|||
//
|
||||
// RangeOption{Start: 0, End: 99} - fetch the first 100 bytes
|
||||
// RangeOption{Start: 100, End: 199} - fetch the second 100 bytes
|
||||
// RangeOption{Start: 100} - fetch bytes from offset 100 to the end
|
||||
// RangeOption{End: 100} - fetch the last 100 bytes
|
||||
// RangeOption{Start: 100, End: -1} - fetch bytes from offset 100 to the end
|
||||
// RangeOption{Start: -1, End: 100} - fetch the last 100 bytes
|
||||
//
|
||||
// A RangeOption implements a single byte-range-spec from
|
||||
// https://tools.ietf.org/html/rfc7233#section-2.1
|
||||
|
|
@ -141,10 +141,10 @@ func (o *RangeOption) Decode(size int64) (offset, limit int64) {
|
|||
func FixRangeOption(options []OpenOption, size int64) {
|
||||
if size == 0 {
|
||||
// if size 0 then remove RangeOption~s
|
||||
// replacing with an empty HTTPOption~s which won't be rendered
|
||||
// replacing with an NullOptions~s which won't be rendered
|
||||
for i := range options {
|
||||
if _, ok := options[i].(*RangeOption); ok {
|
||||
options[i] = &HTTPOption{}
|
||||
options[i] = NullOption{}
|
||||
|
||||
}
|
||||
}
|
||||
|
|
@ -230,6 +230,25 @@ func (o *HashesOption) Mandatory() bool {
|
|||
return false
|
||||
}
|
||||
|
||||
// NullOption defines an Option which does nothing
|
||||
type NullOption struct {
|
||||
}
|
||||
|
||||
// Header formats the option as an http header
|
||||
func (o NullOption) Header() (key string, value string) {
|
||||
return "", ""
|
||||
}
|
||||
|
||||
// String formats the option into human readable form
|
||||
func (o NullOption) String() string {
|
||||
return fmt.Sprintf("NullOption()")
|
||||
}
|
||||
|
||||
// Mandatory returns whether the option must be parsed or can be ignored
|
||||
func (o NullOption) Mandatory() bool {
|
||||
return false
|
||||
}
|
||||
|
||||
// OpenOptionAddHeaders adds each header found in options to the
|
||||
// headers map provided the key was non empty.
|
||||
func OpenOptionAddHeaders(options []OpenOption, headers map[string]string) {
|
||||
|
|
@ -264,10 +283,3 @@ func OpenOptionAddHTTPHeaders(headers http.Header, options []OpenOption) {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
// check interface
|
||||
var (
|
||||
_ OpenOption = (*RangeOption)(nil)
|
||||
_ OpenOption = (*SeekOption)(nil)
|
||||
_ OpenOption = (*HTTPOption)(nil)
|
||||
)
|
||||
|
|
|
|||
|
|
@ -2,8 +2,10 @@ package fs
|
|||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"testing"
|
||||
|
||||
"github.com/rclone/rclone/fs/hash"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
|
@ -58,3 +60,197 @@ func TestRangeOptionDecode(t *testing.T) {
|
|||
assert.Equal(t, test.wantLimit, gotLimit, "limit "+what)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRangeOption(t *testing.T) {
|
||||
opt := &RangeOption{Start: 1, End: 10}
|
||||
var _ OpenOption = opt // check interface
|
||||
assert.Equal(t, "RangeOption(1,10)", opt.String())
|
||||
key, value := opt.Header()
|
||||
assert.Equal(t, "Range", key)
|
||||
assert.Equal(t, "bytes=1-10", value)
|
||||
assert.Equal(t, true, opt.Mandatory())
|
||||
|
||||
opt = &RangeOption{Start: -1, End: 10}
|
||||
assert.Equal(t, "RangeOption(-1,10)", opt.String())
|
||||
key, value = opt.Header()
|
||||
assert.Equal(t, "Range", key)
|
||||
assert.Equal(t, "bytes=-10", value)
|
||||
assert.Equal(t, true, opt.Mandatory())
|
||||
|
||||
opt = &RangeOption{Start: 1, End: -1}
|
||||
assert.Equal(t, "RangeOption(1,-1)", opt.String())
|
||||
key, value = opt.Header()
|
||||
assert.Equal(t, "Range", key)
|
||||
assert.Equal(t, "bytes=1-", value)
|
||||
assert.Equal(t, true, opt.Mandatory())
|
||||
|
||||
opt = &RangeOption{Start: -1, End: -1}
|
||||
assert.Equal(t, "RangeOption(-1,-1)", opt.String())
|
||||
key, value = opt.Header()
|
||||
assert.Equal(t, "Range", key)
|
||||
assert.Equal(t, "bytes=-", value)
|
||||
assert.Equal(t, true, opt.Mandatory())
|
||||
}
|
||||
|
||||
func TestSeekOption(t *testing.T) {
|
||||
opt := &SeekOption{Offset: 1}
|
||||
var _ OpenOption = opt // check interface
|
||||
assert.Equal(t, "SeekOption(1)", opt.String())
|
||||
key, value := opt.Header()
|
||||
assert.Equal(t, "Range", key)
|
||||
assert.Equal(t, "bytes=1-", value)
|
||||
assert.Equal(t, true, opt.Mandatory())
|
||||
}
|
||||
|
||||
func TestHTTPOption(t *testing.T) {
|
||||
opt := &HTTPOption{Key: "k", Value: "v"}
|
||||
var _ OpenOption = opt // check interface
|
||||
assert.Equal(t, `HTTPOption("k","v")`, opt.String())
|
||||
key, value := opt.Header()
|
||||
assert.Equal(t, "k", key)
|
||||
assert.Equal(t, "v", value)
|
||||
assert.Equal(t, false, opt.Mandatory())
|
||||
}
|
||||
|
||||
func TestHashesOption(t *testing.T) {
|
||||
opt := &HashesOption{hash.Set(hash.MD5 | hash.SHA1)}
|
||||
var _ OpenOption = opt // check interface
|
||||
assert.Equal(t, `HashesOption([MD5, SHA-1])`, opt.String())
|
||||
key, value := opt.Header()
|
||||
assert.Equal(t, "", key)
|
||||
assert.Equal(t, "", value)
|
||||
assert.Equal(t, false, opt.Mandatory())
|
||||
}
|
||||
|
||||
func TestNullOption(t *testing.T) {
|
||||
opt := NullOption{}
|
||||
var _ OpenOption = opt // check interface
|
||||
assert.Equal(t, "NullOption()", opt.String())
|
||||
key, value := opt.Header()
|
||||
assert.Equal(t, "", key)
|
||||
assert.Equal(t, "", value)
|
||||
assert.Equal(t, false, opt.Mandatory())
|
||||
}
|
||||
|
||||
func TestFixRangeOptions(t *testing.T) {
|
||||
for _, test := range []struct {
|
||||
name string
|
||||
in []OpenOption
|
||||
size int64
|
||||
want []OpenOption
|
||||
}{
|
||||
{
|
||||
name: "Nil options",
|
||||
in: nil,
|
||||
want: nil,
|
||||
},
|
||||
{
|
||||
name: "Empty options",
|
||||
in: []OpenOption{},
|
||||
want: []OpenOption{},
|
||||
},
|
||||
{
|
||||
name: "Fetch a range with size=0",
|
||||
in: []OpenOption{
|
||||
&HTTPOption{Key: "a", Value: "1"},
|
||||
&RangeOption{Start: 1, End: 10},
|
||||
&HTTPOption{Key: "b", Value: "2"},
|
||||
},
|
||||
want: []OpenOption{
|
||||
&HTTPOption{Key: "a", Value: "1"},
|
||||
NullOption{},
|
||||
&HTTPOption{Key: "b", Value: "2"},
|
||||
},
|
||||
size: 0,
|
||||
},
|
||||
{
|
||||
name: "Fetch a range",
|
||||
in: []OpenOption{
|
||||
&HTTPOption{Key: "a", Value: "1"},
|
||||
&RangeOption{Start: 1, End: 10},
|
||||
&HTTPOption{Key: "b", Value: "2"},
|
||||
},
|
||||
want: []OpenOption{
|
||||
&HTTPOption{Key: "a", Value: "1"},
|
||||
&RangeOption{Start: 1, End: 10},
|
||||
&HTTPOption{Key: "b", Value: "2"},
|
||||
},
|
||||
size: 100,
|
||||
},
|
||||
{
|
||||
name: "Fetch to end",
|
||||
in: []OpenOption{
|
||||
&RangeOption{Start: 1, End: -1},
|
||||
},
|
||||
want: []OpenOption{
|
||||
&RangeOption{Start: 1, End: -1},
|
||||
},
|
||||
size: 100,
|
||||
},
|
||||
{
|
||||
name: "Fetch the last 10 bytes",
|
||||
in: []OpenOption{
|
||||
&RangeOption{Start: -1, End: 10},
|
||||
},
|
||||
want: []OpenOption{
|
||||
&RangeOption{Start: 90, End: -1},
|
||||
},
|
||||
size: 100,
|
||||
},
|
||||
{
|
||||
name: "Fetch with end bigger than size",
|
||||
in: []OpenOption{
|
||||
&RangeOption{Start: 10, End: 200},
|
||||
},
|
||||
want: []OpenOption{
|
||||
&RangeOption{Start: 10, End: 99},
|
||||
},
|
||||
size: 100,
|
||||
},
|
||||
} {
|
||||
FixRangeOption(test.in, test.size)
|
||||
assert.Equal(t, test.want, test.in, test.name)
|
||||
}
|
||||
}
|
||||
|
||||
var testOpenOptions = []OpenOption{
|
||||
&HTTPOption{Key: "a", Value: "1"},
|
||||
&RangeOption{Start: 1, End: 10},
|
||||
&HTTPOption{Key: "b", Value: "2"},
|
||||
NullOption{},
|
||||
&HashesOption{hash.Set(hash.MD5 | hash.SHA1)},
|
||||
}
|
||||
|
||||
func TestOpenOptionAddHeaders(t *testing.T) {
|
||||
m := map[string]string{}
|
||||
want := map[string]string{
|
||||
"a": "1",
|
||||
"Range": "bytes=1-10",
|
||||
"b": "2",
|
||||
}
|
||||
OpenOptionAddHeaders(testOpenOptions, m)
|
||||
assert.Equal(t, want, m)
|
||||
}
|
||||
|
||||
func TestOpenOptionHeaders(t *testing.T) {
|
||||
want := map[string]string{
|
||||
"a": "1",
|
||||
"Range": "bytes=1-10",
|
||||
"b": "2",
|
||||
}
|
||||
m := OpenOptionHeaders(testOpenOptions)
|
||||
assert.Equal(t, want, m)
|
||||
assert.Nil(t, OpenOptionHeaders([]OpenOption{}))
|
||||
}
|
||||
|
||||
func TestOpenOptionAddHTTPHeaders(t *testing.T) {
|
||||
headers := http.Header{}
|
||||
want := http.Header{
|
||||
"A": {"1"},
|
||||
"Range": {"bytes=1-10"},
|
||||
"B": {"2"},
|
||||
}
|
||||
OpenOptionAddHTTPHeaders(headers, testOpenOptions)
|
||||
assert.Equal(t, want, headers)
|
||||
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue