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

Fix gzhttp transport response. #977

Closed
wants to merge 1 commit into from

Conversation

DeyiXu
Copy link

@DeyiXu DeyiXu commented Jul 1, 2024

  1. Fix response header Content-Encoding: gzip,zstd but not decompress
  2. Server side Content-Encoding is not controlled.
    Fix the decompression problem when Content-Encoding appears gzip and zstd at the same time

1. Fix response header Content-Encoding: gzip,zstd but not decompress
2. Server side Content-Encoding is not controlled.
Fix the decompression problem when Content-Encoding appears gzip and zstd at the same time
@klauspost
Copy link
Owner

Could you please explain a bit deeper. Perhaps with an example.

Note that we do not decompress if we did not ourselves add the "Accept-Encoding". That is by design.

Without more context I think you are breaking a lot of existing use of this package.

@DeyiXu
Copy link
Author

DeyiXu commented Jul 1, 2024

I am implementing a caching middleware, and during use I found that when saving files locally, gzip data that could not be decompressed appeared.

If the data returned through the gzhttp transport middleware needs to be processed and decompressed gzip a second time.

If it can't solve the data perfectly, then what's the point of this middleware's existence?

@DeyiXu
Copy link
Author

DeyiXu commented Jul 1, 2024

When accessing an uncertain backend HTTP Server, there is no guarantee that the backend server will fully comply with HTTP standards. There may be some data that does not meet the standards. At this time, the data needs to be processed.

The PR I submitted is to solve this problem.

If there is a problem with my code, please reject my PR and I will continue to improve it.

@klauspost
Copy link
Owner

There can be plenty of reasons.

A) If "Accept-Encoding" is added upstream, we do not touch the request or the response.
B) If the server returns with "Content-Encoding" without you asking for it, then the server has an issue.

Since you don't really provide any information, I am assuming it is B). I can add an option that will allow for that.

@DeyiXu
Copy link
Author

DeyiXu commented Jul 1, 2024

Thanks.

I think the option is feasible.

@DeyiXu
Copy link
Author

DeyiXu commented Jul 1, 2024

Looking forward to your code.

klauspost added a commit that referenced this pull request Jul 1, 2024
TransportAlwaysDecompress will always decompress the response, regardless of whether we requested it or not.

Default is false, which will pass compressed data through if we did not request compression.

Replaces #977

Bonus: Remove code for Go 1.19 and older.
@klauspost
Copy link
Owner

See #978

klauspost added a commit that referenced this pull request Jul 1, 2024
* Add TransportAlwaysDecompress option.

TransportAlwaysDecompress will always decompress the response, regardless of whether we requested it or not.

Default is false, which will pass compressed data through if we did not request compression.

Replaces #977

Bonus: Remove code for Go 1.19 and older.
@klauspost klauspost closed this Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants