Commit 911f5910 authored by Alessio Caiazza's avatar Alessio Caiazza Committed by Nick Thomas

Case insensitive ETag comparison

Some object storage providers returns upcase ETag headers.

This commit will perform the check in case insensitive mode
parent e6b0c5e5
...@@ -155,11 +155,8 @@ func (m *Multipart) complete(cmu *CompleteMultipartUpload) error { ...@@ -155,11 +155,8 @@ func (m *Multipart) complete(cmu *CompleteMultipartUpload) error {
} }
m.extractETag(result.ETag) m.extractETag(result.ETag)
if err := m.verifyETag(cmu); err != nil {
return fmt.Errorf("ETag verification failure: %v", err)
}
return nil return m.verifyETag(cmu)
} }
func (m *Multipart) verifyETag(cmu *CompleteMultipartUpload) error { func (m *Multipart) verifyETag(cmu *CompleteMultipartUpload) error {
...@@ -167,11 +164,8 @@ func (m *Multipart) verifyETag(cmu *CompleteMultipartUpload) error { ...@@ -167,11 +164,8 @@ func (m *Multipart) verifyETag(cmu *CompleteMultipartUpload) error {
if err != nil { if err != nil {
return err return err
} }
if expectedChecksum != m.etag {
return fmt.Errorf("got %q expected %q", m.etag, expectedChecksum)
}
return nil return compareMD5(expectedChecksum, m.etag)
} }
func (m *Multipart) readAndUploadOnePart(partURL string, putHeaders map[string]string, src io.Reader, partNumber int) (*completeMultipartUploadPart, error) { func (m *Multipart) readAndUploadOnePart(partURL string, putHeaders map[string]string, src io.Reader, partNumber int) (*completeMultipartUploadPart, error) {
......
package objectstore_test
import (
"context"
"fmt"
"io/ioutil"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"
"github.com/stretchr/testify/require"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/objectstore"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/objectstore/test"
)
func TestMultipartUploadWithUpcaseETags(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
var putCnt, postCnt int
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_, err := ioutil.ReadAll(r.Body)
require.NoError(t, err)
defer r.Body.Close()
// Part upload request
if r.Method == "PUT" {
putCnt++
w.Header().Set("ETag", strings.ToUpper(test.ObjectMD5))
}
// POST with CompleteMultipartUpload request
if r.Method == "POST" {
expectedETag := "6e6b164c392b04bfbb82368179d9ade2-1"
completeBody := fmt.Sprintf(`<CompleteMultipartUploadResult>
<Bucket>test-bucket</Bucket>
<ETag>%s</ETag>
</CompleteMultipartUploadResult>`,
strings.ToUpper(expectedETag))
postCnt++
w.Write([]byte(completeBody))
}
}))
defer ts.Close()
deadline := time.Now().Add(testTimeout)
m, err := objectstore.NewMultipart(ctx,
[]string{ts.URL}, // a single presigned part URL
ts.URL, // the complete multipart upload URL
"", // no abort
"", // no delete
map[string]string{}, // no custom headers
deadline,
test.ObjectSize) // parts size equal to the whole content. Only 1 part
require.NoError(t, err)
_, err = m.Write([]byte(test.ObjectContent))
require.NoError(t, err)
require.NoError(t, m.Close())
require.Equal(t, 1, putCnt, "1 part expected")
require.Equal(t, 1, postCnt, "1 complete multipart upload expected")
}
...@@ -7,6 +7,7 @@ import ( ...@@ -7,6 +7,7 @@ import (
"io/ioutil" "io/ioutil"
"net" "net"
"net/http" "net/http"
"strings"
"time" "time"
"gitlab.com/gitlab-org/labkit/correlation" "gitlab.com/gitlab-org/labkit/correlation"
...@@ -124,10 +125,7 @@ func newObject(ctx context.Context, putURL, deleteURL string, putHeaders map[str ...@@ -124,10 +125,7 @@ func newObject(ctx context.Context, putURL, deleteURL string, putHeaders map[str
} }
o.extractETag(resp.Header.Get("ETag")) o.extractETag(resp.Header.Get("ETag"))
if o.etag != o.md5Sum() { o.uploadError = compareMD5(o.md5Sum(), o.etag)
o.uploadError = fmt.Errorf("ETag mismatch. expected %q got %q", o.md5Sum(), o.etag)
return
}
}() }()
return o, nil return o, nil
...@@ -136,3 +134,11 @@ func newObject(ctx context.Context, putURL, deleteURL string, putHeaders map[str ...@@ -136,3 +134,11 @@ func newObject(ctx context.Context, putURL, deleteURL string, putHeaders map[str
func (o *Object) delete() { func (o *Object) delete() {
o.syncAndDelete(o.DeleteURL) o.syncAndDelete(o.DeleteURL)
} }
func compareMD5(local, remote string) error {
if !strings.EqualFold(local, remote) {
return fmt.Errorf("ETag mismatch. expected %q got %q", local, remote)
}
return nil
}
...@@ -18,10 +18,12 @@ import ( ...@@ -18,10 +18,12 @@ import (
const testTimeout = 10 * time.Second const testTimeout = 10 * time.Second
func testObjectUploadNoErrors(t *testing.T, useDeleteURL bool, contentType string) { type osFactory func() (*test.ObjectstoreStub, *httptest.Server)
func testObjectUploadNoErrors(t *testing.T, startObjectStore osFactory, useDeleteURL bool, contentType string) {
assert := assert.New(t) assert := assert.New(t)
osStub, ts := test.StartObjectStore() osStub, ts := startObjectStore()
defer ts.Close() defer ts.Close()
objectURL := ts.URL + test.ObjectPath objectURL := ts.URL + test.ObjectPath
...@@ -77,9 +79,26 @@ func testObjectUploadNoErrors(t *testing.T, useDeleteURL bool, contentType strin ...@@ -77,9 +79,26 @@ func testObjectUploadNoErrors(t *testing.T, useDeleteURL bool, contentType strin
} }
func TestObjectUpload(t *testing.T) { func TestObjectUpload(t *testing.T) {
t.Run("with delete URL", func(t *testing.T) { testObjectUploadNoErrors(t, true, "application/octet-stream") }) t.Run("with delete URL", func(t *testing.T) {
t.Run("without delete URL", func(t *testing.T) { testObjectUploadNoErrors(t, false, "application/octet-stream") }) testObjectUploadNoErrors(t, test.StartObjectStore, true, "application/octet-stream")
t.Run("with custom content type", func(t *testing.T) { testObjectUploadNoErrors(t, false, "image/jpeg") }) })
t.Run("without delete URL", func(t *testing.T) {
testObjectUploadNoErrors(t, test.StartObjectStore, false, "application/octet-stream")
})
t.Run("with custom content type", func(t *testing.T) {
testObjectUploadNoErrors(t, test.StartObjectStore, false, "image/jpeg")
})
t.Run("with upcase ETAG", func(t *testing.T) {
factory := func() (*test.ObjectstoreStub, *httptest.Server) {
md5s := map[string]string{
test.ObjectPath: strings.ToUpper(test.ObjectMD5),
}
return test.StartObjectStoreWithCustomMD5(md5s)
}
testObjectUploadNoErrors(t, factory, false, "application/octet-stream")
})
} }
func TestObjectUpload404(t *testing.T) { func TestObjectUpload404(t *testing.T) {
......
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