OpenFeign / feign

Feign makes writing java http clients easier
Apache License 2.0
9.43k stars 1.92k forks source link

The "Accept-Encoding" header is added twice when the " compression" option is enabled #1721

Open pastrehe opened 2 years ago

pastrehe commented 2 years ago

feign-bom.version 11.9.1 spring-boot 2.7.2

When feign.compression.response.enabled is set to true in configuration, feign adds "Accept-Encoding" header(s) when sending request. Currently, two header fields are added (Accept-Encoding: gzip and Accept-Encoding: deflate). According to rfc7230 section 3.2.2 A sender MUST NOT generate multiple header fields with the same field name in a message unless either the entire field value for that header field is defined as a comma-separated list [i.e., #(values)] or the header field is a well-known exception Expected behaivour is to have one header field with the name "Accept-Encoding" and value "gzip, deflate". ATM, having 2 "Accept-Encoding" headers causing remote side to "flatten" them and pick only one value randomly ("gzip" or "deflate")

vitalijr2 commented 2 years ago

As far as I undertand you use httpclient. Could you confirm, please?

I had made a test project with Spring 2.7.2 and Spring Cloud's OpenFeing and met the same issue:

DEBUG i.g.r.t.Monobank - [Monobank#getRates] ---> GET https://api.monobank.ua/bank/currency HTTP/1.1
DEBUG i.g.r.t.Monobank - [Monobank#getRates] Accept-Encoding: gzip
DEBUG i.g.r.t.Monobank - [Monobank#getRates] Accept-Encoding: deflate
DEBUG i.g.r.t.Monobank - [Monobank#getRates] Content-Type: application/json
DEBUG i.g.r.t.Monobank - [Monobank#getRates] ---> END HTTP (0-byte body)

I suppose that it is Apache HTTP Client's issue and does not relates with Spring or Feing.

Update:

I have made another simple feign application with httpclient:

vitalijr2 commented 2 years ago

This loop adds another header for each its value https://github.com/OpenFeign/feign/blob/307a322c96e28339d90fef31e962f107588e6e2b/core/src/main/java/feign/Request.java#L247

muzinian commented 8 months ago

Hi,this fix may cause add Content-Length twice if Content-Encoding not be set. https://github.com/OpenFeign/feign/blob/7336c3e548612f90f7c3d05e84f3c8793dcef2a4/core/src/main/java/feign/Client.java#L184-L196

izdt commented 6 months ago

Hi,this fix may cause add Content-Length twice if Content-Encoding not be set.

https://github.com/OpenFeign/feign/blob/7336c3e548612f90f7c3d05e84f3c8793dcef2a4/core/src/main/java/feign/Client.java#L184-L196

Actually "Content-Length" will not be added because this key is in the restrictedHeaderSet of HttpURLConnection, @see sun.net.www.protocol.http.HttpURLConnection.isRestrictedHeader. But "else" should be added to the code.

muzinian commented 6 months ago

Hi,this fix may cause add Content-Length twice if Content-Encoding not be set. https://github.com/OpenFeign/feign/blob/7336c3e548612f90f7c3d05e84f3c8793dcef2a4/core/src/main/java/feign/Client.java#L184-L196

Actually "Content-Length" will not be added because this key is in the restrictedHeaderSet of HttpURLConnection, @see sun.net.www.protocol.http.HttpURLConnection.isRestrictedHeader. But "else" should be added to the code.

In our case, We may set System.setProperty("sun.net.http.allowRestrictedHeaders", "true") after RestTemplate be constructed.So sun.net.www.protocol.http.HttpURLConnection.isRestrictedHeaderwill always return false.And yes,an else can fix this.