Conversation
I'll check and run it on other platforms/compilers before 1.5.2 release
|
Note: it's not that the constructor subtracts a second, it is simply about timing when toString() is called; both constructor and toString() take a timestamp, while _maxAge remains the same. So, the string generated by toString() call will "drift" into the future with the lifetime of the cookie. I'm not sure if this is good design but that's how the class behaves now. To me it seems that the absolute "expires" was much more clear but, if I'm not mistaken, that's obsolete ... |
|
I guess the problem with the absolute expire time was that it may lead to issues for very short expire times when the clocks of client and server drift too much. |
|
Ok, thanks for clarifying. Sure, that sounds better. I will change the testcase to time the elapsed time between creating the cookie and calling toString and then use that delta in a fuzzy comparison with the expected value. |
|
Commit 5a70971 (real changes in d3a61a0) I changed the testcase to be nicer and less brittle for now. Idea is to pass an expiry time string into the cookie on creation, call toString, time how long the cookie was alive between those two events, then use that delta in a comparison between the given expiry information and the expiry information printed by toString in both v0 and v1 modes. So I think that matches "parse out the date/time from the string and compare against expected value with a delta." As an aside, it might be nice to have overloads of get/setMaxAge which take DateTimes/Timestamps to make it more intuitive for applications to create a cookie with a specific expiry date(for web applications trying to generate session-cookie-expiring Set-Cookie headers, or trying to set the length of a login session). I can do that, but I'm busy with my own projects at the moment so might not be in time for 1.5.2. |
|
Apologies Aleksandar, I see you cleaned up my old test case. I'll merge and fix up the style according to how you have it and submit a new pull request. |
Make HTTPCookie support expiry times in the past. Fix #222
The testcase passes fine for me on Linux-gcc, but the code probably isn't great. Specifically I'm not sure why the constructor for HTTPCookie seems to (consistently) subtract one second from timestamps in the future. I worked around it though.