{"thread":{"id":"61681","subject":"[PATCH v4 0/2] Darcy's \"date underflow fix\" topic, final reroll","startedAt":"2024-06-25T23:12:51Z","lastAt":"2024-06-26T18:32:54Z","messageCount":7,"participants":["Junio C Hamano","Eric Sunshine","Phillip Wood"],"isPatch":true,"patchVersion":4,"patchTotal":2},"messages":[{"id":"497662","messageId":"20240625231248.4070257-1-gitster@pobox.com","threadId":"61681","inReplyTo":null,"subject":"[PATCH v4 0/2] Darcy's \"date underflow fix\" topic, final reroll","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-06-25T23:12:46Z","receivedAt":"2024-06-25T23:12:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"So it has been some time since we discussed this topic.  Let's clean\nup the messy \"SQUASH???\" patches I had to queue on top of the main\npatch to keep the CI working and make them into a preliminary patch.\n\nThe tree at the end of the series is identical to what has been\nqueued in 'seen' for the past few weeks.  The only difference is\nthat we first lay groundwork to skip certain time-parsing tests on\n32-bit systems first, and then use Darcy's patch with minimum\nadjustments for 32-bit systems.\n\n\nDarcy Burke (1):\n  date: detect underflow/overflow when parsing dates with timezone offset\n\nJunio C Hamano (1):\n  t0006: simplify prerequisites\n\n date.c          | 12 +++++++++++-\n t/t0006-date.sh | 51 +++++++++++++++++++++++++++++++++++++++++++------\n 2 files changed, 56 insertions(+), 7 deletions(-)\n\n-- \n2.45.2-796-g2ef7a3d713\n\n"},{"id":"497663","messageId":"20240625231248.4070257-2-gitster@pobox.com","threadId":"61681","inReplyTo":"20240625231248.4070257-1-gitster@pobox.com","subject":"[PATCH v4 1/2] t0006: simplify prerequisites","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-06-25T23:12:47Z","receivedAt":"2024-06-25T23:12:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The system must support 64-bit time and its time_t must be 64-bit\nwide to pass these tests.  Combine these two prerequisites together\nto simplify the tests.  In theory, they could be fulfilled\nindependently and tests could require only one without the other,\nbut in practice, but in practice these must come hand-in-hand.\n\nUpdate the \"check_parse\" test helper to pay attention to the\nREQUIRE_64BIT_TIME variable, which can be set to the HAVE_64BIT_TIME\nprerequisite so that a parse test can be skipped on 32-bit systems.\nThis will be used in the next step to skip tests for timestamps near\nthe end of year 2099, as 32-bit systems will not be able to express\na timestamp beyond 2038 anyway.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t0006-date.sh | 18 ++++++++++++------\n 1 file changed, 12 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex 3031256d14..24e8647f26 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -8,6 +8,11 @@ TEST_PASSES_SANITIZE_LEAK=true\n # arbitrary reference time: 2009-08-30 19:20:00\n GIT_TEST_DATE_NOW=1251660000; export GIT_TEST_DATE_NOW\n \n+if test_have_prereq TIME_IS_64BIT,TIME_T_IS_64BIT\n+then\n+\ttest_set_prereq HAVE_64BIT_TIME\n+fi\n+\n check_relative() {\n \tt=$(($GIT_TEST_DATE_NOW - $1))\n \techo \"$t -> $2\" >expect\n@@ -80,14 +85,15 @@ check_show raw \"$TIME\" '1466000000 -0200'\n \n # arbitrary time absurdly far in the future\n FUTURE=\"5758122296 -0400\"\n-check_show iso       \"$FUTURE\" \"2152-06-19 18:24:56 -0400\" TIME_IS_64BIT,TIME_T_IS_64BIT\n-check_show iso-local \"$FUTURE\" \"2152-06-19 22:24:56 +0000\" TIME_IS_64BIT,TIME_T_IS_64BIT\n+check_show iso       \"$FUTURE\" \"2152-06-19 18:24:56 -0400\" HAVE_64BIT_TIME\n+check_show iso-local \"$FUTURE\" \"2152-06-19 22:24:56 +0000\" HAVE_64BIT_TIME\n \n-check_parse() {\n+REQUIRE_64BIT_TIME=\n+check_parse () {\n \techo \"$1 -> $2\" >expect\n-\ttest_expect_${4:-success} \"parse date ($1${3:+ TZ=$3})\" \"\n-\tTZ=${3:-$TZ} test-tool date parse '$1' >actual &&\n-\ttest_cmp expect actual\n+\ttest_expect_success $REQUIRE_64BIT_TIME \"parse date ($1${3:+ TZ=$3}) -> $2\" \"\n+\t\tTZ=${3:-$TZ} test-tool date parse '$1' >actual &&\n+\t\ttest_cmp expect actual\n \t\"\n }\n \n-- \n2.45.2-796-g2ef7a3d713\n\n"},{"id":"497664","messageId":"20240625231248.4070257-3-gitster@pobox.com","threadId":"61681","inReplyTo":"20240625231248.4070257-1-gitster@pobox.com","subject":"[PATCH v4 2/2] date: detect underflow/overflow when parsing dates with timezone offset","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-06-25T23:12:48Z","receivedAt":"2024-06-25T23:13:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Darcy Burke <acednes@gmail.com>\n\nOverriding the date of a commit to be close to \"1970-01-01 00:00:00\"\nwith a large enough positive timezone for the equivelant GMT time to be\nbefore the epoch is considered valid by `parse_date_basic`. Similar\nbehaviour occurs when using a date close to \"2099-12-31 23:59:59\" (the\nmaximum date allowed by `tm_to_time_t`) with a large enough negative\ntimezone offset.\n\nThis leads to an integer underflow or underflow respectively in the\ncommit timestamp, which is not caught by `git-commit`, but will cause\nother services to fail, such as `git-fsck`, which, for the first case,\nreports \"badDateOverflow: invalid author/committer line - date causes\ninteger overflow\".\n\nInstead check the timezone offset and fail if the resulting time comes\nbefore the epoch \"1970-01-01T00:00:00Z\" or after the maximum date\n\"2099-12-31T23:59:59Z\".\n\nUsing the REQUIRE_64BIT_TIME prerequisite, make sure that the tests\nnear the end of Git time (aka end of year 2099) are not attempted on\npurely 32-bit systems, as they cannot express timestamp beyond 2038\nanyway.\n\nSigned-off-by: Darcy Burke <acednes@gmail.com>\n[jc: fixups for 32-bit platforms]\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n date.c          | 12 +++++++++++-\n t/t0006-date.sh | 33 +++++++++++++++++++++++++++++++++\n 2 files changed, 44 insertions(+), 1 deletion(-)\n\ndiff --git a/date.c b/date.c\nindex 7365a4ad24..bee9fe8f10 100644\n--- a/date.c\n+++ b/date.c\n@@ -868,6 +868,10 @@ static int match_object_header_date(const char *date, timestamp_t *timestamp, in\n \treturn 0;\n }\n \n+\n+/* timestamp of 2099-12-31T23:59:59Z, including 32 leap days */\n+static const timestamp_t timestamp_max = (((timestamp_t)2100 - 1970) * 365 + 32) * 24 * 60 * 60 - 1;\n+\n /* Gr. strptime is crap for this; it doesn't have a way to require RFC2822\n    (i.e. English) day/month names, and it doesn't work correctly with %z. */\n int parse_date_basic(const char *date, timestamp_t *timestamp, int *offset)\n@@ -937,8 +941,14 @@ int parse_date_basic(const char *date, timestamp_t *timestamp, int *offset)\n \t\t}\n \t}\n \n-\tif (!tm_gmt)\n+\tif (!tm_gmt) {\n+\t\tif (*offset > 0 && *offset * 60 > *timestamp)\n+\t\t\treturn -1;\n+\t\tif (*offset < 0 && -*offset * 60 > timestamp_max - *timestamp)\n+\t\t\treturn -1;\n \t\t*timestamp -= *offset * 60;\n+\t}\n+\n \treturn 0; /* success */\n }\n \ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex 24e8647f26..fd373e1b39 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -123,6 +123,39 @@ check_parse '2008-02-14 20:30:45 -05:00' '2008-02-14 20:30:45 -0500'\n check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 -0500' EST5\n check_parse 'Thu, 7 Apr 2005 15:14:13 -0700' '2005-04-07 15:14:13 -0700'\n \n+check_parse '1970-01-01 00:00:00' '1970-01-01 00:00:00 +0000'\n+check_parse '1970-01-01 00:00:00 +00' '1970-01-01 00:00:00 +0000'\n+check_parse '1970-01-01 00:00:00 Z' '1970-01-01 00:00:00 +0000'\n+check_parse '1970-01-01 00:00:00 -01' '1970-01-01 00:00:00 -0100'\n+check_parse '1970-01-01 00:00:00 +01' bad\n+check_parse '1970-01-01 00:00:00 +11' bad\n+check_parse '1970-01-01 00:59:59 +01' bad\n+check_parse '1970-01-01 01:00:00 +01' '1970-01-01 01:00:00 +0100'\n+check_parse '1970-01-01 01:00:00 +11' bad\n+check_parse '1970-01-02 00:00:00 +11' '1970-01-02 00:00:00 +1100'\n+check_parse '1969-12-31 23:59:59' bad\n+check_parse '1969-12-31 23:59:59 +00' bad\n+check_parse '1969-12-31 23:59:59 Z' bad\n+check_parse '1969-12-31 23:59:59 +11' bad\n+check_parse '1969-12-31 23:59:59 -11' bad\n+\n+REQUIRE_64BIT_TIME=HAVE_64BIT_TIME\n+check_parse '2099-12-31 23:59:59' '2099-12-31 23:59:59 +0000'\n+check_parse '2099-12-31 23:59:59 +00' '2099-12-31 23:59:59 +0000'\n+check_parse '2099-12-31 23:59:59 Z' '2099-12-31 23:59:59 +0000'\n+check_parse '2099-12-31 23:59:59 +01' '2099-12-31 23:59:59 +0100'\n+check_parse '2099-12-31 23:59:59 -01' bad\n+check_parse '2099-12-31 23:59:59 -11' bad\n+check_parse '2099-12-31 23:00:00 -01' bad\n+check_parse '2099-12-31 22:59:59 -01' '2099-12-31 22:59:59 -0100'\n+check_parse '2100-00-00 00:00:00' bad\n+check_parse '2099-12-30 00:00:00 -11' '2099-12-30 00:00:00 -1100'\n+check_parse '2100-00-00 00:00:00 +00' bad\n+check_parse '2100-00-00 00:00:00 Z' bad\n+check_parse '2100-00-00 00:00:00 -11' bad\n+check_parse '2100-00-00 00:00:00 +11' bad\n+REQUIRE_64BIT_TIME=\n+\n check_approxidate() {\n \techo \"$1 -> $2 +0000\" >expect\n \ttest_expect_${3:-success} \"parse approxidate ($1)\" \"\n-- \n2.45.2-796-g2ef7a3d713\n\n"},{"id":"497667","messageId":"CAPig+cRaQbWTLnvbx-Yec3mGEJDOQQr8RX+bT=J1FzVDqjq2_g@mail.gmail.com","threadId":"61681","inReplyTo":"20240625231248.4070257-2-gitster@pobox.com","subject":"Re: [PATCH v4 1/2] t0006: simplify prerequisites","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-06-25T23:30:12Z","receivedAt":"2024-06-25T23:30:24Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jun 25, 2024 at 7:13 PM Junio C Hamano <gitster@pobox.com> wrote:\n> The system must support 64-bit time and its time_t must be 64-bit\n> wide to pass these tests.  Combine these two prerequisites together\n> to simplify the tests.  In theory, they could be fulfilled\n> independently and tests could require only one without the other,\n> but in practice, but in practice these must come hand-in-hand.\n\ns/but in practice, but in practice/but in practice/\n\n> Update the \"check_parse\" test helper to pay attention to the\n> REQUIRE_64BIT_TIME variable, which can be set to the HAVE_64BIT_TIME\n> prerequisite so that a parse test can be skipped on 32-bit systems.\n> This will be used in the next step to skip tests for timestamps near\n> the end of year 2099, as 32-bit systems will not be able to express\n> a timestamp beyond 2038 anyway.\n>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n"},{"id":"497668","messageId":"xmqq5xtw603e.fsf@gitster.g","threadId":"61681","inReplyTo":"CAPig+cRaQbWTLnvbx-Yec3mGEJDOQQr8RX+bT=J1FzVDqjq2_g@mail.gmail.com","subject":"Re: [PATCH v4 1/2] t0006: simplify prerequisites","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-06-26T00:04:21Z","receivedAt":"2024-06-26T00:04:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> On Tue, Jun 25, 2024 at 7:13 PM Junio C Hamano <gitster@pobox.com> wrote:\n>> The system must support 64-bit time and its time_t must be 64-bit\n>> wide to pass these tests.  Combine these two prerequisites together\n>> to simplify the tests.  In theory, they could be fulfilled\n>> independently and tests could require only one without the other,\n>> but in practice, but in practice these must come hand-in-hand.\n>\n> s/but in practice, but in practice/but in practice/\n\nThanks, always, for your sharp eyes.\n"},{"id":"497705","messageId":"dedaa590-b9bf-498f-b743-ba058cba8486@gmail.com","threadId":"61681","inReplyTo":"20240625231248.4070257-1-gitster@pobox.com","subject":"Re: [PATCH v4 0/2] Darcy's \"date underflow fix\" topic, final reroll","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-06-26T15:21:01Z","receivedAt":"2024-06-26T15:21:04Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio\n\nOn 26/06/2024 00:12, Junio C Hamano wrote:\n> So it has been some time since we discussed this topic.  Let's clean\n> up the messy \"SQUASH???\" patches I had to queue on top of the main\n> patch to keep the CI working and make them into a preliminary patch.\n> \n> The tree at the end of the series is identical to what has been\n> queued in 'seen' for the past few weeks.  The only difference is\n> that we first lay groundwork to skip certain time-parsing tests on\n> 32-bit systems first, and then use Darcy's patch with minimum\n> adjustments for 32-bit systems.\n\nI've had a read through this verison and I don't have anything to add to \nEric's comments.\n\nBest Wishes\n\nPhillip\n\n\n> Darcy Burke (1):\n>    date: detect underflow/overflow when parsing dates with timezone offset\n> \n> Junio C Hamano (1):\n>    t0006: simplify prerequisites\n> \n>   date.c          | 12 +++++++++++-\n>   t/t0006-date.sh | 51 +++++++++++++++++++++++++++++++++++++++++++------\n>   2 files changed, 56 insertions(+), 7 deletions(-)\n> \n"},{"id":"497717","messageId":"xmqqle2rzh9o.fsf@gitster.g","threadId":"61681","inReplyTo":"dedaa590-b9bf-498f-b743-ba058cba8486@gmail.com","subject":"Re: [PATCH v4 0/2] Darcy's \"date underflow fix\" topic, final reroll","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-06-26T18:32:51Z","receivedAt":"2024-06-26T18:32:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> Hi Junio\n>\n> On 26/06/2024 00:12, Junio C Hamano wrote:\n>> So it has been some time since we discussed this topic.  Let's clean\n>> up the messy \"SQUASH???\" patches I had to queue on top of the main\n>> patch to keep the CI working and make them into a preliminary patch.\n>> The tree at the end of the series is identical to what has been\n>> queued in 'seen' for the past few weeks.  The only difference is\n>> that we first lay groundwork to skip certain time-parsing tests on\n>> 32-bit systems first, and then use Darcy's patch with minimum\n>> adjustments for 32-bit systems.\n>\n> I've had a read through this verison and I don't have anything to add\n> to Eric's comments.\n>\n> Best Wishes\n\nThanks, let's mark it for 'next' then.\n"}]}