sahaya / rest-assured

Automatically exported from code.google.com/p/rest-assured
0 stars 0 forks source link

Accept-Encoding request header cannot be changed #154

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?

1. Set given().header("Accept-Encoding", "junk")... and see debug

What is the expected output?

Accept-Encoding: junk[EOL]

What do you see instead?

Accept-Encoding: gzip,deflate[EOL]

What version of the product are you using? On what operating system?

1.4 (sorry, couldn't test with 1.5) on OSX

Please provide any additional information below.

It seems that RESTAssured always sends following header and it's impossible to 
override it:

    Accept-Encoding: gzip,deflate[EOL]

For example, following test will fail for webapp which provides uncompressed 
output when there's no Accept-Encoding header or Accept-Encoding does not 
contain "deflate" compression:

given().
    header("Accept-Encoding", "none").  // put junk into header, app looks for "deflate"
    pathParam("messageId", messageId).
expect().
    statusCode(200).and().
    header("Content-Type", equalTo("text/plain")).
    header("Content-Encoding", not(equalTo("deflate"))).  // we still get compressesed output!
when().
    get(REST_PATH);

Original issue reported on code.google.com by rus...@gmail.com on 2 Feb 2012 at 1:21

GoogleCodeExporter commented 9 years ago
That's probably because that HTTP Builder doesn't care about it. In the next 
version I've forked HTTP Builder so now I can control most things. Not sure if 
this will make it to the next release though. Thanks for reporting.

Original comment by johan.ha...@gmail.com on 2 Feb 2012 at 7:16

GoogleCodeExporter commented 9 years ago

Original comment by johan.ha...@gmail.com on 3 Feb 2012 at 9:46

GoogleCodeExporter commented 9 years ago
Any update or workaround for this issue?

Original comment by thomas.l...@gmail.com on 26 Jun 2013 at 3:42

GoogleCodeExporter commented 9 years ago
No not yet :(

Original comment by johan.ha...@gmail.com on 27 Jun 2013 at 5:38

GoogleCodeExporter commented 9 years ago
In RequestSpecificationImpl.grooy, RestAssuredHttpBuilder.doRequest(): headers 
are added but not overriden. The problem is that the "Accept-Encoding" header 
has already been set in the constructor of the ancestor class HTTPBuilder:

this.setContentEncoding(ContentEncoding.Type.GZIP,ContentEncoding.Type.DEFLATE)

Maybe a quick & dirty fix would be to delete this line (why would the HTTP 
client enable compression if not asked by the API user anyway ?).

Original comment by thomas.l...@gmail.com on 28 Jun 2013 at 12:07

GoogleCodeExporter commented 9 years ago
The problem with the quick fix is that it breaks backward compatibility without 
a good way to add GZIP support. I think we need to solve it in a better way.

Original comment by johan.ha...@gmail.com on 1 Jul 2013 at 10:46

GoogleCodeExporter commented 9 years ago
That's why I called it dirty :-)
Another simple way would be to allow overriding headers. Is there a reason to 
only allow adding headers but not override already existing ones ?

Original comment by thomas.l...@gmail.com on 1 Jul 2013 at 11:59

GoogleCodeExporter commented 9 years ago
The only reason is that Rest Assured forked HTTP Builder which had this as 
default behavior. I've simply not have time to fix it properly :)

Original comment by johan.ha...@gmail.com on 1 Jul 2013 at 12:19

GoogleCodeExporter commented 9 years ago
I propose a pull request that keeps the default behavior. Tell me what you 
think about it: https://github.com/jayway/rest-assured/pull/17

Original comment by thomas.l...@gmail.com on 2 Jul 2013 at 1:18

GoogleCodeExporter commented 9 years ago
Good work! However I would prefer to have it in a configuration, called 
something like ContentEncodingConfig or AcceptEncodingConfig. This way it's 
possible to define it statically for all requests (using RestAssured.config = 
..) or using it an a request specification builder for better re-use. See for 
example com.jayway.restassured.conf ig.ObjectMapperConfig. It might be a good 
idea to a have a shortcut to setting the content encoders like you've done with 
the "acceptEncoding" method in the RequestSpecification as well. 

Btw, do you think it ought to be called "acceptEncoding" or 
"contentEncoding"/"contentEncoders"? Perhaps setting the Accept-Encoding header 
is just a side-effect of specifying a content encoder? 

So would you be so kind to make it a config instead and amend that to the pull 
request? I think you've done a great start but making it a config would, in my 
view, be a more correct way to do it for the reasons mentioned earlier.

Original comment by johan.ha...@gmail.com on 3 Jul 2013 at 5:26

GoogleCodeExporter commented 9 years ago
I tried to follow your advices in my new commit.

Original comment by thomas.l...@gmail.com on 3 Jul 2013 at 2:36

GoogleCodeExporter commented 9 years ago
Thanks! I'll try to take a look at it during the day or this evening.

Original comment by johan.ha...@gmail.com on 4 Jul 2013 at 5:56

GoogleCodeExporter commented 9 years ago
You can now configure this in the DecoderConfig

Original comment by johan.ha...@gmail.com on 14 Nov 2013 at 9:12