From b0154ba97fceea7e66dc2c56a8d908c90ae34b5d Mon Sep 17 00:00:00 2001 From: Dmytro Vyshnevskyi Date: Tue, 2 Sep 2025 23:17:51 +0300 Subject: [PATCH 1/6] fix sorting --- src/Core/src/Signer/SignerV4.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/src/Signer/SignerV4.php b/src/Core/src/Signer/SignerV4.php index 602999076..b7da7cc58 100644 --- a/src/Core/src/Signer/SignerV4.php +++ b/src/Core/src/Signer/SignerV4.php @@ -321,7 +321,7 @@ private function buildCanonicalQuery(Request $request): string return ''; } - ksort($query); + uksort($query, static fn($a, $b) => strcmp(rawurlencode($a), rawurlencode($b))); $encodedQuery = []; foreach ($query as $key => $values) { if (!\is_array($values)) { From 92c0a679ec4d775a935655077389e956a26db39a Mon Sep 17 00:00:00 2001 From: Dmytro Vyshnevskyi Date: Wed, 3 Sep 2025 02:05:25 +0300 Subject: [PATCH 2/6] cs fix + change log --- src/Core/CHANGELOG.md | 4 ++++ src/Core/src/Signer/SignerV4.php | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Core/CHANGELOG.md b/src/Core/CHANGELOG.md index 603e36241..d28ef758e 100644 --- a/src/Core/CHANGELOG.md +++ b/src/Core/CHANGELOG.md @@ -2,6 +2,10 @@ ## NOT RELEASED +### Fixed +- SignerV4: sort canonical query parameters **after** URI-encoding (per AWS SigV4). Fixes incorrect ordering for array-style keys (e.g., `transaction_ids[10]` vs `transaction_ids[1]`), which could cause `The request signature we calculated does not match the signature you provided`. See: https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_sigv-create-signed-request.html + + ## 1.27.0 ### Added diff --git a/src/Core/src/Signer/SignerV4.php b/src/Core/src/Signer/SignerV4.php index b7da7cc58..17931dda2 100644 --- a/src/Core/src/Signer/SignerV4.php +++ b/src/Core/src/Signer/SignerV4.php @@ -321,7 +321,7 @@ private function buildCanonicalQuery(Request $request): string return ''; } - uksort($query, static fn($a, $b) => strcmp(rawurlencode($a), rawurlencode($b))); + uksort($query, static fn ($a, $b) => strcmp(rawurlencode($a), rawurlencode($b))); $encodedQuery = []; foreach ($query as $key => $values) { if (!\is_array($values)) { From 80b02250a8863786a2e996cb6585c98f7d0845e6 Mon Sep 17 00:00:00 2001 From: Dmytro Vyshnevskyi Date: Wed, 3 Sep 2025 02:14:31 +0300 Subject: [PATCH 3/6] fix change log --- src/Core/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/CHANGELOG.md b/src/Core/CHANGELOG.md index d28ef758e..1966e9cf9 100644 --- a/src/Core/CHANGELOG.md +++ b/src/Core/CHANGELOG.md @@ -3,8 +3,8 @@ ## NOT RELEASED ### Fixed -- SignerV4: sort canonical query parameters **after** URI-encoding (per AWS SigV4). Fixes incorrect ordering for array-style keys (e.g., `transaction_ids[10]` vs `transaction_ids[1]`), which could cause `The request signature we calculated does not match the signature you provided`. See: https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_sigv-create-signed-request.html +- SignerV4: fix sort of query parameters to build correct canoncal query string ## 1.27.0 From e085e8f149560123abc1a0e09a278133cb99a0b8 Mon Sep 17 00:00:00 2001 From: Dmytro Vyshnevskyi Date: Wed, 3 Sep 2025 02:26:20 +0300 Subject: [PATCH 4/6] fix for psalm --- src/Core/src/Signer/SignerV4.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Core/src/Signer/SignerV4.php b/src/Core/src/Signer/SignerV4.php index 17931dda2..2d371de2a 100644 --- a/src/Core/src/Signer/SignerV4.php +++ b/src/Core/src/Signer/SignerV4.php @@ -321,7 +321,9 @@ private function buildCanonicalQuery(Request $request): string return ''; } - uksort($query, static fn ($a, $b) => strcmp(rawurlencode($a), rawurlencode($b))); + uksort($query, static function (string $a, string $b): int { + return strcmp(rawurlencode($a), rawurlencode($b)); + }); $encodedQuery = []; foreach ($query as $key => $values) { if (!\is_array($values)) { From 93305003029b14dea23cf27fcf67aca6a4c36b73 Mon Sep 17 00:00:00 2001 From: Dmytro Vyshnevskyi Date: Thu, 4 Sep 2025 03:58:48 +0300 Subject: [PATCH 5/6] add tests --- src/Core/tests/Unit/Signer/SignerV4Test.php | 59 ++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/src/Core/tests/Unit/Signer/SignerV4Test.php b/src/Core/tests/Unit/Signer/SignerV4Test.php index 495dfb70b..57b1568f4 100644 --- a/src/Core/tests/Unit/Signer/SignerV4Test.php +++ b/src/Core/tests/Unit/Signer/SignerV4Test.php @@ -140,7 +140,58 @@ public function provideRequests() ]; } - private function parseRequest(string $req): Request + /** + * @dataProvider provideRequestsWithQueryParams + */ + public function testSignsRequestsWithArrayInQueryParams($rawRequestWithoutQuery, $rawExpected, $queryParams) + { + $request = $this->parseRequest($rawRequestWithoutQuery, $queryParams); + + $signer = new SignerV4('host', 'us-east-1'); + $context = new RequestContext(['currentDate' => new \DateTimeImmutable('20110909T233600Z')]); + $credentials = new Credentials('AKIDEXAMPLE', 'wJalrXUtnFEMI/K7MDENG+bPxRfiCYEXAMPLEKEY'); + + $signer->sign($request, $credentials, $context); + + $expected = $this->parseRequest($rawExpected, $queryParams); + + self::assertEquals($expected, $request); + } + + private function provideRequestsWithQueryParams() + { + return [ + // GET Case with array in query params + [ + "GET / HTTP/1.1\r\nHost: host.foo.com:443\r\n\r\n", + "GET / HTTP/1.1\r\nHost: host.foo.com:443\r\nX-Amz-Date: 20110909T233600Z\r\nAuthorization: AWS4-HMAC-SHA256 Credential=AKIDEXAMPLE/20110909/us-east-1/host/aws4_request, SignedHeaders=host;x-amz-date, Signature=7bd9c6fed0473be1f7ea96ff34c7d79e71a92932054a24b4210b3a56e4ab46d3\r\n\r\n", + [ + 'foos[0]' => '834127', + 'foos[1]' => '59', + 'foos[2]' => '90123', + 'foos[3]' => '4708', + 'foos[4]' => '120001', + 'foos[5]' => '333', + 'foos[6]' => '78005', + 'foos[7]' => '2', + 'foos[8]' => 'string value', + 'foos[9]' => '40617', + 'foos[10]' => '715', + ], + ], + // GET Simple case with query params (copy of one of the cases above from testSignsRequests) + [ + "GET / HTTP/1.1\r\nHost: host.foo.com:443\r\n\r\n", + "GET / HTTP/1.1\r\nHost: host.foo.com:443\r\nX-Amz-Date: 20110909T233600Z\r\nAuthorization: AWS4-HMAC-SHA256 Credential=AKIDEXAMPLE/20110909/us-east-1/host/aws4_request, SignedHeaders=host;x-amz-date, Signature=1c3274381ae12d8817336268d7da17672bd57e7348e39b7b9c567280f73742af\r\n\r\n", + [ + 'a' => 'foo', + 'b' => 'foo', + ], + ], + ]; + } + + private function parseRequest(string $req, array $queryParams = []): Request { $lines = explode("\r\n", $req); [$method, $path] = explode(' ', array_shift($lines)); @@ -156,6 +207,12 @@ private function parseRequest(string $req): Request $req = new Request($method, '/', [], $headers, StringStream::create(implode("\n", $lines))); $req->setEndpoint('https://' . $headers['Host'] . $path); + + // currently the library cannot properly parse query params if they contain arrays, so we need to set them manually + foreach ($queryParams as $k => $v) { + $req->setQueryAttribute($k, $v); + } + // Ensure that the memoized property is filled, so that comparison works consistently. $req->getEndpoint(); From 715f61bfbacdeca636369d041afef201a9fda6c8 Mon Sep 17 00:00:00 2001 From: Dmytro Vyshnevskyi Date: Thu, 4 Sep 2025 04:20:38 +0300 Subject: [PATCH 6/6] fix deprecations --- src/Core/phpunit.xml.dist | 19 ++++++------------- src/Core/tests/Unit/Signer/SignerV4Test.php | 4 ++-- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/Core/phpunit.xml.dist b/src/Core/phpunit.xml.dist index 9894ce353..3185bb954 100644 --- a/src/Core/phpunit.xml.dist +++ b/src/Core/phpunit.xml.dist @@ -1,17 +1,5 @@ - - - - ./src - - + @@ -20,4 +8,9 @@ ./tests/ + + + ./src + + diff --git a/src/Core/tests/Unit/Signer/SignerV4Test.php b/src/Core/tests/Unit/Signer/SignerV4Test.php index 57b1568f4..18acb9353 100644 --- a/src/Core/tests/Unit/Signer/SignerV4Test.php +++ b/src/Core/tests/Unit/Signer/SignerV4Test.php @@ -78,7 +78,7 @@ public function testSignsRequests($rawRequest, $rawExpected) self::assertEquals($expected, $request); } - public function provideRequests() + public static function provideRequests() { return [ // POST headers should be signed. @@ -158,7 +158,7 @@ public function testSignsRequestsWithArrayInQueryParams($rawRequestWithoutQuery, self::assertEquals($expected, $request); } - private function provideRequestsWithQueryParams() + public static function provideRequestsWithQueryParams() { return [ // GET Case with array in query params