Skip to content

aws_v4_signature bugfix#645

Merged
hadley merged 11 commits into
r-lib:mainfrom
jeffreyzuber:aws_v4_sig_bugfix
Jan 24, 2025
Merged

aws_v4_signature bugfix#645
hadley merged 11 commits into
r-lib:mainfrom
jeffreyzuber:aws_v4_sig_bugfix

Conversation

@jeffreyzuber

Copy link
Copy Markdown
Contributor

Fixed bug where 'CanonicalQueryString' was used as an argument to query-build before being defined, instead of 'sorted_query'. Before, using the function req_auth_aws_v4 would result in the error 'object 'CanonicalQueryString' not found'.

@hadley

hadley commented Jan 21, 2025

Copy link
Copy Markdown
Member

Oops 😬 Would you mind adding a test too?

@jeffreyzuber

Copy link
Copy Markdown
Contributor Author

I thought about it, but couldn't think of a way to add a test without hard-coding some sort of AWS authentication tokens. The best that I could come up with is to test the output of aws_v4_signature with a fixed set of arguments, including the timestamp, but that wouldn't test the actual authentication with AWS. Would that be good enough?

@hadley

hadley commented Jan 21, 2025

Copy link
Copy Markdown
Member

Yeah, I think that would be good enough, given that the previous code wouldn't even run.

@jeffreyzuber

Copy link
Copy Markdown
Contributor Author

Added the requested test of aws_v4_signature

@hadley

hadley commented Jan 23, 2025

Copy link
Copy Markdown
Member

Thanks! I switched your test to use a snapshot test, since the values aren't so important here and just clutter up the test file.

@hadley hadley merged commit 97d2944 into r-lib:main Jan 24, 2025
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