Skip to content

Misleading Content-Encoding Header in Default-Setup for Request Compression #1048

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

Closed
AndreTeigler opened this issue Aug 16, 2024 · 3 comments · Fixed by #1071
Closed

Misleading Content-Encoding Header in Default-Setup for Request Compression #1048

AndreTeigler opened this issue Aug 16, 2024 · 3 comments · Fixed by #1071
Assignees
Labels
bug Something isn't working
Milestone

Comments

@AndreTeigler
Copy link
Contributor

AndreTeigler commented Aug 16, 2024

Describe the bug
When the openfeign request compression is used the the FeignContentGzipEncodingInterceptor adds two values to the header:

addHeader(template, HttpEncoding.CONTENT_ENCODING_HEADER, HttpEncoding.GZIP_ENCODING, HttpEncoding.DEFLATE_ENCODING);

The compression itself is dependent on the used client. The feign default client (https://github.com/OpenFeign/feign/blob/master/core/src/main/java/feign/Client.java) can either use gzip compression or deflate compression. If both headers are present it will currently choose gzip but the headers remain unchanged.

According to the Content-Encoding documentation (https://datatracker.ietf.org/doc/html/rfc2616#section-14.11) the header indicates that both compressions have been applied. The default client only applies one so the receiving server (if it handles the Content-Encoding header correctly) will first try to decompress with defalte which fails.

Sample
Can be reproduced by using a simple spring boot app using spring-cloud-starter-openfeign and setting up a client which will sent a body with request compression. The headers will contain Content-Enconding: gzip, deflate but the compressed body will only be gzip data.

@OlgaMaciaszek
Copy link
Collaborator

Thanks @AndreTeigler, makes sense; we can't know which client and which setup will be used in the app configuration, however, we should allow the users to apply a more fine-grained configuration via properties.

@OlgaMaciaszek OlgaMaciaszek added enhancement New feature or request and removed waiting-for-triage labels Sep 4, 2024
@OlgaMaciaszek OlgaMaciaszek self-assigned this Sep 4, 2024
@OlgaMaciaszek OlgaMaciaszek moved this to In Progress in 2024.0.0-M2 Sep 4, 2024
@OlgaMaciaszek OlgaMaciaszek added bug Something isn't working and removed enhancement New feature or request labels Sep 4, 2024
@OlgaMaciaszek OlgaMaciaszek moved this to In Progress in 2023.0.4 Sep 4, 2024
@OlgaMaciaszek OlgaMaciaszek added this to the 4.1.4 milestone Sep 4, 2024
@AndreTeigler
Copy link
Contributor Author

AndreTeigler commented Sep 4, 2024

@OlgaMaciaszek
I could provide a pull request over the next weekend by just adding another configuration property for the encoding header. It defaults to gzip, deflate to keep backwards compatibility but would allow to set the header to the appropriate value for the setup in use.

@OlgaMaciaszek
Copy link
Collaborator

Thanks @AndreTeigler. Sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Status: Done
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants