From 7589429b43022e6da5d85f43fda22149f4bd6806 Mon Sep 17 00:00:00 2001
From: 7y-9 <lianyu.sun@ly.com>
Date: Mon, 22 Jun 2026 17:00:27 +0800
Subject: [PATCH] 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>
---
 weed/s3api/auth_signature_v4.go      | 13 ++++----
 weed/s3api/auth_signature_v4_test.go | 50 ++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/weed/s3api/auth_signature_v4.go b/weed/s3api/auth_signature_v4.go
index 79e19c33a..799962f71 100644
--- a/weed/s3api/auth_signature_v4.go
+++ b/weed/s3api/auth_signature_v4.go
@@ -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 {
diff --git a/weed/s3api/auth_signature_v4_test.go b/weed/s3api/auth_signature_v4_test.go
index a13740a8b..0fc65f3ee 100644
--- a/weed/s3api/auth_signature_v4_test.go
+++ b/weed/s3api/auth_signature_v4_test.go
@@ -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