Commit cfe75f1e authored by Stan Hu's avatar Stan Hu

Fix HTTP Range Requests not working on some S3 providers

According to
https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.35:

Origin servers that accept byte-range requests MAY send Accept-Ranges:
bytes, but are not required to do so. Clients MAY generate byte-range
requests without having received this header for the resource involved.

Since some servers (e.g. Dell ECS) don't send an `Accept-Ranges: bytes`
header and we are already checking that range requests are supported
when we call `HttpReadSeeker.rangeRequest`, `canSeek` only becomes a
problem and should be removed.

Since it's not clear whether https://github.com/jfbus/httprs is actively
maintained, this commit applies https://github.com/jfbus/httprs/pull/6
to our vendored module.

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/223806
parent 9a10d347
---
title: Fix HTTP Range Requests not working on some S3 providers
merge_request: 549
author:
type: fixed
...@@ -31,13 +31,12 @@ const shortSeekBytes = 1024 ...@@ -31,13 +31,12 @@ const shortSeekBytes = 1024
// A HttpReadSeeker reads from a http.Response.Body. It can Seek // A HttpReadSeeker reads from a http.Response.Body. It can Seek
// by doing range requests. // by doing range requests.
type HttpReadSeeker struct { type HttpReadSeeker struct {
c *http.Client c *http.Client
req *http.Request req *http.Request
res *http.Response res *http.Response
ctx context.Context ctx context.Context
r io.ReadCloser r io.ReadCloser
pos int64 pos int64
canSeek bool
Requests int Requests int
} }
...@@ -64,11 +63,10 @@ var ( ...@@ -64,11 +63,10 @@ var (
// res.Request will be reused for range requests, headers may be added/removed // res.Request will be reused for range requests, headers may be added/removed
func NewHttpReadSeeker(res *http.Response, client ...*http.Client) *HttpReadSeeker { func NewHttpReadSeeker(res *http.Response, client ...*http.Client) *HttpReadSeeker {
r := &HttpReadSeeker{ r := &HttpReadSeeker{
req: res.Request, req: res.Request,
ctx: res.Request.Context(), ctx: res.Request.Context(),
res: res, res: res,
r: res.Body, r: res.Body,
canSeek: (res.Header.Get("Accept-Ranges") == "bytes"),
} }
if len(client) > 0 { if len(client) > 0 {
r.c = client[0] r.c = client[0]
...@@ -85,11 +83,10 @@ func (r *HttpReadSeeker) Clone() (*HttpReadSeeker, error) { ...@@ -85,11 +83,10 @@ func (r *HttpReadSeeker) Clone() (*HttpReadSeeker, error) {
return nil, err return nil, err
} }
return &HttpReadSeeker{ return &HttpReadSeeker{
req: req.(*http.Request), req: req.(*http.Request),
res: r.res, res: r.res,
r: nil, r: nil,
canSeek: r.canSeek, c: r.c,
c: r.c,
}, nil }, nil
} }
...@@ -137,9 +134,6 @@ func (r *HttpReadSeeker) Close() error { ...@@ -137,9 +134,6 @@ func (r *HttpReadSeeker) Close() error {
// //
// May return ErrNoContentLength or ErrRangeRequestsNotSupported // May return ErrNoContentLength or ErrRangeRequestsNotSupported
func (r *HttpReadSeeker) Seek(offset int64, whence int) (int64, error) { func (r *HttpReadSeeker) Seek(offset int64, whence int) (int64, error) {
if !r.canSeek {
return 0, ErrRangeRequestsNotSupported
}
var err error var err error
switch whence { switch whence {
case 0: case 0:
......
...@@ -66,9 +66,14 @@ func (f *fakeRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) { ...@@ -66,9 +66,14 @@ func (f *fakeRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) {
const SZ = 4096 const SZ = 4096
const (
downgradeZeroToNoRange = 1 << iota
sendAcceptRanges
)
type RSFactory func() *HttpReadSeeker type RSFactory func() *HttpReadSeeker
func newRSFactory(brokenServer bool) RSFactory { func newRSFactory(flags int) RSFactory {
return func() *HttpReadSeeker { return func() *HttpReadSeeker {
tmp, err := ioutil.TempFile(os.TempDir(), "httprs") tmp, err := ioutil.TempFile(os.TempDir(), "httprs")
if err != nil { if err != nil {
...@@ -83,13 +88,16 @@ func newRSFactory(brokenServer bool) RSFactory { ...@@ -83,13 +88,16 @@ func newRSFactory(brokenServer bool) RSFactory {
return nil return nil
} }
res := &http.Response{ res := &http.Response{
Header: http.Header{
"Accept-Ranges": []string{"bytes"},
},
Request: req, Request: req,
ContentLength: SZ * 4, ContentLength: SZ * 4,
} }
return NewHttpReadSeeker(res, &http.Client{Transport: &fakeRoundTripper{src: tmp, downgradeZeroToNoRange: brokenServer}})
if flags&sendAcceptRanges > 0 {
res.Header = http.Header{"Accept-Ranges": []string{"bytes"}}
}
downgradeZeroToNoRange := (flags & downgradeZeroToNoRange) > 0
return NewHttpReadSeeker(res, &http.Client{Transport: &fakeRoundTripper{src: tmp, downgradeZeroToNoRange: downgradeZeroToNoRange}})
} }
} }
...@@ -138,8 +146,10 @@ func TestHttpReaderSeeker(t *testing.T) { ...@@ -138,8 +146,10 @@ func TestHttpReaderSeeker(t *testing.T) {
name string name string
newRS func() *HttpReadSeeker newRS func() *HttpReadSeeker
}{ }{
{name: "compliant", newRS: newRSFactory(false)}, {name: "with no flags", newRS: newRSFactory(0)},
{name: "broken", newRS: newRSFactory(true)}, {name: "with only Accept-Ranges", newRS: newRSFactory(sendAcceptRanges)},
{name: "downgrade 0-range to no range", newRS: newRSFactory(downgradeZeroToNoRange)},
{name: "downgrade 0-range with Accept-Ranges", newRS: newRSFactory(downgradeZeroToNoRange | sendAcceptRanges)},
} }
for _, test := range tests { for _, test := range tests {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment