-
Notifications
You must be signed in to change notification settings - Fork 654
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
MIGRATION ISSUE: s3: Can no longer use Accept-Encoding: gzip on GetObject #2848
Comments
With the help of @darthShadow we came up with this workaround diff --git a/backend/s3/s3.go b/backend/s3/s3.go
index eda36a259..ef7a4d740 100644
--- a/backend/s3/s3.go
+++ b/backend/s3/s3.go
@@ -5862,6 +5862,14 @@ func (o *Object) downloadFromURL(ctx context.Context, bucketPath string, options
return resp.Body, err
}
+// middleware to remove the gzip encoding
+func removeDisableGzip() func(*middleware.Stack) error {
+ return func(stack *middleware.Stack) error {
+ _, err := stack.Finalize.Remove("DisableAcceptEncodingGzip")
+ return err
+ }
+}
+
// Open an object for read
func (o *Object) Open(ctx context.Context, options ...fs.OpenOption) (in io.ReadCloser, err error) {
bucket, bucketPath := o.split()
@@ -5898,7 +5906,7 @@ func (o *Object) Open(ctx context.Context, options ...fs.OpenOption) (in io.Read
// Override the automatic decompression in the transport to
// download compressed files as-is
if o.fs.opt.UseAcceptEncodingGzip.Value {
- APIOptions = append(APIOptions, smithyhttp.AddHeaderValue("Accept-Encoding", "gzip"))
+ APIOptions = append(APIOptions, removeDisableGzip(), smithyhttp.AddHeaderValue("Accept-Encoding", "gzip"))
}
for _, option := range options { This assumes that the |
Hi @ncw, Thanks for reaching out. I had to dig around to get some context about this change, but this seems like an intentional change that was introduced because the Go standard library's HTTP client (which the Go SDK uses) will auto decompress the gzip file, and will cause issues with checksum validations. I tried to corroborate this by writing my own test code, but was not seeing any checksum validation errors when gzip accept encoding was present on an object with checksum on it. In some clients there were config options introduced to disable this middleware (or rather enable gzip accept-encoding): ddb := dynamodb.NewFromConfig(cfg, func(options *dynamodb.Options) {
options.EnableAcceptEncodingGzip = true
}) The way you are disabling this middleware on the S3 client looks right to me. To sum up, this is an intentional change. Let us know if you have additional questions / concerns. Thanks, |
Thanks for your help and investigations @RanVaknin Given that we've found an acceptable workaround I think we can close this issue now, and if anyone in the future has the same problem then hopefully they will find this issue. |
This issue is now closed. Comments on closed issues are hard for our team to see. |
@RanVaknin, I think we should reconsider having the client default to We should either not use it in the signature or be an opt-in option. Removing the middleware What do you think? |
Pre-Migration Checklist
Go Version Used
go1.23.2
Describe the Migration Issue
In SDK v1 it was possible to specify
Accept-Encoding: gzip
on a GET of an s3 object.This is necessary because without it both Cloudflare R2 and the S3 compatible interface to Google Cloud Storage will transparently decompress files stored with
Content-Encoding: gzip
and withoutCache-Control: no-transform
and rclone wants to be able to download the files as uploaded otherwise the MD5SUM in the ETags don't match.This is explained in the google cloud storage docs. Cloudflare R2 appears to work the same way
but I can't find the docs for that.as this pending commit shows.The SDK v1 let users add
Accept-Encoding: gzip
however the SDK v2 seems to overwrite it withAccept-Encoding: identity
whatever I try.This had lead to this rclone issue rclone/rclone#8137
Code Comparison
V1 code snippet to add
Accept-Encoding: gzip
and the converted to the SDKv2 code
Observed Differences/Errors
With SDK v1 we see
Accept-Encoding: gzip
in the REQUESTAnd with SDKv2 - Note the Accept-Encoding has been turned into
identity
in the REQUESTAdditional Context
This problem does not occur with AWS S3. However it could be worked around with the SDK v1 and can no longer be worked around with the SDK v2 so I think it counts as a regression.
The text was updated successfully, but these errors were encountered: