Skip to content
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

Only pass SSE headers for multipart upload requests #81

Merged

Conversation

mdedetrich
Copy link
Contributor

This PR fixes akka/alpakka#2906, Alpakka created a regression whereby their latest Alpakka BSL release (4.0.0) has multipart upload broken.

According to S3 docs (see https://docs.aws.amazon.com/AmazonS3/latest/API/API_CompleteMultipartUpload.html#API_CompleteMultipartUpload_Examples), you are not allowed to have any other headers that is not SSE related.

I tested this on my companies S3 account and I can confirm that it passes.

@mdedetrich mdedetrich force-pushed the only-pass-sse-headers-for-multipart-upload branch from 73ca7b6 to a071e8b Compare April 19, 2023 01:11
@mdedetrich mdedetrich added the bug Something isn't working label Apr 19, 2023
@mdedetrich
Copy link
Contributor Author

I have added this PR onto the 1.0.0 project because currently the Pekko S3 client is broken without this fix.

.withHeader(sseCAlgorithmHeader, new EqualToPattern(sseCAlgorithmHeaderValue))
.withHeader(sseCKeyHeader, new EqualToPattern(sseCKeyHeaderValue))
.withHeader(sseCKeyHeaderMd5, new EqualToPattern(sseCKeyHeaderMd5Value))
.withoutHeader(requestPayerHeader))
Copy link
Member

Choose a reason for hiding this comment

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

new EqualToPattern(...) would looks better with EqualToPattern(...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just followed the same style that is done in the rest of the test

Copy link
Member

@He-Pin He-Pin left a comment

Choose a reason for hiding this comment

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

lgtm

@He-Pin He-Pin merged commit b9264dc into apache:main Apr 19, 2023
46 of 50 checks passed
@mdedetrich mdedetrich deleted the only-pass-sse-headers-for-multipart-upload branch May 11, 2023 09:22
@pjfanning pjfanning added the release note should be mentioned in release notes label Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working release note should be mentioned in release notes
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

AWS S3: uploadMultipart* with S3Headers fails
3 participants