tests: update cookie expiry dates to far in the future

[kanavin]

This allows testing Y2038 with system time set to after that, so that actual Y2038 issues can be exposed, and not masked by expiry errors.

Closes #11576

[bagder]

I wonder if we ever build or test on any platforms with 32 bit time_t...

[bagder]

We do: mingw: TESTFAIL: These test cases failed: 31 46 61 1415 1915

[bagder]

This breaks testing, needs update.

[kanavin]

This breaks testing, needs update.

But how? I am not exactly sure what to do here. Dropping legacy platforms from testing isn't going to be accepted.

Another option is to not run the tests that use post-2038 dates where they're never going to pass, but I'm not sure how to technically implement that. Is there a selective test exclusion facility in curl or its pipelines?

[bagder]

But how?

By adapting the tests for platforms where sizeof(time_t) < 8.

Perhaps this could be done by making a variable for the test cases so that we can do %if [] .. %else ... %endif.

[bagder]

@kanavin do you plan on addressing these problems?

[kanavin]

@kanavin do you plan on addressing these problems?

Yes, I do, although I'd rather convince you to drop 32 bit time_t systems from testing, and relegate them to retro computing museum where they belong. So we can focus time and effort on, oh, power grid and water supply and traffic control systems not collapsing in 2038.

For background, fixing curl is a part of a broader effort to fix all known Y2038 issues in Yocto based linux stacks, so I'm juggling multiple tickets, upstream submissions and digging into large codebases I've never seen before:
https://git.yoctoproject.org/poky-contrib/tree/meta/conf/distro/include/time64.inc?h=akanavin/y2038
https://git.yoctoproject.org/poky-contrib/log/?h=akanavin/y2038

I'll get to it.

[bagder]

I'd rather convince you to drop 32 bit time_t systems from testing,

If it would be very hard to get them working and this work solved a lot of problems, then we could have discussed that. But as we know, this PR mostly brings a visual change that isn't strictly necessary and I consider that a weak reason to break testing for a few legacy platforms.

In the mean time, I have created a large-time feature we can make test cases depend on and do conditionals with, which can end up handy when improving tests for small and large time_ts.

[kanavin]

If it would be very hard to get them working and this work solved a lot of problems, then we could have discussed that. But as we know, this PR mostly brings a visual change that isn't strictly necessary and I consider that a weak reason to break testing for a few legacy platforms.

This work does solve problems, so let's discuss.

Ensuring things are not going break down in 2038 is not just about extending time_t to 64 bits and recompiling.

Components also can, and do place time into 32 bit variables internally, usually by oversight. Examples:
https://github.com/Perl/perl5/pull/21379/files
https://git.yoctoproject.org/poky-contrib/tree/meta/recipes-devtools/tcltk/tcl/0001-generic-tcl.h-use-Tcl_WideInt-for-seconds-in-Tcl_Tim.patch?h=akanavin/y2038
https://git.yoctoproject.org/poky-contrib/tree/meta/recipes-devtools/tcltk/tcl/0002-tclInterp.c-use-Tcl_WideInt-for-seconds-in-ChildTime.patch?h=akanavin/y2038
(I didn't submit the tcl patches upstream yet, but it'll happen)

Sometimes there is a configuration switch (off by default) which needs to be switched on to prevent copying time into 32 bits from happening:
busybox: https://git.yoctoproject.org/poky-contrib/commit/?h=akanavin/y2038&id=33257f4bb2bff558bb5191693388b9bffa1e64bf
perl: https://git.yoctoproject.org/poky-contrib/commit/?h=akanavin/y2038&id=219b9421c0a581c72ec059da786ef2736121e9a4

How can these problems be found in the first place? The only way to do it is to set system time to post-2038 and run components' test suites one by one. Which, in turn, requires that the test suites don't throw errors because they do not like the date. So no, this PR is not about ' visual change that isn't strictly necessary'. I did run curl's test suite with it, and I can confirm to you that no issues were found. Which you should be happy with, rather than be annoyed about. There's a lot of invisible 32 bit embedded infrastructure out there, it will still be 32 bit in 2038, and I'd say having electricity and water in your house is somewhat important.

In the mean time, I have created a large-time feature we can make test cases depend on and do conditionals with, which can end up handy when improving tests for small and large time_ts.

