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

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

Open
pastrehe opened this issue Aug 9, 2022 · 5 comments
Labels
feedback provided Feedback has been provided to the author question General usage or 'how-to' questions waiting for feedback Issues waiting for a response from either to the author or other maintainers

Comments

@pastrehe
Copy link

pastrehe commented Aug 9, 2022

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
Copy link
Collaborator

vitalijr2 commented Aug 11, 2022

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:

  • if there are any annotated headers the client doesn't send "Accept-Encoding"
  • if I add them manually like that @Headers({"Accept: application/json", "Accept-Encoding: gzip", "Accept-Encoding: deflate"}) it sends the correct request:
[DEBUG] feign.Logger - [Monobank#getRates] ---> GET https://api.monobank.ua/bank/currency HTTP/1.1
[DEBUG] feign.Logger - [Monobank#getRates] Accept: application/json
[DEBUG] feign.Logger - [Monobank#getRates] Accept-Encoding: gzip, deflate
[DEBUG] feign.Logger - [Monobank#getRates] ---> END HTTP (0-byte body)

@vitalijr2
Copy link
Collaborator

This loop adds another header for each its value

builder.append(field).append(": ").append(value).append('\n');

@kdavisk6 kdavisk6 added question General usage or 'how-to' questions waiting for feedback Issues waiting for a response from either to the author or other maintainers feedback provided Feedback has been provided to the author labels Oct 7, 2022
@izdt izdt mentioned this issue Jul 26, 2023
izdt added a commit to izdt/feign that referenced this issue Jul 26, 2023
velo pushed a commit that referenced this issue Jul 28, 2023
* Fix issuse #1721 Accept-Encoding header is added twice

* format core code
@muzinian
Copy link

muzinian commented Jan 5, 2024

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

if (field.equals(CONTENT_LENGTH)) {
if (!gzipEncodedRequest && !deflateEncodedRequest) {
contentLength = Integer.valueOf(value);
connection.addRequestProperty(field, value);
}
}
// Avoid add "Accept-encoding" twice or more when "compression" option is enabled
if (field.equals(ACCEPT_ENCODING)) {
connection.addRequestProperty(field, String.join(", ", request.headers().get(field)));
break;
} else {
connection.addRequestProperty(field, value);
}

izdt added a commit to izdt/feign that referenced this issue Jan 25, 2024
Fix Isuue OpenFeign#1721 Content-Length may be added twice
@izdt
Copy link
Contributor

izdt commented Feb 23, 2024

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

if (field.equals(CONTENT_LENGTH)) {
if (!gzipEncodedRequest && !deflateEncodedRequest) {
contentLength = Integer.valueOf(value);
connection.addRequestProperty(field, value);
}
}
// Avoid add "Accept-encoding" twice or more when "compression" option is enabled
if (field.equals(ACCEPT_ENCODING)) {
connection.addRequestProperty(field, String.join(", ", request.headers().get(field)));
break;
} else {
connection.addRequestProperty(field, value);
}

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
Copy link

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

if (field.equals(CONTENT_LENGTH)) {
if (!gzipEncodedRequest && !deflateEncodedRequest) {
contentLength = Integer.valueOf(value);
connection.addRequestProperty(field, value);
}
}
// Avoid add "Accept-encoding" twice or more when "compression" option is enabled
if (field.equals(ACCEPT_ENCODING)) {
connection.addRequestProperty(field, String.join(", ", request.headers().get(field)));
break;
} else {
connection.addRequestProperty(field, value);
}

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.

velo pushed a commit that referenced this issue Jun 3, 2024
* Update Client.java

Fix Isuue #1721 Content-Length may be added twice

* format code

* Add ClientTest unit tests

* update license header

* Update comment of ClientTest
velo pushed a commit that referenced this issue Oct 7, 2024
* Fix issuse #1721 Accept-Encoding header is added twice

* format core code
velo pushed a commit that referenced this issue Oct 7, 2024
* Update Client.java

Fix Isuue #1721 Content-Length may be added twice

* format code

* Add ClientTest unit tests

* update license header

* Update comment of ClientTest
velo pushed a commit that referenced this issue Oct 8, 2024
* Fix issuse #1721 Accept-Encoding header is added twice

* format core code
velo pushed a commit that referenced this issue Oct 8, 2024
* Update Client.java

Fix Isuue #1721 Content-Length may be added twice

* format code

* Add ClientTest unit tests

* update license header

* Update comment of ClientTest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feedback provided Feedback has been provided to the author question General usage or 'how-to' questions waiting for feedback Issues waiting for a response from either to the author or other maintainers
Projects
None yet
Development

No branches or pull requests

5 participants