Repository navigation
Conversation
s3_calculate_signature_v4_with_time carried on when an allocation for the request's canonical form failed: - No memory for the canonical headers: it signed with empty headers. - No memory for the canonical query: it signed with an empty query, while the URL still carried the real one. S3 can only reject such a signature, so the failure shows up as a 403 that says nothing about memory. Both now return no signature. s3_build_auth_header_ex passes that on as NULL, and every caller already treats a NULL header as a failure. Test: s3_backup_test gains test_signer_out_of_memory, which fails the driver's Nth allocation through macros in the test, the way http_init_failure_test does. Red before: each case returned a signature. Green after. A control call with memory available must still sign. write-code: s3_backup_test under ASan+UBSan and TSan, as upstream's workflow runs it, red on master, green here; full RetroArch build not run. All nine callers' NULL checks were read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
s3_canonicalize_query_string returned the query as given, with a comment listing what a full implementation would do. Every request the driver signs today already has its parameters in order, so this changes no request: partNumber sorts before uploadId, and the rest have one parameter. A new request with two parameters out of order would get a signature S3 rejects, with only a 403 to show for it. It now splits on '&', sorts and re-joins. The comparator takes the names on their own: on the whole "name=value" string, "a1=2" would sort before "a=1", since '1' is below '='. URL encoding stays with the callers, as they already do it; the comment names the encoder to use. Test: s3_backup_test gains test_query_order. Red before: every out-of-order case kept its order. Green after. Mutation, red: comparing whole strings fails the "a1" and "a-b" cases. Failing each of the three allocations returns NULL with no leak under ASan. write-code: s3_backup_test under ASan+UBSan and TSan, red on the previous commit, green here, mutation above; full RetroArch build not run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
milnet01
added a commit
to milnet01/RetroArch
that referenced
this pull request
Oct 31, 2026
…ning and query sort) Also records libretro#19954's CI state, and the readability lane return for libretro#19956. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two small fixes to S3 request signing, one commit each, so either can be taken alone.
1. Sign nothing when out of memory. This is a live defect, though only on a failed allocation.
s3_calculate_signature_v4_with_timecarried on when it could not allocate the request's canonical form:S3 can only reject such a signature, so the failure shows up as a 403 that says nothing about memory. Both now return no signature.
s3_build_auth_header_expasses that on as NULL, and every caller already treats a NULL header as a failure.2. Sort query parameters as SigV4 requires. This fixes no failure today, and changes no request: every query the driver signs is already in order.
s3_canonicalize_query_stringreturned the query as given, with a comment listing what a full implementation would do, so the next request with two parameters out of order would get a 403 with nothing to explain it. It now splits, sorts and re-joins. Names are compared on their own: on the wholename=valuestring,a1=2would sort beforea=1, since1is below=. Encoding stays with the callers, and the comment names the encoder to use. If you would rather not carry this until something needs it, the first commit stands without it.Testing:
samples/network/s3_backupfails the driver's Nth allocation through macros in the test, the wayhttp_init_failure_testdoes.test_signer_out_of_memory: on master, both cases above return a signature. With the change, neither does, and a control call with memory available still signs.test_query_order: on master, every out-of-order case keeps its order. With the change, all sort. Comparing whole strings instead fails thea1anda-bcases. Failing each of the canonicalizer's three allocations returns NULL with no leak; removing the cleanup fails under LeakSanitizer.The sample passes under ASan with UBSan and under TSan, as
Linux-samples-network.ymlruns it.s3.calso shows no declaration-after-statement under-std=c89. I did not run a full RetroArch build or try this against a real S3 server.🤖 Generated with Claude Code