-
Notifications
You must be signed in to change notification settings - Fork 584
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(s3-request-presigner): skip hoisting SSE headers #1701
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, I think this PR is great 🙂 Thank you so much for the fix!
Codecov Report
@@ Coverage Diff @@
## master #1701 +/- ##
==========================================
- Coverage 79.77% 79.77% -0.01%
==========================================
Files 325 329 +4
Lines 12087 12578 +491
Branches 2553 2672 +119
==========================================
+ Hits 9643 10034 +391
- Misses 2444 2544 +100
Continue to review full report at Codecov.
|
Co-authored-by: Attila Večerek <[email protected]>
31fcb0f
to
94e7b4c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread. |
Issue #, if available:
fix: #1576
Description of changes:
When
signature-v4
generate a presigned url, it will try to hoist the headers to query strings, so these headers will exist in the url, hense easy to use. However, S3 requres server-side encryption headers to be signed in headers due to S3 limitation. So this change solve the issue by 2 changes:signature-v4
presign configunhoistableHeaders
. The presigner won't hoist the given headers to query before presign it.s3-request-presigner
, it detects all the server-side encryption headers, and supply them to theunhoistableHeaders
config of the presigner.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.