curl
curl copied to clipboard
curl
cookies: the max expire time is 400 days
"The limit SHOULD NOT be greater than 400 days (34560000 seconds) in duration. The RECOMMENDED limit is 400 days in duration, but the user agent MAY adjust the limit. Max-Age attributes that are greater than the limit MUST be reduced to the limit."
Let's see how much breakage this causes...
The 400 day limit comes from the RFC 6265 bis draft. It was added to the draft in -10, a year ago, and is still in there today in -13. I say that makes it likely to also get shipped into the final RFC as well, although of course nothing is certain until it actually happens.
This seems like an odd way to test. I would just set an env var for MAX_AGE to allow overriding the 400day default and use a much shorter expiration in the tests.
I don't think that would change much. Such an environment variable check would only be present in a debug build so it would have the same limitations. Using the actual 400 days or a custom shorter time in tests don't change much in the actual tests. Since the cap is 400 days, these tests make sure that the implemented cap actually is 400 days.
The difference is that such a check doesn't need to be only present in debug builds. There may be site-specific deployments that would like to take advantage of the ability to set shorter expiration caps.
Ok sure, but I would be against that idea.
Controlling library behavior using environment variable is not a good API for applications. For that reason, we avoid introducing any more such than what we already have.
As nobody ever asked for such a feature, the only reason to add it would be to be able to test it with non-debug builds, and I find that a bad reason to introduce additional complexity.
OK skip the env var idea, and just add it to CURLOPT. I don't see any way you're going to be able to test this in a non-debug build other than to make it a standard feature.
How would we test it then? It would still be a challenge since the end time is relative and we store the end time with an absolute number.
It is the moving start time that we can "fix" with debug build, which makes it easy to test.
Can't we just test for Expires: 2034-11-16 xx.yy.zz in the testfiles and be content that we have 10 years worth of tests before we need to tweak? That will also have the side-benefit that every test run will have a slightly different interval, reducing the risk of a subtle bug that happens to not be exposed for a pre-determined test.
My only other idea, after this I'm out of ideas: keep the approach in this patch, but move time_now() to its own file. When you want to run the tests, always explicitly link the test binaries against the debug version of the object file.
Can't we just test for Expires: 2034-11-16 xx.yy.zz in the testfiles and be content that we have 10 years worth of tests before we need to tweak? That will also have the side-benefit that every test run will have a slightly different interval, reducing the risk of a subtle bug that happens to not be exposed for a pre-determined test.
Currently we store the expiry time in its absolute epoch ASCII number of seconds in the cookie jar file, which with this patch is capped to a maximum of 400 days into the future from now.
Incoming cookies today that are asked to expire in 2034, are thus instead capped to expire already early 2025 (400 days from now), which is stored as 1738364400 or something. But ten seconds later, we store it as 1738364410.
I will put this PR on hold and pause it at least until the proposed RFC is still only in draft. If this wording remains when it eventually possibly get published, we can bring it back up.