fix(object_store): Include Content-MD5 header for S3 DeleteObjects #5415
Conversation
This file contains 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
S3 API [specification](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObjects.html) requires the presence of this header for all `DeleteObjects` requests to general purpose buckets: > The Content-MD5 request header is required for all Multi-Object Delete requests Some platform, such as MinIO, enforce this requirement, failing requests that don't include the header.
tustvold
reviewed
Feb 21, 2024
object_store/Cargo.toml
Outdated
| @@ -54,6 +54,7 @@ reqwest = { version = "0.11", default-features = false, features = ["rustls-tls- | |||
| ring = { version = "0.17", default-features = false, features = ["std"], optional = true } | |||
| rustls-pemfile = { version = "2.0", default-features = false, features = ["std"], optional = true } | |||
| tokio = { version = "1.25.0", features = ["sync", "macros", "rt", "time", "io-util"] } | |||
| md5 = { version = "0.7.0", default-features = false, features = ["std"], optional = true } | |||
There was a problem hiding this comment.
Is this a new dependency?
https://crates.io/crates/md-5 is perhaps better maintained
There was a problem hiding this comment.
Yes, new dependency, I couldn't find any other existing dependencies that supports md5. Good catch, I'll switch to md-5
md-5 seems better maintained.
tustvold
approved these changes
Feb 21, 2024
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.
S3 API specification requires the presence of this header for all
DeleteObjectsrequests to general purpose buckets:Some platforms, such as MinIO, enforce this requirement, rejecting requests that don't include the header.
I have successfully tested the change locally against MinIO.
What changes are included in this PR?
Content-MD5header unconditionally for allDeleteObjectsrequestsNo breaking changes, platforms not interested in the header will ignore it.
Closes #5414