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.
This commit is contained in:
Chris Lu
2026-04-17 14:55:06 -07:00
committed by GitHub
parent 2da24cc230
commit 0bddc2652e
2 changed files with 219 additions and 11 deletions
+54 -11
View File
@@ -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
@@ -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) {