fix core: storing all cookies without possibly wrong selection, keeping session cookies semantics

[alex-aparin]

As mentioned previously in issue #1182. The current implementation of cookie storage in clients::http::response does not conform to RFC 6265 (it can incorrectly write cookies). Additionally, the accessor methods for Max-Age/Expires have default values, which is also incorrect (breaking the semantics for Session Cookies). The goal of this PR is primarily to fix the incorrect cookie writing, but without a full-fledged implementation of CookiesJar.

closes: #1182

---

Note: by creating a PR or an issue you automatically agree to the CLA. See CONTRIBUTING.md. Feel free to remove this note, the agreement holds.

[alex-aparin]

UPD: I have noticed some updates in master branch like this. Please note that just overwriting cookie with the same name is not enough, you should take into account expire/max age and domains/urls, because in some cases we should delete cookie at all or just keep additional cookie with the same name but for another domain/url. That's why I am just recording all cookies without any actions (without implementing full cookiejar)

[Malevrovich]

Hi, and thanks for the PR!

I’m not fully comfortable with the idea of putting all cookies into a vector. At the very least, this changes the public API, and I’m pretty sure many users rely on the current way cookies are exposed and read.

Would you be interested in properly supporting the cases that don’t fully comply with the RFC instead? I’d be happy to review such a change and help get it merged.

I’d prefer to avoid moving everyone from a map to a vector now, only to potentially move them back to a map later.

[alex-aparin]

Hi all. Thank you for response. There was hot discussion in userver's telegram group about breaking changes, I'll make this pr as draft, additional tests for tricky cookies will be added. I think the better way is to add additional method to allow user to specify - use old but wrong api with the same semantics (map of cookies) or use new method which will return CookieJar with needed getters (some kind of map of map (or map of list) can be used internally)

[alex-aparin]

This version can be viewed as starting point for API and tests for CookieJar compliable with RFC 6265.
Resume of commits:

1. Reverted changes. New api is compatible with current one
2. Implemented CookieJar, it now primarly used for receiving cookies from server. It has also simple api (but not so effective in terms of parsing urls) in order to SEND cookies, we need to think about API to set this to http client.
3. Added tests to covert tricky cookies.

This was not so easy as I thought and a little bit messy from my implementation, but I hope storage covers many issues, any help will be greatly appreciated. Thank you!

UPD: I have updated code due paragraph 2. Now client partially supports sending cookies from cookie jar, but we need to think where to store cookiejar, response object can be null before sending request. HttpClient.CookiesFromServerCookieJar test demonstrates the problem (it fails at runtime)

UPD2: Now client fully supports sending cookies from cookie jar. HttpClient.CookiesFromServerCookieJar test is working

[alex-aparin]

Yet another improvement, now http client fully supports sending cookies from cookies jar and also receiving. Some bugs were also fixed. At the moment APIs of cookie jar and http client are stabilized. I recommend the latest code snapshot to get familiar with. Maybe tests will be polished a little bit later

[Malevrovich]

I have reviewed the PR, and this topic is even more complex than I expected when we discussed it in the chat.

At this point, I am concerned about the RFC deviations and the additional shared_ptr allocation on every request. I would also like to study the redirect behavior in more detail: what happens if an intermediate redirect hop returns a Set-Cookie header? Is that cookie applied to the next hop? Another question is whether cookies whose Expires time has already passed can still be sent in a request.

I am not asking you to implement any changes right now. These are the areas I would like us to keep in mind for the next review iteration. I would appreciate some more time to investigate the topic, and I will come back with implementation ideas.

[alex-aparin]

@Malevrovich CookieJar was redesigned, now it uses libcurl's one. It should be rfc 6265 compliant, and at the same time it should solve issues with request reusing, redirections as you menioned earlier. Some polishing are needed like additional tests for request reusing, redirecting etc. I tried to preserve majority of tests I used for my custom implementation to test libcurl's one. (btw these tests discovered a few libcurl bugs like curl/curl#9930)