Kong / unirest-java

Unirest in Java: Simplified, lightweight HTTP client library.
http://kong.github.io/unirest-java/
MIT License
2.6k stars 592 forks source link

Bug for parse cookies from header #341

Closed LywLover closed 4 years ago

LywLover commented 4 years ago

Describe the bug If the cookies contain "+" or "=" , the cookies can't be parse correct

To Reproduce I got a respon header like this Set-Cookie: _id=A528A0D64DA61CB01241EF6E18E4D675170DDB56CB430000AF58625E920CF940~pl1ZYwDDdoAnfY3RtqZ2Ti1MYOf7Q8jRrFQLneSGK7qQoUX1GHW/xDcqweGyclm5rm/g/YFCV3ohuHoz2oad5M0MX9Ru9V7bFr2s08d1lHxbn39gw71AI+ZVejq5FpMHKBzyjoBGG6NY6xYVTwP9NHo14SY0CXs60k2UTpJsOTNzAHIZaedg7o6R/8qyAQ8GF25K2o773pFLrYtjgKHohkk5ukz/yEGQitq8NgC5hiqX0=; expires=Fri, 06 Mar 2020 16:05:35 GMT; max-age=7200; path=/; domain=xxx.com; HttpOnly

Then println(it.cookies) [_id=A528A0D64DA61CB01241EF6E18E4D675170DDB56CB430000AF58625E920CF940~pl1ZYwDDdoAnfY3RtqZ2Ti1MYOf7Q8jRrFQLneSGK7qQoUX1GHW/xDcqweGyclm5rm/g/YFCV3ohuHoz2oad5M0MX9Ru9V7bFr2s08d1lHxbn39gw71AI ZVejq5FpMHKBzyjoBGG6NY6xYVTwP9NHo14SY0CXs60k2UTpJsOTNzAHIZaedg7o6R/8qyAQ8GF25K2o773pFLrYtjgKHohkk5ukz/yEGQitq8NgC5hiqX0;Path=/;Domain=xxx.com;Max-Age=7200;HttpOnly]

Expected behavior The right cookie value should be the same as header ,but it replace "+" to " " and removed "="

I think the problem is the URLDecoder.decode can't parase "+" correct and you use "=" as split character

And I have a test on it https://play.kotlinlang.org/#eyJ2ZXJzaW9uIjoiMS4zLjMwIiwiY29kZSI6ImZ1biBtYWluKCkge1xuICAgIHZhbCBzdHIgPSBcIkR6TWh4SnI4VW9MRGs4K1ZacStsR3UyNT1cIlxuICAgIHZhbCB2ID0gamF2YS5uZXQuVVJMRGVjb2Rlci5kZWNvZGUoc3RyLCBcInV0Zi04XCIpXG4gICAgcHJpbnRsbih2KVxuICAgIHByaW50bG4oXCJoZWxsb3dvcmxkXCIpXG59IiwicGxhdGZvcm0iOiJqYXZhIiwiYXJncyI6IiJ9

Environmental Data:

Additional context related code https://github.com/Kong/unirest-java/blob/master/unirest/src/main/java/kong/unirest/Cookie.java#L67-L92

ryber commented 4 years ago

The Cookie spec is pretty horrible and is quite vague about the ability to put '=' in a value or a name, the subsequent attempts to fix the spec are also horrible and nobody follows them. I think you will find that there are lots of issues with special characters in values across platforms/languages/libraries/browsers. If you have any control over the thing that is setting this cookie I would strongly recommend you not use values with anything other than lower ASCII, a good way to do this is by Base64url encoding the value.

I'll take a look at attempting to fix it but I'm a little unclear about what you are asking for the +. Are you expecting a + or a " "? Because the test I just wrote shows it as a " ". This is indeed probably wrong and was an attempt to be nice as most people URLEncode cookie values. So the decoder is working, but it probably shouldn't decode by default and have a different method on cookie to get that and have the raw value be the default.

ryber commented 4 years ago

@LywLover take a look at the tests in this commit and see if it matches your expectations: https://github.com/Kong/unirest-java/commit/7e8bd41582ad068c4150e323dcba71c91c9cf112

LywLover commented 4 years ago

Thanks for your hard working! That is what I want.

ryber commented 4 years ago

complete in 3.4.01