diff --git a/internal/objectstore/multipart.go b/internal/objectstore/multipart.go
index 09e2f12308ede910f54656089a2c2b352fcf220f..9f8bce4f613075083f15226f3ce2ee1514ae62c1 100644
--- a/internal/objectstore/multipart.go
+++ b/internal/objectstore/multipart.go
@@ -155,11 +155,8 @@ func (m *Multipart) complete(cmu *CompleteMultipartUpload) error {
}
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 {
@@ -167,11 +164,8 @@ func (m *Multipart) verifyETag(cmu *CompleteMultipartUpload) error {
if err != nil {
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) {
diff --git a/internal/objectstore/multipart_test.go b/internal/objectstore/multipart_test.go
new file mode 100644
index 0000000000000000000000000000000000000000..c0a8b46f49af77898d6492d7f23cf399e3b0814c
--- /dev/null
+++ b/internal/objectstore/multipart_test.go
@@ -0,0 +1,69 @@
+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")
+}
diff --git a/internal/objectstore/object.go b/internal/objectstore/object.go
index 70e060ef403b3c1458df7a9bba2b604dfd213329..169d76d7270958d0fdfff947d3bd6368a528e42f 100644
--- a/internal/objectstore/object.go
+++ b/internal/objectstore/object.go
@@ -7,6 +7,7 @@ import (
"io/ioutil"
"net"
"net/http"
+ "strings"
"time"
"gitlab.com/gitlab-org/labkit/correlation"
@@ -124,10 +125,7 @@ func newObject(ctx context.Context, putURL, deleteURL string, putHeaders map[str
}
o.extractETag(resp.Header.Get("ETag"))
- if o.etag != o.md5Sum() {
- o.uploadError = fmt.Errorf("ETag mismatch. expected %q got %q", o.md5Sum(), o.etag)
- return
- }
+ o.uploadError = compareMD5(o.md5Sum(), o.etag)
}()
return o, nil
@@ -136,3 +134,11 @@ func newObject(ctx context.Context, putURL, deleteURL string, putHeaders map[str
func (o *Object) delete() {
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
+}
diff --git a/internal/objectstore/object_test.go b/internal/objectstore/object_test.go
index e5f9a562caf1c428f0bfc55cfe8e1184544e605e..bd367b2769fc4dcb622fc5c8d1c35b4c5dabe41e 100644
--- a/internal/objectstore/object_test.go
+++ b/internal/objectstore/object_test.go
@@ -18,10 +18,12 @@ import (
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)
- osStub, ts := test.StartObjectStore()
+ osStub, ts := startObjectStore()
defer ts.Close()
objectURL := ts.URL + test.ObjectPath
@@ -77,9 +79,26 @@ func testObjectUploadNoErrors(t *testing.T, useDeleteURL bool, contentType strin
}
func TestObjectUpload(t *testing.T) {
- t.Run("with delete URL", func(t *testing.T) { testObjectUploadNoErrors(t, true, "application/octet-stream") })
- t.Run("without delete URL", func(t *testing.T) { testObjectUploadNoErrors(t, false, "application/octet-stream") })
- t.Run("with custom content type", func(t *testing.T) { testObjectUploadNoErrors(t, false, "image/jpeg") })
+ t.Run("with delete URL", func(t *testing.T) {
+ testObjectUploadNoErrors(t, test.StartObjectStore, true, "application/octet-stream")
+ })
+ 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) {