fix(s3api): sort repeated SigV4 query values (#10031)

* fix(s3api): sort repeated SigV4 query values

Problem
SigV4 canonical query strings must sort repeated parameter values. SeaweedFS preserved the incoming value order, so a correctly signed request with repeated query parameters could fail verification when the request order differed from canonical order.

Root cause
getCanonicalQueryString delegated directly to url.Values.Encode(), which sorts parameter names but preserves each key's value slice order.

Fix
Sort every query value slice before encoding the canonical query string, while still excluding X-Amz-Signature for presigned URLs.

Reproduction
go test ./weed/s3api -run TestGetCanonicalQueryStringSortsRepeatedValues -count=1 failed before the fix with partNumber=2 before partNumber=10.

Co-authored-by: Codex <noreply@openai.com>

* test(s3api): expand canonical query coverage

Co-authored-by: Codex <noreply@openai.com>

---------

Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
7y-9andCodex authored and GitHub committed 2026-06-22 02:00:27 -07:00
1 parent 4f9393889c
commit 7589429b43
2 files changed
+56 -7

No files matched your search

+6 -7
View File
@@ -599,15 +599,14 @@ func extractV4AuthInfoFromQuery(r *http.Request) (*v4AuthInfo, s3err.ErrorCode)
}
func getCanonicalQueryString(r *http.Request, isPresigned bool) string {
var queryToEncode string
if !isPresigned {
queryToEncode = r.URL.Query().Encode()
} else {
queryForCanonical := r.URL.Query()
queryForCanonical := r.URL.Query()
if isPresigned {
queryForCanonical.Del("X-Amz-Signature")
queryToEncode = queryForCanonical.Encode()
}
return queryToEncode
for key := range queryForCanonical {
sort.Strings(queryForCanonical[key])
}
return queryForCanonical.Encode()
}
func checkPresignedRequestExpiry(r *http.Request, t time.Time) s3err.ErrorCode {
+50
View File
@@ -111,6 +111,56 @@ func TestExtractV4AuthInfoFromQueryRejectsEmptySignedHeaderNames(t *testing.T) {
}
}
func TestGetCanonicalQueryString(t *testing.T) {
tests := []struct {
name string
target string
isPresigned bool
want string
}{
{
name: "sorts repeated values",
target: "http://localhost/bucket/key?partNumber=2&partNumber=10&uploadId=z",
want: "partNumber=10&partNumber=2&uploadId=z",
},
{
name: "removes presigned signature",
target: "http://localhost/bucket/key?X-Amz-Date=20260618T000000Z&X-Amz-Signature=dummy&X-Amz-SignedHeaders=host",
isPresigned: true,
want: "X-Amz-Date=20260618T000000Z&X-Amz-SignedHeaders=host",
},
{
name: "sorts mixed single and repeated parameters",
target: "http://localhost/bucket/key?z=last&a=2&bucket=b&a=1",
want: "a=1&a=2&bucket=b&z=last",
},
{
name: "encodes query parameter values",
target: "http://localhost/bucket/key?prefix=photos/2026&marker=a%2Bb",
want: "marker=a%2Bb&prefix=photos%2F2026",
},
{
name: "empty query string",
target: "http://localhost/bucket/key",
want: "",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
req, err := http.NewRequest(http.MethodGet, tt.target, nil)
if err != nil {
t.Fatalf("NewRequest: %v", err)
}
got := getCanonicalQueryString(req, tt.isPresigned)
if got != tt.want {
t.Fatalf("canonical query = %q, want %q", got, tt.want)
}
})
}
}
func TestBuildPathWithForwardedPrefix(t *testing.T) {
tests := []struct {
name string