Thanks, I'll use it, but small time_ts are only relevant to retro computing hobbyists at this point, usually because they have abandonware blobs that can't be recompiled, i.e. vintage games or old proprietary OSs. I still don't understand why you care for them this much.

[bagder]

How to deprecated curl support for "small" time_t (or anything else really):

• argue for it on the mailing list
• add a mention of it in DEPRECATED
• set a date for the removed support at least 6 months into the future, ideally longer. To give the world time to act and respond.

This is then not a promise that it actually will happen. It then also depends a lot on the reactions and feedback from the community.

[kanavin]

The community is going to reject the proposal. The major argument is going to be that 32 bit Debian - a popular, current distro - is still on 32 bit time_t (and not just on historical hardware - 32 bit arm is entirely current), they're still making plans, and the plans don't look particularly reassuring regarding timelines or specific steps to make it happen:
https://wiki.debian.org/ReleaseGoals/64bit-time
"The implications of a transition are being studied. The library transition appears to involve about 500 packages, which is a lot, but probably manageable "
I'm not sure they can pull it off through unpaid volunteer work, without corporate funding of some kind.

Other Linux vendors seem to have abandoned 32 bit computing altogether.

Yocto has the benefit of being a from-source distro generator, so we just flipped the switch and no one complained (so far):
https://git.yoctoproject.org/poky-contrib/commit/meta/conf/distro?h=master&id=231866f75c8e162b123a5d6fd9acaaa396df918f

I apologize for being annoying, my goal is to run as many tests as I can think of in a complete system, ensure all issues are documented and all upstreams are notified. There's still a long list of things to look into, so there's only so much time I can allocate to any specific item. Each particular issue, such as this one, gets looked at in a round-robin schedule, rather than being exclusively driven to completion. Embedded distro engineering is critically under-resourced (yes, I know, every open source project is, but distro engineering in particular so :).

I hope this isn't seen as another unwanted lecture.

[bagder]

I was not aware of 32 bit debian still using 32 bit time_t. But I think you're right that could very well be a reason why people would not like to deprecate this in curl yet.

And when not deprecated, we don't willingly break it.

[kanavin]

I have set up a i386 debian docker and ran the tests in there, without the patch, and then with it. Without the patch everything passes, with the patch the failures are the same set as mingw:

31 46 61 1415 1915

I suppose the simplest is to skip them if time_t is too small, for using dates that need more than 32 bits to be correctly represented.

[bagder]

In order to keep cookie-testing working even for small-time_t systems, the nicest approach is to insert conditionals into the test cases like this:

diff --git a/tests/data/test31 b/tests/data/test31
index 821fed9fb..1b9324021 100644
--- a/tests/data/test31
+++ b/tests/data/test31
@@ -52,11 +52,15 @@ Set-Cookie: httpandsec7=myvalue8  ; domain=test31.curl; path=/p4/; secure; httpo
 Set-Cookie: httpandsec8=myvalue9; domain=test31.curl; path=/p4/; secure=; httponly
 Set-Cookie: partmatch=present; domain=test31.curl ; path=/;
 Set-Cookie:eat=this; domain=moo.foo.moo;
 Set-Cookie: eat=this-too; domain=.foo.moo;
 Set-Cookie: nodomainnovalue
+%if large-time
+Set-Cookie:   nodomain=value; expires=Fri Feb 13 11:56:27 GMT 2537
+%else
 Set-Cookie:   nodomain=value; expires=Fri Feb 13 11:56:27 GMT 2037
+%endif
 Set-Cookie: novalue; domain=reallysilly
 Set-Cookie: test=yes; domain=foo.com; expires=Sat Feb 2 11:56:27 GMT 2030
 Set-Cookie: test2=yes; domain=se; expires=Sat Feb 2 11:56:27 GMT 2030
 Set-Cookie: magic=yessir; path=/silly/; HttpOnly
 Set-Cookie: blexp=yesyes; domain=test31.curl; domain=test31.curl; expiry=totally bad;
