Subject: Re: [PATCH v2] date: detect underflow when parsing dates with positive timezone offset

From: darcy <[email-masked]>
Date: Mon, 3 Jun 2024 21:44:53 +1000

> Without having a deep understanding of the code I don't quite see the
> connection between this change and the problem description. Is it
> necessary? If so, it might help to explain why it's needed in the commit
> message or in the code.

> ...I guess that
> checking for tm_hour is assuming that TZ offset should always come
> before the values necessary to compute the timestamp comes, but it
> smells like an unwarranted assumption and not explaining the change
> in the proposed log message is a bad sign.

This line has been reverted. The point was it would only parse the
timezone offset if it occurs after the time part of the date, but I
have realized that this is unrelated to the purpose of this change.

> You should also add at least one test.

Yep, thanks, added now to `t0006-date.sh`.

> Do we also want to check for overflows in the other direction (a large timestamp with a negative timezone offset)?

Is this something people want added? I am happy to implement this if
so, though it wasn't my original intention.

Issues with the commit message should also be resolved. Thank you
everyone for your patience :)

On 3/6/24 21:13, Junio C Hamano wrote:

> "darcy via GitGitGadget" <[email-masked]> writes:
>
>>      fix: prevent date underflow when using positive timezone offset
>>      
>>      cc: Patrick Steinhardt [email-masked] cc: Phillip Wood
>>      [email-masked]
> You're expected to respond to review comments before you send in
> updated patches.  I didn't see the review comments responded to in
> the thread:
>
>    https://lore.kernel.org/git/[email-masked]/
>
> Please see "A typical life cycle of a patch series" section of the
> SubmittingPatches document.
>
>    https://git.github.io/htmldocs/SubmittingPatches.html#patch-flow
>
> Step #3 and Step #4 are distinct.