From 0bddc2652e27bf1f75a23065543ea4b191492ebe Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Fri, 17 Apr 2026 14:55:06 -0700 Subject: [PATCH] fix(s3): propagate validated POST form fields to upload headers (#9123) * fix(s3): propagate validated POST form fields to upload headers POST Object form fields like acl, Content-Encoding, x-amz-storage-class, x-amz-tagging, and x-amz-server-side-encryption were validated against the POST policy but never forwarded to the underlying PUT, so the validated values had no effect. Forward all non-reserved x-amz-* fields plus acl (as x-amz-acl) and Content-Encoding. Reserved POST policy mechanism fields (Policy, Signature, Key, etc.) are still excluded. * fix(s3): also forward Content-Language from POST form to upload headers AWS S3 POST Object supports Content-Language; add it to the set of content headers forwarded by applyPostPolicyFormHeaders alongside Cache-Control, Expires, Content-Disposition, and Content-Encoding. Addresses gemini-code-assist review on PR #9123. * refactor(s3): only look up form value in branches that use it applyPostPolicyFormHeaders previously called formValues.Get(k) for every form field, including fields that fall through the switch (non-reserved fields that are neither Acl, a forwarded content header, nor X-Amz-*). Move the lookup inside the switch cases that actually use it. --- .../s3api/s3api_object_handlers_postpolicy.go | 65 +++++-- .../s3api_object_handlers_postpolicy_test.go | 165 ++++++++++++++++++ 2 files changed, 219 insertions(+), 11 deletions(-) diff --git a/weed/s3api/s3api_object_handlers_postpolicy.go b/weed/s3api/s3api_object_handlers_postpolicy.go index c17953d5c..c82cdfabd 100644 --- a/weed/s3api/s3api_object_handlers_postpolicy.go +++ b/weed/s3api/s3api_object_handlers_postpolicy.go @@ -128,17 +128,8 @@ func (s3a *S3ApiServer) PostPolicyBucketHandler(w http.ResponseWriter, r *http.R } r.Header.Set("Content-Type", contentType) - // Add s3 postpolicy support header - for k, _ := range formValues { - if k == "Cache-Control" || k == "Expires" || k == "Content-Disposition" { - r.Header.Set(k, formValues.Get(k)) - continue - } - - if strings.HasPrefix(k, s3_constants.AmzUserMetaPrefix) { - r.Header.Set(k, formValues.Get(k)) - } - } + // Forward validated POST form fields to the underlying PUT as headers. + applyPostPolicyFormHeaders(r, formValues) etag, errCode, sseMetadata := s3a.putToFiler(r, filePath, fileBody, bucket, object, 1, nil) @@ -180,6 +171,58 @@ func (s3a *S3ApiServer) PostPolicyBucketHandler(w http.ResponseWriter, r *http.R } +// postPolicyReservedFormFields are multipart form fields that are part of the +// POST Object auth/policy mechanism (or handled explicitly elsewhere in the +// handler) and must not be forwarded to the upload as HTTP headers. Keys are +// already run through http.CanonicalHeaderKey before lookup. +var postPolicyReservedFormFields = map[string]struct{}{ + // POST policy signature (V2) + "Policy": {}, + "Signature": {}, + "Awsaccesskeyid": {}, + // POST policy signature (V4) + "X-Amz-Signature": {}, + "X-Amz-Credential": {}, + "X-Amz-Algorithm": {}, + "X-Amz-Date": {}, + "X-Amz-Security-Token": {}, + // Target descriptors + "Key": {}, + "File": {}, + "Bucket": {}, + // Success actions (handled elsewhere in the handler) + "Success_action_redirect": {}, + "Success_action_status": {}, + "Redirect": {}, + // Content-Type is resolved separately above (from form or file part) + "Content-Type": {}, +} + +// applyPostPolicyFormHeaders forwards validated POST Object form fields to the +// request headers so they are applied to the resulting PUT. Reserved fields +// that are part of the POST policy mechanism itself (signature, key, etc.) are +// skipped. The acl form field is translated to the X-Amz-Acl header to match +// how AWS promotes the form value to the underlying PUT. +func applyPostPolicyFormHeaders(r *http.Request, formValues http.Header) { + for k := range formValues { + if _, reserved := postPolicyReservedFormFields[k]; reserved { + continue + } + switch { + case k == "Acl": + r.Header.Set(s3_constants.AmzCannedAcl, formValues.Get(k)) + case k == "Cache-Control", + k == "Expires", + k == "Content-Disposition", + k == "Content-Encoding", + k == "Content-Language": + r.Header.Set(k, formValues.Get(k)) + case strings.HasPrefix(k, "X-Amz-"): + r.Header.Set(k, formValues.Get(k)) + } + } +} + // Extract form fields and file data from a HTTP POST Policy func extractPostPolicyFormValues(form *multipart.Form) (filePart io.ReadCloser, fileName, fileContentType string, fileSize int64, formValues http.Header, err error) { // / HTML Form values diff --git a/weed/s3api/s3api_object_handlers_postpolicy_test.go b/weed/s3api/s3api_object_handlers_postpolicy_test.go index fd6c65960..8e566d55e 100644 --- a/weed/s3api/s3api_object_handlers_postpolicy_test.go +++ b/weed/s3api/s3api_object_handlers_postpolicy_test.go @@ -312,6 +312,171 @@ func TestPostPolicyPathConstruction(t *testing.T) { } } +// canonicalFormValues builds an http.Header mirroring how +// extractPostPolicyFormValues canonicalizes the keys in the POST form. +func canonicalFormValues(pairs map[string]string) http.Header { + h := make(http.Header) + for k, v := range pairs { + h[http.CanonicalHeaderKey(k)] = []string{v} + } + return h +} + +// TestApplyPostPolicyFormHeaders_ForwardsAcl ensures the acl form field is +// promoted to the X-Amz-Acl header on the underlying PUT. +func TestApplyPostPolicyFormHeaders_ForwardsAcl(t *testing.T) { + r := httptest.NewRequest(http.MethodPost, "/bucket", nil) + formValues := canonicalFormValues(map[string]string{ + "acl": "public-read", + }) + + applyPostPolicyFormHeaders(r, formValues) + + assert.Equal(t, "public-read", r.Header.Get(s3_constants.AmzCannedAcl)) + assert.Equal(t, "public-read", r.Header.Get("X-Amz-Acl")) + // The raw "Acl" form key must not be copied verbatim onto the request. + assert.Empty(t, r.Header.Get("Acl")) +} + +// TestApplyPostPolicyFormHeaders_ForwardsContentHeaders ensures the +// Content-Encoding and Content-Language form fields are copied to the +// request header so they reach the underlying PUT. +func TestApplyPostPolicyFormHeaders_ForwardsContentHeaders(t *testing.T) { + tests := []struct { + name string + header string + value string + }{ + { + name: "Content-Encoding", + header: "Content-Encoding", + value: "gzip", + }, + { + name: "Content-Language", + header: "Content-Language", + value: "en-US", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := httptest.NewRequest(http.MethodPost, "/bucket", nil) + formValues := canonicalFormValues(map[string]string{ + tt.header: tt.value, + }) + + applyPostPolicyFormHeaders(r, formValues) + + assert.Equal(t, tt.value, r.Header.Get(tt.header)) + }) + } +} + +// TestApplyPostPolicyFormHeaders_ForwardsXAmzHeaders ensures arbitrary +// x-amz-* form fields (storage class, tagging, SSE, object lock, website +// redirect, metadata) are all forwarded to the request. +func TestApplyPostPolicyFormHeaders_ForwardsXAmzHeaders(t *testing.T) { + r := httptest.NewRequest(http.MethodPost, "/bucket", nil) + formValues := canonicalFormValues(map[string]string{ + "x-amz-storage-class": "STANDARD_IA", + "x-amz-tagging": "project=alpha&env=prod", + "x-amz-server-side-encryption": "AES256", + "x-amz-object-lock-mode": "GOVERNANCE", + "x-amz-website-redirect-location": "/redirected", + "x-amz-meta-foo": "bar", + }) + + applyPostPolicyFormHeaders(r, formValues) + + assert.Equal(t, "STANDARD_IA", r.Header.Get("X-Amz-Storage-Class")) + assert.Equal(t, "project=alpha&env=prod", r.Header.Get("X-Amz-Tagging")) + assert.Equal(t, "AES256", r.Header.Get("X-Amz-Server-Side-Encryption")) + assert.Equal(t, "GOVERNANCE", r.Header.Get("X-Amz-Object-Lock-Mode")) + assert.Equal(t, "/redirected", r.Header.Get("X-Amz-Website-Redirect-Location")) + assert.Equal(t, "bar", r.Header.Get("X-Amz-Meta-Foo")) +} + +// TestApplyPostPolicyFormHeaders_SkipsReserved ensures POST-policy +// mechanism fields (signatures, key, bucket, success actions, etc.) are not +// leaked as headers on the forwarded PUT. +func TestApplyPostPolicyFormHeaders_SkipsReserved(t *testing.T) { + r := httptest.NewRequest(http.MethodPost, "/bucket", nil) + formValues := canonicalFormValues(map[string]string{ + "Policy": "base64-encoded-policy", + "Signature": "v2-signature", + "AWSAccessKeyId": "AKIAEXAMPLE", + "x-amz-signature": "v4-signature", + "x-amz-credential": "AKIAEXAMPLE/20260417/us-east-1/s3/aws4_request", + "x-amz-algorithm": "AWS4-HMAC-SHA256", + "x-amz-date": "20260417T000000Z", + "x-amz-security-token": "session-token", + "key": "uploads/file.txt", + "file": "ignored", + "bucket": "my-bucket", + "success_action_redirect": "https://example.com/ok", + "success_action_status": "201", + "redirect": "https://example.com/legacy", + }) + + applyPostPolicyFormHeaders(r, formValues) + + reserved := []string{ + "Policy", + "Signature", + "Awsaccesskeyid", + "X-Amz-Signature", + "X-Amz-Credential", + "X-Amz-Algorithm", + "X-Amz-Date", + "X-Amz-Security-Token", + "Key", + "File", + "Bucket", + "Success_action_redirect", + "Success_action_status", + "Redirect", + } + for _, k := range reserved { + assert.Empty(t, r.Header.Get(k), "reserved field %q must not be forwarded", k) + } +} + +// TestApplyPostPolicyFormHeaders_KeepsExistingCacheControl is a regression +// check that the headers previously forwarded (Cache-Control, Expires, +// Content-Disposition) remain forwarded by the refactored helper. +func TestApplyPostPolicyFormHeaders_KeepsExistingCacheControl(t *testing.T) { + r := httptest.NewRequest(http.MethodPost, "/bucket", nil) + formValues := canonicalFormValues(map[string]string{ + "Cache-Control": "max-age=3600", + "Expires": "Wed, 21 Oct 2026 07:28:00 GMT", + "Content-Disposition": `attachment; filename="report.pdf"`, + }) + + applyPostPolicyFormHeaders(r, formValues) + + assert.Equal(t, "max-age=3600", r.Header.Get("Cache-Control")) + assert.Equal(t, "Wed, 21 Oct 2026 07:28:00 GMT", r.Header.Get("Expires")) + assert.Equal(t, `attachment; filename="report.pdf"`, r.Header.Get("Content-Disposition")) +} + +// TestApplyPostPolicyFormHeaders_IgnoresContentType ensures Content-Type is +// left alone by the helper (it is set by the caller from either the form +// value or the uploaded file part just before invoking the helper). +func TestApplyPostPolicyFormHeaders_IgnoresContentType(t *testing.T) { + r := httptest.NewRequest(http.MethodPost, "/bucket", nil) + r.Header.Set("Content-Type", "image/png") + formValues := canonicalFormValues(map[string]string{ + "Content-Type": "text/plain", + }) + + applyPostPolicyFormHeaders(r, formValues) + + // The helper must not overwrite the Content-Type that the handler has + // already resolved from the form or from the file part. + assert.Equal(t, "image/png", r.Header.Get("Content-Type")) +} + // TestPostPolicyBucketHandlerKeyExtraction tests that the handler correctly // extracts and normalizes the key from a POST request func TestPostPolicyBucketHandlerKeyExtraction(t *testing.T) {