@@ -155,11 +159,15 @@ Accept: */*
 test31.curl    FALSE   /we/want/       FALSE   0       prespace        yes before
 test31.curl    FALSE   /we/want/       FALSE   0       withspaces2     before equals
 test31.curl    FALSE   /we/want/       FALSE   0       withspaces      yes  within and around
 .test31.curl   TRUE    /we/want/       FALSE   0       blexp   yesyes
 #HttpOnly_test31.curl  FALSE   /silly/ FALSE   0       magic   yessir
+%if large-time
+test31.curl    FALSE   /we/want/       FALSE   17896593387     nodomain        value
+%else
 test31.curl    FALSE   /we/want/       FALSE   2118138987      nodomain        value
+%endif
 .test31.curl   TRUE    /       FALSE   0       partmatch       present
 #HttpOnly_.test31.curl TRUE    /p4/    FALSE   0       httponly        myvalue1
 #HttpOnly_.test31.curl TRUE    /p4/    FALSE   0       httpo4  value4
 #HttpOnly_.test31.curl TRUE    /p3/    FALSE   0       httpo3  value3
 #HttpOnly_.test31.curl TRUE    /p2/    FALSE   0       httpo2  value2

[kanavin]

Let's see what github CI says now.

[dfandrich]

In order to keep cookie-testing working even for small-time_t systems, the nicest approach is to insert conditionals into the test cases like this:

An alternate approach would be to create a new test that just tests the >2038 dates and restrict the entire test to systems with >32 bit time_t. That can be done with <precheck> and doesn't need any additional test harness support.

[bagder]

An alternate approach would be to create a new test that just tests the >2038 dates and restrict the entire test to systems with >32 bit time_t. That can be done with and doesn't need any additional test harness support.

The additional support you mention has already been added. The feature large-time exists now for tests. But yes, we can do a test only run if large-time is set, or if not set. If that is easier.

[bagder]

Grrr. The hyper job failure in test 31 is annoying.

 31: data FAILED:
--- log/check-expected	2023-08-24 14:44:58.208361780 +0000
+++ log/check-generated	2023-08-24 14:44:58.208361780 +0000
@@ -5,7 +5,7 @@
 Content-Type: text/html[CR][LF]
 Funny-head: yesyes[CR][LF]
 Set-Cookie: blankdomain=sure; domain=; path=/[CR][LF]
-Set-Cookie:   nodomain=value; expires=Fri Feb 13 11:56:27 GMT 2525[CR][LF]
+Set-Cookie: nodomain=value; expires=Fri Feb 13 11:56:27 GMT 2525[CR][LF]

A problem that leads to this is that the %if condition in tests cannot be nested - but it does not seem to error/warn about it. I'll work on this.

Hyper is then "cleaning" headers that it delivers to us so even if it reads multiple leading whitespaces it delivers them to us with just one, and the test script uses the same block for what to send and then to check that the exact same contents was received.

[dfandrich]

If hyper is enabled, it's safe to assume 64-bit time_t. Also, one of <strip>, <strippart> or <postcmd> might be usable to fix this if it can't be done another way.

[bagder]

Supporting nested condition was not very hard either, so I made #11728

[bagder]

If hyper is enabled, it's safe to assume 64-bit time_t.

I don't think that's true? I bet you can build hyper on 32 bit debian, can't you?

[bagder]

@kanavin if you rebase this branch onto master and force-push, it will use my %if improvements supporting nested use, which then should fix the hyper test failures... I hope.

[kanavin]

@bagder seems like it didn't help?

[dfandrich]

I don't think that's true? I bet you can build hyper on 32 bit debian, can't you?

Looks like you're right. I had thought that all relevant Rust time types were u64 on all architectures, but some important ones mirror the underlying architecture's time types and will also be 32 bits in that case. I don't know if Hyper is actually affected by that, though, but chances are good that it would be.

[bagder]

I don't know if Hyper is actually affected by that

Even when curl is built to use hyper, the date parsing - including the one done for cookies - is all done by our native code so when we build curl+hyper on 32 bit time_t systems, we are affected.

In theory, we could fix our own date parser to not rely that much on the native time_t size and possibly that way avoid some 2038 problems, but I am not signing up for that job willingly...

[bagder]

@bagder seems like it didn't help?

No, my bad. The %if thing was not good enough. Hang on!

[bagder]

With #11731 I made the hyper tests work fine in my local tests. My initial nested-if PR was a bit too simple.

[bagder]

@kanavin okay, sorry for that intermission, can you rebase it again and I think it should be good?

[bagder]

Lovely, thanks for your patience!