Skip to content

Conversation

DmytroVyshnevsky1
Copy link
Contributor

@DmytroVyshnevsky1 DmytroVyshnevsky1 commented Sep 2, 2025

Hello!

I found a bug while working with Async AWS , AWS sdk have the same one.

If one of the query params is an array with 10+ elements it causes an error:
"The request signature we calculated does not match the signature you provided".
The root of problem is sorting of query params for canonical string, cause params should be sorted by name AFTER url encoding.
You can read about this here

quote:
" You must also sort the parameters in the canonical query string alphabetically by key name. The sorting occurs after encoding."
This ksort is the reason of problem.

Example URL:

'https://example.com/service?transaction_ids[0]=1111&transaction_ids[1]=1111&transaction_ids[10]=1111&transaction_ids[2]=1111&transaction_ids[3]=1111&transaction_ids[4]=1111&transaction_ids[5]=1111&transaction_ids[6]=1111&transaction_ids[7]=1111&transaction_ids[8]=1111&transaction_ids[9]=1111'

The example of sorting result on current version:

array(11) {
  ["transaction_ids[0]"]=>
  string(4) "1111"
  ["transaction_ids[10]"]=>
  string(4) "1111"
  ["transaction_ids[1]"]=>
  string(4) "1111"
  ["transaction_ids[2]"]=>
  string(4) "1111"
  ["transaction_ids[3]"]=>
  string(4) "1111"
  ["transaction_ids[4]"]=>
  string(4) "1111"
  ["transaction_ids[5]"]=>
  string(4) "1111"
  ["transaction_ids[6]"]=>
  string(4) "1111"
  ["transaction_ids[7]"]=>
  string(4) "1111"
  ["transaction_ids[8]"]=>
  string(4) "1111"
  ["transaction_ids[9]"]=>
  string(4) "1111"
}

and the correct one after this fix:

array(11) {
  ["transaction_ids[0]"]=>
  string(7) "1111"
  ["transaction_ids[1]"]=>
  string(7) "1111"
  ["transaction_ids[10]"]=>
  string(7) "1111"
  ["transaction_ids[2]"]=>
  string(7) "1111"
  ["transaction_ids[3]"]=>
  string(7) "1111"
  ["transaction_ids[4]"]=>
  string(7) "1111"
  ["transaction_ids[5]"]=>
  string(7) "1111"
  ["transaction_ids[6]"]=>
  string(7) "1111"
  ["transaction_ids[7]"]=>
  string(7) "1111"
  ["transaction_ids[8]"]=>
  string(7) "1111"
  ["transaction_ids[9]"]=>
  string(7) "1111"
}

As you can see current sorting put 10 and 1 in wrong places. This not allowed signer to build correct canonical query string.
Also need to clarify that Query parameters are added to the request via:

AsyncAws\Core\Request::setQueryAttribute

@DmytroVyshnevsky1 DmytroVyshnevsky1 force-pushed the fix-query-params-sorting-for-SigV4 branch from 87c2dd2 to b0154ba Compare September 2, 2025 22:54
Copy link
Member

@jderusse jderusse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the investigation end the psycho pull request.
could you please provide a test for this ?

@DmytroVyshnevsky1
Copy link
Contributor Author

DmytroVyshnevsky1 commented Sep 3, 2025

@jderusse
Thanks for the quick review — I can add tests. Also, could you clarify what you meant by “end the psycho”? Was that a typo?

@jderusse
Copy link
Member

jderusse commented Sep 3, 2025

@jderusse Thanks for the quick review — I can add tests. Also, could you clarify what you meant by “end the psycho”? Was that a typo?

uh.. my keyboard replaced pull reaquest by psycho 😅

@DmytroVyshnevsky1
Copy link
Contributor Author

@jderusse
I added the tests,
I think they also will help if someone will try to add proper parsing for query string with arrays

@jderusse
Copy link
Member

jderusse commented Sep 4, 2025

thank you @DmytroVyshnevsky1

@jderusse jderusse merged commit 87fd89e into async-aws:master Sep 4, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants