S3: quote ETag in CopyObject and UploadPartCopy XML responses (#11624)

* s3api: extract quoteETag from setEtag

Consolidate ETag quoting so the XML response builders can share it.

* s3api: quote ETag in CopyObjectResult XML

AWS returns the ETag quoted in the copy result body, matching the ETag header. Fixes seaweedfs/seaweedfs#11622

* s3api: quote ETag in CopyPartResult XML

UploadPartCopy returned the raw ETag in the XML body while the response header and other APIs return it quoted. Fixes seaweedfs/seaweedfs#11622

* s3api: test ETag quoting in copy responses

* s3api: assert quoted ETag on the wire in copy response tests

* s3api: use strconv.Quote in quoteETag

Addresses CodeQL 'potentially unsafe quoting' on string concatenation.

weed/s3api/s3api_object_handlers_copy.go

@@ -374,7 +374,7 @@ func (s3a *S3ApiServer) CopyObjectHandler(w http.ResponseWriter, r *http.Request
 		}
 		setEtag(w, etag)
 		writeSuccessResponseXML(w, r, CopyObjectResult{
-			ETag:         etag,
+			ETag:         quoteETag(etag),
 			LastModified: t,
 		})
 		return
@@ -546,7 +546,7 @@ func (s3a *S3ApiServer) CopyObjectHandler(w http.ResponseWriter, r *http.Request
 	setEtag(w, etag)
 
 	response := CopyObjectResult{
-		ETag:         etag,
+		ETag:         quoteETag(etag),
 		LastModified: t,
 	}
 
@@ -863,7 +863,7 @@ func (r CopyPartResult) MarshalXML(e *xml.Encoder, start xml.StartElement) error
 
 func buildCopyPartResult(etag string, lastModified time.Time, metadata SSEResponseMetadata) CopyPartResult {
 	result := CopyPartResult{
-		ETag:         etag,
+		ETag:         quoteETag(etag),
 		LastModified: lastModified,
 	}
 	result.SetChecksum(metadata.ChecksumHeaderName, metadata.ChecksumValue)
@@ -1086,7 +1086,7 @@ func (s3a *S3ApiServer) CopyObjectPartHandler(w http.ResponseWriter, r *http.Req
 		if !s3a.checkUploadStillOpen(w, r, dstBucket, dstObject, uploadID) {
 			return
 		}
-		setEtag(w, "\""+strings.Trim(etag, "\"")+"\"")
+		setEtag(w, etag)
 		// Mirror PutObjectPartHandler: write x-amz-server-side-encryption /
 		// x-amz-server-side-encryption-aws-kms-key-id headers on the response
 		// so clients can see the destination's encryption state.

weed/s3api/s3api_object_handlers_copy_checksum_test.go

@@ -75,7 +75,7 @@ func TestBuildCopyPartResult(t *testing.T) {
 			header:  s3_constants.AmzChecksumCRC32,
 			element: "<ChecksumCRC32>value</ChecksumCRC32>",
 			expected: CopyPartResult{
-				ETag: "etag", LastModified: modified, ChecksumResult: ChecksumResult{ChecksumCRC32: "value"},
+				ETag: `"etag"`, LastModified: modified, ChecksumResult: ChecksumResult{ChecksumCRC32: "value"},
 			},
 		},
 		{
@@ -83,7 +83,7 @@ func TestBuildCopyPartResult(t *testing.T) {
 			header:  s3_constants.AmzChecksumCRC32C,
 			element: "<ChecksumCRC32C>value</ChecksumCRC32C>",
 			expected: CopyPartResult{
-				ETag: "etag", LastModified: modified, ChecksumResult: ChecksumResult{ChecksumCRC32C: "value"},
+				ETag: `"etag"`, LastModified: modified, ChecksumResult: ChecksumResult{ChecksumCRC32C: "value"},
 			},
 		},
 		{
@@ -91,7 +91,7 @@ func TestBuildCopyPartResult(t *testing.T) {
 			header:  s3_constants.AmzChecksumCRC64NVME,
 			element: "<ChecksumCRC64NVME>value</ChecksumCRC64NVME>",
 			expected: CopyPartResult{
-				ETag: "etag", LastModified: modified, ChecksumResult: ChecksumResult{ChecksumCRC64NVME: "value"},
+				ETag: `"etag"`, LastModified: modified, ChecksumResult: ChecksumResult{ChecksumCRC64NVME: "value"},
 			},
 		},
 		{
@@ -99,7 +99,7 @@ func TestBuildCopyPartResult(t *testing.T) {
 			header:  s3_constants.AmzChecksumSHA1,
 			element: "<ChecksumSHA1>value</ChecksumSHA1>",
 			expected: CopyPartResult{
-				ETag: "etag", LastModified: modified, ChecksumResult: ChecksumResult{ChecksumSHA1: "value"},
+				ETag: `"etag"`, LastModified: modified, ChecksumResult: ChecksumResult{ChecksumSHA1: "value"},
 			},
 		},
 		{
@@ -107,14 +107,14 @@ func TestBuildCopyPartResult(t *testing.T) {
 			header:  s3_constants.AmzChecksumSHA256,
 			element: "<ChecksumSHA256>value</ChecksumSHA256>",
 			expected: CopyPartResult{
-				ETag: "etag", LastModified: modified, ChecksumResult: ChecksumResult{ChecksumSHA256: "value"},
+				ETag: `"etag"`, LastModified: modified, ChecksumResult: ChecksumResult{ChecksumSHA256: "value"},
 			},
 		},
 		{
 			name:     "Unknown",
 			header:   "x-amz-checksum-unknown",
 			element:  "",
-			expected: CopyPartResult{ETag: "etag", LastModified: modified},
+			expected: CopyPartResult{ETag: `"etag"`, LastModified: modified},
 		},
 	}
 

weed/s3api/s3api_object_handlers_copy_etag_test.go

@@ -4,6 +4,7 @@ import (
 	"net/http/httptest"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
 	"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
@@ -64,6 +65,42 @@ func newCopyETagTestEntry(t *testing.T, storedETag, computedETag string) *filer_
 	return entry
 }
 
+func TestQuoteETag(t *testing.T) {
+	testCases := []struct {
+		etag string
+		want string
+	}{
+		{etag: "b1946ac92492d2347c6235b4d2611184", want: `"b1946ac92492d2347c6235b4d2611184"`},
+		{etag: `"b1946ac92492d2347c6235b4d2611184"`, want: `"b1946ac92492d2347c6235b4d2611184"`},
+		{etag: "", want: ""},
+	}
+	for _, tc := range testCases {
+		if got := quoteETag(tc.etag); got != tc.want {
+			t.Errorf("quoteETag(%q) = %q, want %q", tc.etag, got, tc.want)
+		}
+	}
+}
+
+func TestBuildCopyPartResultQuotesETag(t *testing.T) {
+	result := buildCopyPartResult("b1946ac92492d2347c6235b4d2611184", time.Now(), SSEResponseMetadata{})
+	if result.ETag != `"b1946ac92492d2347c6235b4d2611184"` {
+		t.Fatalf("buildCopyPartResult().ETag = %q, want quoted", result.ETag)
+	}
+	if encoded := string(s3err.EncodeXMLResponse(result)); !strings.Contains(encoded, `<ETag>&#34;b1946ac92492d2347c6235b4d2611184&#34;</ETag>`) {
+		t.Fatalf("response %q does not contain a quoted ETag", encoded)
+	}
+}
+
+func TestCopyObjectResultXMLQuotesETag(t *testing.T) {
+	encoded := string(s3err.EncodeXMLResponse(CopyObjectResult{
+		ETag:         quoteETag("b1946ac92492d2347c6235b4d2611184"),
+		LastModified: time.Now(),
+	}))
+	if !strings.Contains(encoded, `<ETag>&#34;b1946ac92492d2347c6235b4d2611184&#34;</ETag>`) {
+		t.Fatalf("response %q does not contain a quoted ETag", encoded)
+	}
+}
+
 // A part copy has no way to report a short part, so an unsatisfiable
 // x-amz-copy-source-range has to be rejected rather than clamped like a GET —
 // the re-encrypting path would otherwise pad the part out with zeros.

weed/s3api/s3api_object_handlers_put.go

@@ -1397,14 +1397,17 @@ func (s3a *S3ApiServer) resolveFileMode(r *http.Request) uint32 {
 
 func setEtag(w http.ResponseWriter, etag string) {
 	if etag != "" {
-		if strings.HasPrefix(etag, "\"") {
-			w.Header()["ETag"] = []string{etag}
-		} else {
-			w.Header()["ETag"] = []string{"\"" + etag + "\""}
-		}
+		w.Header()["ETag"] = []string{quoteETag(etag)}
 	}
 }
 
+func quoteETag(etag string) string {
+	if etag == "" || strings.HasPrefix(etag, "\"") {
+		return etag
+	}
+	return strconv.Quote(etag)
+}
+
 // setSSEResponseHeaders sets appropriate SSE response headers based on encryption type
 func (s3a *S3ApiServer) setSSEResponseHeaders(w http.ResponseWriter, r *http.Request, sseMetadata SSEResponseMetadata) {
 	switch sseMetadata.SSEType {