nettyCookie turns standard cookie into futuristic cookie - erroneous millisecond/second conversion

[zgael]

Hi,
following the upgrade from karate 0.9.6 to any 1+ version, I encountered the following :
After a specific login call, my server answers a setCookie with the following expires directive :
Expires=Mon, 09 Aug 2021 08:34:21 GMT
(cookie is supposed to have couple hours lifetime).

But then Karate displays Expires=Tue, 25 Aug 53626 23:46:21 GMT;

After some digging, the problem is here :
https://github.com/intuit/karate/blob/3a9d5148c3e0ae167736174527040abb8d7fc536/karate-core/src/main/java/com/intuit/karate/http/ApacheHttpClient.java#L301

c.getExpiryDate().getTime() units are milliseconds, while MAX_AGE expects seconds.

Simple correction would be to divide by 1000 : c.getExpiryDate().getTime()/1000

We could also convert this to Instant and use the toEpochSeconds method, but that approach requires more type conversions and doesn't look clear to me.

What are your thoughts on this ?

[ptrthomas]

@zgael do you want to take a shot at a PR ? I'd like a test added to perhaps this set to catch this: https://github.com/intuit/karate/blob/v1.1.0/karate-core/src/test/java/com/intuit/karate/core/KarateHttpMockHandlerTest.java

[zgael]

Sure I'll try to give it a shot.
Thanks for pointing the direction to the test!

[zgael]

So I've actually tried to tackle the problem... and it is a bit more complicated than I thought. You can find my changes here : https://github.com/zgael/karate (develop branch).

• Wrote a test (pretty similar to the previous last one)
• Tried to correct (with a mere /1000 added, see above c.getExpiryDate().getTime()/1000).
• Then I realized what was really wrong :

There are 2 attributes in a cookie that do almost the same thing : Max_Age, and Expires. They both point to a time when the cookie should stop being used, but the unit is different :
Max_Age : a duration in seconds (could be 1hour for the sake of this example).
Expires : A date (format : https://developer.mozilla.org/fr/docs/Web/HTTP/Headers/Date)

So the line map.put(Cookies.MAX_AGE, c.getExpiryDate().getTime()); is quite wrong, as it puts in the max_age field a timestamp, which leads to huge futuristic dates.

I tried to convert max_age to expiry_date, relatively to Instant.now()... But that leads to erroneous conversions if there's some leeway (you can try to execute the new test I've written to check : server sends 08/13/2021 08:34:21 GMT, I receive 08/13/2021 08:34:20 GMT).

Also, the ServerCookieEncoder.LAX.encode function performs a transformation from max_age to expires, using System.currentTimeMillis, but includes both directives in the final cookie, which I find weird (once again you can check the test failure and the comparison : expected Expires, found Max_Age + Expires).

All in all, the problem arises because :

• On one side, we use io.netty.handler.codec.http.cookie.Cookie, which uses exclusively Max_Age directive.
• On the other side, we use org.apache.http.cookie.Cookie which uses exclusively Expires directive.
• We convert from Apache to Netty Cookie.

Ultimately, since both implementation fail to adress what we need, I'd rather use a "custom" Cookie class (could re-use the 0.9.6 simple one) that can handle both Expires and Max_Age (and bring the utilities from Netty we use, like the encode function)

As a consequence, you wouldn't need to rely anymore on Netty Cookies or Apache Cookies, since their implementations are "incomplete" (they chose to do it this way - one using Expires, the other Max-Age for probably good reasons, but as a test framework that should behave like a browser, I believe karate has to handle correctly both directives, as do browsers).

What are your thoughts on this @ptrthomas ? If I'm not clear enough, we could plan a small video talk to make it clearer.

@davinkevin

[ptrthomas]

@zgael I like your idea of a custom cookie class and re-using netty where possible. if we can succeed in not using apache cookie management at all, that will be a bonus. I can volunteer to re-engineer the "retry until" and "follow redirects" logic to propagate cookies across the re-tries which was the "blocker" I referred to earlier.

[ptrthomas]

closing because of inactivity

[elafontaine]

https://github.com/karatelabs/karate/blob/925eebb3dfbd7f3ac7a39d11ad43720107bf2a2f/karate-core/src/main/java/com/intuit/karate/http/ApacheHttpClient.java#L291..315

We use Karate for testing web stuff. Since version 1.0.1 we are unable to validate that the cookies we return are set correctly. Especially the cookie properties called "samesite" and "Path". This was removed and we're wondering what was the reason for manual management of the cookies by Karate.

The problematic is that the cookie is made anew by lines 309-311 and the new value was generated from a map (line 304) that does not contain the whole cookie that was received (Line 296-303, missing "samesite" attribute and "path").

I would like to suggest that instead of making everything about cookie managed in Karate, that we give a config for disablingCookieManagement (old behaviour) or leave it (new behaviour).

This would basically put back the old behaviour of leaving the cookie as a string (but not take action on it). At the same time, I could just add the 2 fields I see missing (Path, SameSite).

I can work on this if you want.

[ptrthomas]

@elafontaine sure we welcome a PR. please do check whether the latest develop version works first, I don't exactly remember - but some tweaks to cookie handling were contributed in the mean time for e.g. #1965

[ptrthomas]

that said - I try to avoid introducing config like this - so if you can fix things so that it works for all cases, that will be very much preferred. some history can be found here for cookie handling: #1777

[ptrthomas]

@elafontaine would be great if you can weigh in on if this is related: #2165

[elafontaine]

Hi @ptrthomas , I saw the new commit :) good job, I'll look into testing it tomorow. However, I still don't see the "sameSite" attribute being passed, so I'm hoping it will be present in responseCookie either way.

I'll keep you posted.

[elafontaine]

I tested, but the cookies aren't showing either :( .

    When method post
    Then status 302
    * print response
    * print responseHeaders
    * print responseCookies
    And match responseCookies == '#notnull'
    And match responseHeaders['Set-Cookie'][0] contains 'SameSite=None'

The Set-Cookie is not present nor is there any other way of retrieving it... Unless I'm missing something

[elafontaine]

For me, it's clear that the following documentation isn't working ;
https://github.com/karatelabs/karate#responsecookies

[ptrthomas]

@elafontaine feel free to create a fresh issue if you are able to create a simple example to replicate.