Repository navigation
Always set the Date header to the current date - #399
Merged
Merged
Conversation
This fixes a bug where if you resend a request and resign it without changing the date, you can get an auth error that your signature is too old if the retry loops fall outside the auth window (~15 minutes). What was happening is that we were trying a failed S3 request, but because we weren't re-setting the Date header, eventually the retried request would have a signature that was too old.
Part of the reason this change looks more complicated than you would expect is because we use the sigv4 test suite (http://docs.aws.amazon.com/general/latest/gr/signature-v4-test-suite.html) to test our implementation. However, these tests use a Date header whereas previous versions used an X-Amz-Date. We can possibly investigate always using Date instead of X-Amz-Date, but we'd need to make sure we aren't introducing any regressions.
Contributor
|
Looks good. Looks like you appropriately deleted and then re-added all of the appropriate date headers. 🚢 |
Contributor
There was a problem hiding this comment.
I don't think this line could ever be True because the same check is performed on line 346.
Member
Author
There was a problem hiding this comment.
Good catch, I'll fix this.
Contributor
|
LGTM |
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.
This fixes a bug where if you resend a request and resign
it without changing the date, you can get an auth error
that your signature is too old if the retry loops fall outside
the auth window (~15 minutes).
What was happening is that we were retrying a failed S3 request,
but because we weren't re-setting the Date header, eventually
the retried request would have a signature that was too old.
I also had to update this for sigv4, but it's a little more complicated.
Part of the reason this change looks more complicated than
you would expect is because we use the sigv4 test suite
(http://docs.aws.amazon.com/general/latest/gr/signature-v4-test-suite.html)
to test our implementation. However, these tests use a Date header
whereas previous versions used an X-Amz-Date.
We can possibly investigate always using Date instead of X-Amz-Date,
but we'd need to make sure we aren't introducing any regressions.
cc @danielgtaylor @kyleknap