-
Notifications
You must be signed in to change notification settings - Fork 875
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
Always explicitly disable gzip
automatic decompression on reqwest client used by object_store
#6843
Always explicitly disable gzip
automatic decompression on reqwest client used by object_store
#6843
Conversation
@@ -671,6 +671,10 @@ impl ClientOptions { | |||
builder = builder.danger_accept_invalid_certs(true) | |||
} | |||
|
|||
// Reqwest will remove the `Content-Length` header if it is configured to | |||
// transparently decompress the body via the non-default `gzip` feature. | |||
builder = builder.no_gzip(); |
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.
does this mean that gzipped data content will be left gzipped?
So if I request a resource that the server gzip's in response, that the result I get from ObjectStore::get
would also be gzipped 🤔
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.
Yes, that is correct - sorry I could have made this clearer.
All this affects is what happens when the response has the header Content-Encoding: gzip
, which HTTP servers will usually only do when the request has the header Accept-Encoding: gzip
. If that is the case, then reqwest will transparently decode the body as a gzip stream and remove the Content-Length
header (if the gzip
feature is enabled - this no_gzip
function explicitly disables that behavior even if the feature is)
For object store APIs, it will just return the bytes of the object as they are (including objects that are gzipped).
Which issue does this PR close?
Closes #6842
Rationale for this change
Fixes an issue where enabling a non-default feature (
gzip
) forreqwest
would causeobject_store
to stop working if using the HTTP store against an HTTP server that supports gzip response compression.What changes are included in this PR?
Call the
no_gzip
method on the reqest ClientBuilder to ensure that even if thegzip
feature is enabled, theobject_store
client will not use the transparent decompression logic.I considered making this an option instead of always setting it, but since this is such a frustrating footgun to encounter and debug, I think its better to always set it unless there is a compelling reason not to.
Are there any user-facing changes?
My understanding is that most/all users interacting with object stores do not want the
gzip
compression logic (since none of the major cloud object store providers support it), so this change should not be breaking.