diff --git a/backend/azure/azure.go b/backend/azure/azure.go index e3648c5d..4d75ae8f 100644 --- a/backend/azure/azure.go +++ b/backend/azure/azure.go @@ -2000,7 +2000,7 @@ func (az *Azure) CompleteMultipartUpload(ctx context.Context, input *s3.Complete if zbPartsMap[*part.PartNumber] { expectedETag := blockIDInt32ToBase64(*part.PartNumber) if clientETag != expectedETag { - return res, "", s3err.GetInvalidPartErr(*input.UploadId, *part.PartNumber, expectedETag) + return res, "", s3err.GetInvalidPartErr(*input.UploadId, *part.PartNumber, clientETag) } // Non-last zero-byte parts violate the minimum part size. if i < last { @@ -2010,7 +2010,7 @@ func (az *Azure) CompleteMultipartUpload(ctx context.Context, input *s3.Complete partSizes = append(partSizes, totalSize) continue } - return res, "", s3err.GetInvalidPartErr(*input.UploadId, *part.PartNumber, "") + return res, "", s3err.GetInvalidPartErr(*input.UploadId, *part.PartNumber, clientETag) } if clientETag != *block.Name { diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 9c5902c0..db77ddab 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -2714,7 +2714,7 @@ func (p *Posix) CompleteMultipartUploadWithCopy(ctx context.Context, input *s3.C return res, "", s3err.GetAPIError(s3err.ErrMalformedXML) } if !backend.AreEtagsSame(etag, *parts[i].ETag) { - return res, "", s3err.GetInvalidPartErr(uploadID, *part.PartNumber, etag) + return res, "", s3err.GetInvalidPartErr(uploadID, *part.PartNumber, *parts[i].ETag) } partChecksum, err := p.retrieveChecksums(nil, bucket, partObjPath) diff --git a/s3api/controllers/base_test.go b/s3api/controllers/base_test.go index 85da3f25..a2b12b22 100644 --- a/s3api/controllers/base_test.go +++ b/s3api/controllers/base_test.go @@ -572,8 +572,8 @@ func TestProcessController(t *testing.T) { // check the status assert.Equal(t, tt.expected.status, ctx.Response().StatusCode()) - assert.Equal(t, testRequestID, string(ctx.Response().Header.Peek(utils.HeaderAmzRequestID))) - assert.Equal(t, testHostID, string(ctx.Response().Header.Peek(utils.HeaderAmzID2))) + assert.Equal(t, testRequestID, string(ctx.Response().Header.PeekCanonical([]byte(utils.HeaderAmzRequestID)))) + assert.Equal(t, testHostID, string(ctx.Response().Header.PeekCanonical([]byte(utils.HeaderAmzID2)))) // check the response headers to be set if tt.expected.headers != nil { diff --git a/s3api/utils/request_ids.go b/s3api/utils/request_ids.go index 0a18b5bf..37752a3d 100644 --- a/s3api/utils/request_ids.go +++ b/s3api/utils/request_ids.go @@ -82,8 +82,12 @@ func EnsureRequestIDs(ctx fiber.Ctx) (requestID, hostID string) { ContextKeyHostID.Set(ctx, hostID) } - ctx.Response().Header.Set(HeaderAmzRequestID, requestID) - ctx.Response().Header.Set(HeaderAmzID2, hostID) + // The canonical variants bypass the response's header name + // normalization flag, which SetResponseHeaders turns off part way + // through a request. Going through Set would store these names in a + // different case before and after that point and emit each twice. + ctx.Response().Header.SetCanonical([]byte(HeaderAmzRequestID), []byte(requestID)) + ctx.Response().Header.SetCanonical([]byte(HeaderAmzID2), []byte(hostID)) return requestID, hostID } @@ -94,7 +98,7 @@ func RequestID(ctx fiber.Ctx) string { return requestID } - return string(ctx.Response().Header.Peek(HeaderAmzRequestID)) + return string(ctx.Response().Header.PeekCanonical([]byte(HeaderAmzRequestID))) } func HostID(ctx fiber.Ctx) string { @@ -103,7 +107,7 @@ func HostID(ctx fiber.Ctx) string { return hostID } - return string(ctx.Response().Header.Peek(HeaderAmzID2)) + return string(ctx.Response().Header.PeekCanonical([]byte(HeaderAmzID2))) } func mustReadRandom(b []byte) { diff --git a/s3api/utils/request_ids_test.go b/s3api/utils/request_ids_test.go index 104c2892..cdba3240 100644 --- a/s3api/utils/request_ids_test.go +++ b/s3api/utils/request_ids_test.go @@ -17,6 +17,7 @@ package utils import ( "encoding/base64" "regexp" + "strings" "testing" "github.com/gofiber/fiber/v3" @@ -48,6 +49,32 @@ func TestEnsureRequestIDs(t *testing.T) { assert.Equal(t, hostID, hostIDAgain) assert.Equal(t, requestID, RequestID(ctx)) assert.Equal(t, hostID, HostID(ctx)) - assert.Equal(t, requestID, string(ctx.Response().Header.Peek(HeaderAmzRequestID))) - assert.Equal(t, hostID, string(ctx.Response().Header.Peek(HeaderAmzID2))) + assert.Equal(t, requestID, string(ctx.Response().Header.PeekCanonical([]byte(HeaderAmzRequestID)))) + assert.Equal(t, hostID, string(ctx.Response().Header.PeekCanonical([]byte(HeaderAmzID2)))) +} + +func TestEnsureRequestIDsNormalizingToggled(t *testing.T) { + for _, disableFirst := range []bool{false, true} { + ctx := fiber.New().AcquireCtx(&fasthttp.RequestCtx{}) + if disableFirst { + ctx.Response().Header.DisableNormalizing() + } + EnsureRequestIDs(ctx) + + // flip the flag the way SetResponseHeaders does mid-request + if disableFirst { + ctx.Response().Header.EnableNormalizing() + } else { + ctx.Response().Header.DisableNormalizing() + } + EnsureRequestIDs(ctx) + + keys := map[string]int{} + for k := range ctx.Response().Header.All() { + if strings.HasPrefix(strings.ToLower(string(k)), "x-amz-") { + keys[string(k)]++ + } + } + assert.Equal(t, map[string]int{HeaderAmzRequestID: 1, HeaderAmzID2: 1}, keys) + } } diff --git a/s3err/invalid-part-error.go b/s3err/invalid-part-error.go index 4cc7fc11..1bc7a59b 100644 --- a/s3err/invalid-part-error.go +++ b/s3err/invalid-part-error.go @@ -16,6 +16,7 @@ package s3err import ( "encoding/xml" + "strings" ) // InvalidPartError is returned when one or more specified parts cannot be found. @@ -61,11 +62,13 @@ func (e InvalidPartError) Is(target error) bool { return ok && e.APIError == t } +// GetInvalidPartErr builds an InvalidPart error. etag is the value the client +// sent; like S3, the error echoes it without the surrounding quotes. func GetInvalidPartErr(uploadId string, partNumber int32, etag string) InvalidPartError { return InvalidPartError{ APIError: GetAPIError(ErrInvalidPart), UploadId: uploadId, PartNumber: partNumber, - ETag: etag, + ETag: strings.ReplaceAll(etag, `"`, ""), } } diff --git a/tests/integration/CompleteMultipartUpload.go b/tests/integration/CompleteMultipartUpload.go index 9a577b3d..ec98eb36 100644 --- a/tests/integration/CompleteMultipartUpload.go +++ b/tests/integration/CompleteMultipartUpload.go @@ -213,22 +213,36 @@ func CompleteMultipartUpload_invalid_ETag_error_fields(s *S3Conf) error { return err } - // the sdk doesn't expose the InvalidPart error fields, so the - // request is signed and sent by hand - body := []byte(`invalidETag1`) - req, err := createSignedReq(http.MethodPost, s.endpoint, - fmt.Sprintf("%v/%v?uploadId=%v", bucket, obj, *out.UploadId), - s.awsID, s.awsSecret, "s3", s.awsRegion, "", body, time.Now(), nil) - if err != nil { - return err + // S3 echoes the ETag the client sent, stripped of quotes, and + // never the ETag of the stored part + for _, test := range []struct { + sent string + echoed string + }{ + {"invalidETag", "invalidETag"}, + {`"00000000000000000000000000000000"`, "00000000000000000000000000000000"}, + } { + // the sdk doesn't expose the InvalidPart error fields, so the + // request is signed and sent by hand + body := fmt.Appendf(nil, `%s1`, test.sent) + req, err := createSignedReq(http.MethodPost, s.endpoint, + fmt.Sprintf("%v/%v?uploadId=%v", bucket, obj, *out.UploadId), + s.awsID, s.awsSecret, "s3", s.awsRegion, "", body, time.Now(), nil) + if err != nil { + return err + } + + resp, err := s.httpClient.Do(req) + if err != nil { + return err + } + + if err := checkHTTPResponseApiErr(resp, s3err.GetInvalidPartErr(*out.UploadId, 1, test.echoed)); err != nil { + return fmt.Errorf("etag %s: %w", test.sent, err) + } } - resp, err := s.httpClient.Do(req) - if err != nil { - return err - } - - return checkHTTPResponseApiErr(resp, s3err.GetInvalidPartErr(*out.UploadId, 1, "invalidETag")) + return nil }) } func CompleteMultipartUpload_invalid_checksum_type(s *S3Conf) error { diff --git a/tests/integration/utils.go b/tests/integration/utils.go index 0a7b1a49..e925c34a 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -532,6 +532,14 @@ func checkHTTPResponseApiErr(resp *http.Response, expected s3err.S3Error) error return err } + // the header map canonicalizes names, so a header sent twice under + // different casings shows up here as two values + for _, name := range []string{"x-amz-request-id", "x-amz-id-2"} { + if got := len(resp.Header.Values(name)); got != 1 { + return fmt.Errorf("expected exactly one %s header, got %v", name, got) + } + } + return compareS3ApiError(expected, &errResp) }