diff --git a/.github/workflows/s3tests.yml b/.github/workflows/s3tests.yml index 980b921ff..b40f8d3ea 100644 --- a/.github/workflows/s3tests.yml +++ b/.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 \ diff --git a/weed/s3api/s3_content_encoding_test.go b/weed/s3api/s3_content_encoding_test.go index a50a8bb3c..0b8f5cd9c 100644 --- a/weed/s3api/s3_content_encoding_test.go +++ b/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"]) + } + }) + } +} diff --git a/weed/s3api/s3_metadata_util.go b/weed/s3api/s3_metadata_util.go index 71ef623f9..6f71776f1 100644 --- a/weed/s3api/s3_metadata_util.go +++ b/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, ", ") +} diff --git a/weed/s3api/s3api_object_handlers_copy.go b/weed/s3api/s3api_object_handlers_copy.go index 222b8c6a4..6f6cbf3ec 100644 --- a/weed/s3api/s3api_object_handlers_copy.go +++ b/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) } } diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go index 53f7fe027..eeea0a736 100644 --- a/weed/s3api/s3api_object_handlers_put.go +++ b/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" {