Merge branch 'sh-fix-artifacts-http-range-requests' into 'master'

Fix HTTP Range Requests not working on some S3 providers

Closes gitlab#223806

See merge request gitlab-org/gitlab-workhorse!549

changelogs/unreleased/sh-fix-artifacts-http-range-requests.yml

+---
+title: Fix HTTP Range Requests not working on some S3 providers
+merge_request: 549
+author:
+type: fixed

internal/httprs/httprs.go

@@ -31,13 +31,12 @@ const shortSeekBytes = 1024
 // A HttpReadSeeker reads from a http.Response.Body. It can Seek
 // by doing range requests.
 type HttpReadSeeker struct {
-	c       *http.Client
-	req     *http.Request
-	res     *http.Response
-	ctx     context.Context
-	r       io.ReadCloser
-	pos     int64
-	canSeek bool
+	c   *http.Client
+	req *http.Request
+	res *http.Response
+	ctx context.Context
+	r   io.ReadCloser
+	pos int64
 
 	Requests int
 }
@@ -64,11 +63,10 @@ var (
 // res.Request will be reused for range requests, headers may be added/removed
 func NewHttpReadSeeker(res *http.Response, client ...*http.Client) *HttpReadSeeker {
 	r := &HttpReadSeeker{
-		req:     res.Request,
-		ctx:     res.Request.Context(),
-		res:     res,
-		r:       res.Body,
-		canSeek: (res.Header.Get("Accept-Ranges") == "bytes"),
+		req: res.Request,
+		ctx: res.Request.Context(),
+		res: res,
+		r:   res.Body,
 	}
 	if len(client) > 0 {
 		r.c = client[0]
@@ -85,11 +83,10 @@ func (r *HttpReadSeeker) Clone() (*HttpReadSeeker, error) {
 		return nil, err
 	}
 	return &HttpReadSeeker{
-		req:     req.(*http.Request),
-		res:     r.res,
-		r:       nil,
-		canSeek: r.canSeek,
-		c:       r.c,
+		req: req.(*http.Request),
+		res: r.res,
+		r:   nil,
+		c:   r.c,
 	}, nil
 }
 
@@ -137,9 +134,6 @@ func (r *HttpReadSeeker) Close() error {
 //
 // May return ErrNoContentLength or ErrRangeRequestsNotSupported
 func (r *HttpReadSeeker) Seek(offset int64, whence int) (int64, error) {
-	if !r.canSeek {
-		return 0, ErrRangeRequestsNotSupported
-	}
 	var err error
 	switch whence {
 	case 0:

internal/httprs/httprs_test.go

@@ -66,9 +66,14 @@ func (f *fakeRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) {
 
 const SZ = 4096
 
+const (
+	downgradeZeroToNoRange = 1 << iota
+	sendAcceptRanges
+)
+
 type RSFactory func() *HttpReadSeeker
 
-func newRSFactory(brokenServer bool) RSFactory {
+func newRSFactory(flags int) RSFactory {
 	return func() *HttpReadSeeker {
 		tmp, err := ioutil.TempFile(os.TempDir(), "httprs")
 		if err != nil {
@@ -83,13 +88,16 @@ func newRSFactory(brokenServer bool) RSFactory {
 			return nil
 		}
 		res := &http.Response{
-			Header: http.Header{
-				"Accept-Ranges": []string{"bytes"},
-			},
 			Request:       req,
 			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) {
 		name  string
 		newRS func() *HttpReadSeeker
 	}{
-		{name: "compliant", newRS: newRSFactory(false)},
-		{name: "broken", newRS: newRSFactory(true)},
+		{name: "with no flags", newRS: newRSFactory(0)},
+		{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 {