Kong / unirest-java

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

Cookies are not included in the subsequent requests #387

Closed haroon-sheikh closed 3 years ago

haroon-sheikh commented 3 years ago

Describe the bug When the cookie is dropped and cookie value is surrounded with double quotes then the cookie is not passed onto the subsequent requests.

E.g

To Reproduce Steps to reproduce the behavior:

  1. Make a request that will drop cookie_name="blah";Max-Age=86400;Expires=Wed, 9 Dec 2020 18:01:58 GMT;Path=/;Domain=example.com;HTTPOnly
  2. Make another request on same domain

Expected behavior the cookie should be passed and sent with the 2nd request.

Screenshots If applicable, add screenshots to help explain your problem.

Environmental Data:

ryber commented 3 years ago

If the cookie is expired by the server, why would client continue to send it? Per the spec it shouldn't because the cookie is now expired. Or am I misunderstanding sometihng?

haroon-sheikh commented 3 years ago

Sorry @ryber please set the Expires time in future before setting the cookie.

I dont know if it helps, I'm also seeing the following warning when cookie is dropped:

WARNING: Invalid cookie header: "Set-Cookie: cookie_name="blah";Max-Age=86400;Expires=Wed, 9 Dec 2020 20:26:05 GMT;Path=/;Domain=example.com;HTTPOnly". Invalid 'expires' attribute: Wed, 9 Dec 2020 19:26:05 GMT
ryber commented 3 years ago

Oh Ok, I was thrown by you saying "dropped". I thought they were deleting the cookie.

The problem is that the format of the cookie is slightly non-standard from what is expected. by default Unirest uses the Netscape cookie spec which is the same as Apache Client. You need to set it to "standard", per the config https://github.com/Kong/unirest-java/blob/main/unirest/src/main/java/kong/unirest/Config.java#L566-L581

Unirest.config().cookieSpec("standard");

haroon-sheikh commented 3 years ago

Ah perfect. That worked with no problems.

Thanks for your help. Much appreciated. 👍

haroon-sheikh commented 3 years ago

@ryber One more question please, do you know if a cookie is dropped from a request, is there a way for me to either override / remove for the next request?

I've tried setting the Cookie: cookie_name=value header for the next request but that doesn't work when enableCookieManagement is set to true.

ryber commented 3 years ago

You can force sending the cookies with cookiemanagement on, but you have to do it every time. Cookies .... the setting and expiring of them, is the domain of the server. Clients aren't really supposed to be in charge of that and they aren't going to be part of the management store because they didn't come from the server.

There are lots of tests for force-sending one though:

https://github.com/Kong/unirest-java/blob/main/unirest/src/test/java/BehaviorTests/CookieTest.java

haroon-sheikh commented 3 years ago

I see your point about cookiemanagement, My scenario is just to force send on top:

Unirest.config().enableCookieManagement(true);
Unirest.config().cookieSpec("standard");

Request 1 drops cookie cookie_name=old_value Request 2 tries to force sending cookie_name with a different new_value. E.g.

Unirest.get(MockServer.GET)
  .cookie("cookie_name", "new_value")
  .asObject(RequestCapture.class)

However when running this request, it's still sending the cookie with the old_value.

ryber commented 3 years ago

So, just to fix my mistake. sort of. Either you want cookie management or not, we either honor the contract of cookies, OR you can override the whole thing. But we cannot do a mix

haroon-sheikh commented 3 years ago

Okay I'll just work with cookie management being off.

ryber commented 3 years ago

So, just to clarify the reason, Apache manages the cookie store, and this is just it's behavior. It is possible to write your own CookieStore and make a custom HttpClient but, I'm also in the process of re-writing Unirest to not use Apache at all, so any more investment in it is just tech debt at this point.

You could handle this pretty easily with a custom Unirest interceptor.

haroon-sheikh commented 3 years ago

Thanks, I think It'll be easier for me to just turn cookie management off for now ;)

Just out of curiosity, how would I implement this using an interceptor?