Subject: Re: [PATCH v2 3/4] parse_commit(): handle broken whitespace-only timestamp

From: Phillip Wood <[email-masked]>
Date: Wed, 26 Apr 2023 15:44:08 +0100

Hi Andreas

On 26/04/2023 15:31, Andreas Schwab wrote:
> On Apr 26 2023, Phillip Wood wrote:
> 
>> On 25/04/2023 17:06, Junio C Hamano wrote:
>>> Phillip Wood <[email-masked]> writes:
>>>
>>>> This probably doesn't matter in practice but we define our own
>>>> isspace() that does not treat '\v' and '\f' as whitespace. However
>>>> parse_timestamp() (which is just strtoumax()) uses the standard
>>>> library's isspace() which does treat those characters as whitespace
>>>> and is locale dependent. This means we can potentially stop at a
>>>> character that parse_timestamp() treats as whitespace and if there are
>>>> no digits after it we'll still walk past the end of the line. Using
>>>> Rene's suggestion of testing the character with isdigit() would fix
>>>> that. It would also avoid parsing negative timestamps as positive
>>>> numbers
>>
>>>> and reject any timestamps that begin with a locale dependent
>>>> digit.
>>
>> Sorry, that bit is not correct, I've since checked the C standard and I
>> think strtoul() and friends expect ascii digits (isdigit() and isxdigit()
>> are also locale independent unlike isspace(), isalpha() etc.)
> 
> The standard says:
> 
>      In other than the "C" locale, additional locale-specific subject
>      sequence forms may be accepted.

Thanks, looking at the standard again I don't know how I managed to miss 
that, my initial recollection was correct after all.

Best Wishes

Phillip