fix: echo the client ETag in InvalidPart errors and stop duplicating request ID headers

Fixes #2525

`CompleteMultipartUpload` returned the stored part's ETag in `InvalidPart` errors instead of the ETag the client sent: posix on an ETag mismatch, and azure on a zero-byte part mismatch, where it returned the expected block ID. Azure also returned an empty ETag for a part that was never uploaded. All three now return the client's ETag. Like S3, `GetInvalidPartErr` removes every `"` from it, inner quotes included, and keeps surrounding whitespace.

`SetResponseHeaders` turns off header name normalization and leaves it off, so response header names stay lowercase. Before it runs, the `RequestIDs` middleware stored `X-Amz-Request-Id` and `X-Amz-Id-2`. Afterwards, the `EnsureRequestIDs` call in `ProcessController` stored `x-amz-request-id` and `x-amz-id-2`. Every response with a headers map, success responses included, therefore sent both pairs. `EnsureRequestIDs`, `RequestID` and `HostID` now use `SetCanonical` and `PeekCanonical` with the lowercase names, so the normalization setting doesn't affect them, and each header is sent once in lowercase as S3 does.

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 {

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)

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 {

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) {

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)
+	}
 }

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, `"`, ""),
 	}
 }

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(`<CompleteMultipartUpload><Part><ETag>invalidETag</ETag><PartNumber>1</PartNumber></Part></CompleteMultipartUpload>`)
-		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, `<CompleteMultipartUpload><Part><ETag>%s</ETag><PartNumber>1</PartNumber></Part></CompleteMultipartUpload>`, 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 {

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)
 }