Re: [PATCH 1/1] rebase -r: let `label` generate safer labels

From: Matt Rogers <mattr94@gmail.com>
Date: Sep 3, 2019, 22:40 UTC
Message-ID: <CAOjrSZsFeGsuNM2v=fPMnbHJH27Z6NU3UQrgoWt-peXjzWsD+w@mail.gmail.com>
In-Reply-To: <xmqq5zm9taza.fsf@gitster-ct.c.googlers.com>

I agree that the code locally was simple enough.

Ultimately I feel that sanitizing and uniqueifying the label should probably be done closer together/at the same place. I'm just not familiar enough with the codebase to know a good place (if any) to move that to. Eventually though this would still need to be expanded further to protect against reserved filenames (e.g. NUL on windows). Although the behavior around these (espescially with file extensions like NUL.txt) become less reliable, and although they are much more unlikely to be encountered in practice, are still allowed by git as oneliners.

On Tue, Sep 3, 2019 at 3:51 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 22 quoted lines

>
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > If you care deeply about double dashes and leading dashes, how about
> > this instead?
> >
> >               char *from, *to;
> >
> >               for (from = to = label.buf; *from; from++)
> >                       if ((*from & 0x80) || isalnum(*from))
> >                               *(to++) = *from;
> >                       /* avoid leading dash and double-dashes */
> >                       else if (to != label.buf && to[-1] != '-')
> >                               *(to++) = '-';
> >               strbuf_setlen(&label, to - label.buf);
>
> Simple enough and is a good change when judged locally.
>
> It still would cause readers to wonder if label_oid() later append
> '-%d' to end up with double-dash near the end, etc., which made me
> wonder if the resulting code becomes better if sanitization and
> uniquefying are done at the same single place in the other message.

-- 
Matthew Rogers