Re: [PATCH v5 1/1] worktree add: sanitize worktree names

[Eric Sunshine <sunshine@sunshineco.com>]

On Fri, Mar 8, 2019 at 4:28 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> Worktree names are based on $(basename $GIT_WORK_TREE). They aren't
> significant until 3a3b9d8cde (refs: new ref types to make per-worktree
> refs visible to all worktrees - 2018-10-21), where worktree name could
> be part of a refname and must follow refname rules.
>
> Update 'worktree add' code to remove special characters to follow
> these rules. In the future the user will be able to specify the
> worktree name by themselves if they're not happy with this dumb
> character substitution.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/refs.c b/refs.c
> @@ -72,30 +72,57 @@ static unsigned char refname_disposition[256] = {
> +static int check_refname_component(const char *refname, int *flags,
> +                                  struct strbuf *sanitized)
>  {
>         for (cp = refname; ; cp++) {
>                 unsigned char disp = refname_disposition[ch];
> +               if (sanitized && disp != 1)
> +                       strbuf_addch(sanitized, ch);
> +
>                 switch (disp) {
>                 case 1:
>                         goto out;
>                 case 2:
> +                       if (last == '.') { /* Refname contains "..". */
> +                               if (sanitized)
> +                                       sanitized->len--; /* collapse ".." to single "." */

I think this needs to be:

    strbuf_setlen(sanitized, sanitized->len - 1);

to ensure that NUL-terminator ends up in the correct place if this "."
is the very last character in 'refname'. (Otherwise, the NUL will
remain after the second ".", thus ".." won't be collapsed to "." at
all.)

> +                               else
> +                                       return -1;
> +                       }
>                         break;