s3api: do not store aws-chunked in an object's Content-Encoding (#11528)

* s3api: do not store aws-chunked in an object's Content-Encoding

aws-chunked in Content-Encoding names the SigV4 streaming framing of
the request body, which the gateway decodes on upload. PutObject and
CreateMultipartUpload stored the header as sent, so an object uploaded
with "gzip, aws-chunked" was served with that Content-Encoding, and one
uploaded with "aws-chunked" alone was served as aws-chunked. S3 drops
aws-chunked and keeps the other encodings.

Also run ceph/s3-tests' test_object_content_encoding_aws_chunked in CI.

* s3api: read every Content-Encoding field, and drop aws-chunked on copy

A client can send aws-chunked and the object's own encoding as separate
Content-Encoding fields. Only the first was read, so "aws-chunked"
followed by "gzip" left the object without its gzip. Combine all the
fields before dropping aws-chunked. CopyObject with the REPLACE
directive stored the requested Content-Encoding as sent: drop
aws-chunked there too.

.github/workflows/s3tests.yml

@@ -289,6 +289,7 @@ jobs:
           s3tests/functional/test_s3.py::test_object_write_check_etag \
           s3tests/functional/test_s3.py::test_object_write_cache_control \
           s3tests/functional/test_s3.py::test_object_write_expires \
+          s3tests/functional/test_s3.py::test_object_content_encoding_aws_chunked \
           s3tests/functional/test_s3.py::test_object_write_read_update_read_delete \
           s3tests/functional/test_s3.py::test_object_metadata_replaced_on_put \
           s3tests/functional/test_s3.py::test_object_write_file \
@@ -1149,6 +1150,7 @@ jobs:
           s3tests/functional/test_s3.py::test_object_write_check_etag \
           s3tests/functional/test_s3.py::test_object_write_cache_control \
           s3tests/functional/test_s3.py::test_object_write_expires \
+          s3tests/functional/test_s3.py::test_object_content_encoding_aws_chunked \
           s3tests/functional/test_s3.py::test_object_write_read_update_read_delete \
           s3tests/functional/test_s3.py::test_object_metadata_replaced_on_put \
           s3tests/functional/test_s3.py::test_object_write_file \

weed/s3api/s3_content_encoding_test.go

@@ -2,6 +2,7 @@ package s3api
 
 import (
 	"bytes"
+	"net/http"
 	"net/http/httptest"
 	"testing"
 
@@ -202,3 +203,57 @@ func TestContentEncodingWithOtherHeaders(t *testing.T) {
 	assert.Equal(t, "max-age=3600", getResp.Header().Get("Cache-Control"))
 	assert.Equal(t, "attachment; filename=test.txt", getResp.Header().Get("Content-Disposition"))
 }
+
+// TestContentEncodingDropsAwsChunked verifies that aws-chunked, the SigV4
+// streaming framing of the request body, is not stored with the object, also
+// when the encodings come in separate Content-Encoding fields
+func TestContentEncodingDropsAwsChunked(t *testing.T) {
+	testCases := []struct {
+		name   string
+		fields []string
+		stored string
+	}{
+		{"last", []string{"gzip, aws-chunked"}, "gzip"},
+		{"first", []string{"aws-chunked, gzip"}, "gzip"},
+		{"no spaces", []string{"aws-chunked,gzip,br"}, "gzip, br"},
+		{"alone", []string{"aws-chunked"}, ""},
+		{"capitals", []string{"AWS-Chunked"}, ""},
+		{"twice", []string{"aws-chunked, aws-chunked"}, ""},
+		{"separate fields", []string{"aws-chunked", "gzip"}, "gzip"},
+		{"separate fields, alone", []string{"aws-chunked", "aws-chunked"}, ""},
+		{"without aws-chunked", []string{"deflate, gzip"}, "deflate, gzip"},
+		{"without aws-chunked, separate fields", []string{"deflate", "gzip"}, "deflate, gzip"},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, tc.stored, storedContentEncoding(tc.fields))
+
+			// CreateMultipartUpload
+			putReq := httptest.NewRequest("PUT", "/test-bucket/test-object.txt", bytes.NewBufferString("body"))
+			for _, field := range tc.fields {
+				putReq.Header.Add("Content-Encoding", field)
+			}
+			metadata, errCode := ParseS3Metadata(putReq, nil, false)
+			require.Equal(t, 0, int(errCode))
+			if tc.stored == "" {
+				assert.NotContains(t, metadata, "Content-Encoding")
+			} else {
+				assert.Equal(t, []byte(tc.stored), metadata["Content-Encoding"])
+			}
+
+			// CopyObject with the REPLACE metadata directive
+			copyReq := http.Header{}
+			for _, field := range tc.fields {
+				copyReq.Add("Content-Encoding", field)
+			}
+			metadata, err := processMetadataBytes(copyReq, map[string][]byte{"Content-Encoding": []byte("br")}, true, false)
+			require.NoError(t, err)
+			if tc.stored == "" {
+				assert.NotContains(t, metadata, "Content-Encoding")
+			} else {
+				assert.Equal(t, []byte(tc.stored), metadata["Content-Encoding"])
+			}
+		})
+	}
+}

