From: Fabian Stelzer <fs@gigacodes.de>
Date: Thu, 30 Dec 2021 10:25:48 GMT
Subject: Re: [PATCH] gpg-interface: trim CR from ssh-keygen -Y find-principals
Message-ID: <20211230102548.q6ugqiyicswfx24v@fs>
In-Reply-To: <CABPYr=xfotWvTQK9k1eKHa0kP4SsB=TKKuM0d8cpMb5BtuUZLA@mail.gmail.com>

On 09.12.2021 17:58, Pedro Martelletto wrote:
>On Thu, Dec 9, 2021 at 5:33 PM Fabian Stelzer <fs@gigacodes.de> wrote:
>
>> On 06.12.2021 10:06, Pedro Martelletto wrote:
>> >On Sun, Dec 5, 2021 at 6:50 AM Junio C Hamano <gitster@pobox.com> wrote:
>> >
>> >> So instead of the posted patch, we should do something along this
>> >> line instead?
>> >>
>> >>         trust_size = strcspn(line, "\n"); /* truncate at LF */
>> >>         if (trust_size && line[trust_size - 1] == '\r')
>> >>                 trust_size--; /* the LF was part of CRLF at the end */
>> >>
>> >>
>> >>
>> >I agree that's a more consistent fix. A minor nit: if the intention is to
>> >only trim CR as part of a CRLF sequence, we need to ensure a LF is found:
>> >
>>
>> This shouldn't be necessary as we split/loop by LF just above.
>>
>> for (line = ssh_principals_out.buf; *line;
>>       line = strchrnul(line + 1, '\n')) {
>>         while (*line == '\n')
>>                 line++;
>>         if (!*line)
>>                 break;
>>
>>         trust_size = strcspn(line, "\n");
>>         principal = xmemdupz(line, trust_size);
>>
>
>The loop ensures that 'line' points to the first character of
>ssh_principals_out.buf or to a non-NUL character after a '\n'. It does not
>ensure that that 'line' contains a '\n', e.g:
>"principalA\nprincipalB\nprincipalC\r" or just "principalA\r".
>

Just saw that this is still open. @pedro: do you want to send an updated 
version of your patch or would you like me to pick this up and send one?