weed/s3api/s3_metadata_util.go

@@ -30,7 +30,7 @@ func ParseS3Metadata(r *http.Request, existing map[string][]byte, isReplace bool
 	}
 
 	// Content-Encoding (standard HTTP header used by S3)
-	if ce := r.Header.Get("Content-Encoding"); ce != "" {
+	if ce := storedContentEncoding(r.Header.Values("Content-Encoding")); ce != "" {
 		metadata["Content-Encoding"] = []byte(ce)
 	}
 
@@ -108,3 +108,28 @@ func ParseS3Metadata(r *http.Request, existing map[string][]byte, isReplace bool
 
 	return metadata, s3err.ErrNone
 }
+
+// storedContentEncoding returns the Content-Encoding to keep with an object,
+// from the values of the request's Content-Encoding fields, which it combines
+// as one list. aws-chunked names the SigV4 streaming framing of the request
+// body, which is decoded on upload, so S3 does not store it: "gzip,
+// aws-chunked" is kept as "gzip", and "aws-chunked" alone as no
+// Content-Encoding at all.
+func storedContentEncoding(values []string) string {
+	value := strings.Join(values, ", ")
+	var kept []string
+	chunked := false
+	for _, encoding := range strings.Split(value, ",") {
+		encoding = strings.TrimSpace(encoding)
+		switch {
+		case strings.EqualFold(encoding, "aws-chunked"):
+			chunked = true
+		case encoding != "":
+			kept = append(kept, encoding)
+		}
+	}
+	if !chunked {
+		return value
+	}
+	return strings.Join(kept, ", ")
+}

weed/s3api/s3api_object_handlers_copy.go

@@ -1197,7 +1197,11 @@ func processMetadataBytes(reqHeader http.Header, existing map[string][]byte, rep
 			}
 		}
 		for _, h := range copyReplaceSystemHeaders {
-			if v := reqHeader.Get(h); v != "" {
+			v := reqHeader.Get(h)
+			if h == "Content-Encoding" {
+				v = storedContentEncoding(reqHeader.Values(h))
+			}
+			if v != "" {
 				metadata[h] = []byte(v)
 			}
 		}

weed/s3api/s3api_object_handlers_put.go

@@ -833,8 +833,12 @@ func (s3a *S3ApiServer) putToFiler(r *http.Request, filePath string, dataReader
 				entry.Extended[k] = []byte(v[0])
 			} else {
 				switch k {
-				case "Cache-Control", "Expires", "Content-Disposition", "Content-Encoding", "Content-Language":
+				case "Cache-Control", "Expires", "Content-Disposition", "Content-Language":
 					entry.Extended[k] = []byte(v[0])
+				case "Content-Encoding":
+					if ce := storedContentEncoding(v); ce != "" {
+						entry.Extended[k] = []byte(ce)
+					}
 				}
 			}
 			if k == "Response-Content-Disposition" {