Re: [PATCH] worktree add: sanitize worktree names

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Feb 21, 2019 at 6:28 PM Konstantin Kharlamov <hi-angel@xxxxxxxxx> wrote:
>
>
>
> On Чт, Feb 21, 2019 at 2:00 PM,
> =?UTF-8?b?Tmd1eeG7hW4gVGjDoWkgTmfhu41j?= Duy <pclouds@xxxxxxxxx> 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. The code could replace chars with '-' more than
> > necessary, but it keeps the code simple. 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.
> >
> > Reported-by: hi-angel@xxxxxxxxx
> > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> > ---
> >  builtin/worktree.c      | 47
> > ++++++++++++++++++++++++++++++++++++++++-
> >  t/t2025-worktree-add.sh |  5 +++++
> >  2 files changed, 51 insertions(+), 1 deletion(-)
> >
> > diff --git a/builtin/worktree.c b/builtin/worktree.c
> > index 3f9907fcc9..ff36838a33 100644
> > --- a/builtin/worktree.c
> > +++ b/builtin/worktree.c
> > @@ -262,6 +262,46 @@ static void validate_worktree_add(const char
> > *path, const struct add_opts *opts)
> >       free_worktrees(worktrees);
> >  }
> >
> > +/*
> > + * worktree name is part of refname and has to pass
> > + * check_refname_component(). Remove unallowed characters to make it
> > + * valid.
> > + */
> > +static void sanitize_worktree_name(struct strbuf *name)
> > +{
> > +     int i;
> > +
> > +     /* no ending with .lock */
> > +     if (ends_with(name->buf, ".lock"))
> > +             strbuf_remove(name, name->len - strlen(".lock"),
> > +                           strlen(".lock"));
> > +
> > +     /*
> > +      * All special chars replaced with dashes. See
> > +      * check_refname_component() for reference.
> > +      */
> > +     for (i = 0; i < name->len; i++) {
> > +             if (strchr(":?[]\\~ \t@{}*/.", name->buf[i]))
> > +                     name->buf[i] = '-';
> > +     }
> > +
> > +     /* remove consecutive dashes, leading or trailing dashes */
> > +     for (i = 0; i < name->len; i++) {
> > +             while (name->buf[i] == '-' &&
> > +                    (i == 0 ||
> > +                     i == name->len - 1 ||
> > +                     (i < name->len - 1 && name->buf[i + 1] == '-')))
> > +                     strbuf_remove(name, i, 1);
> > +     }
> > +
> > +     /* last resort, should never ever happen in practice */
> > +     if (name->len == 0)
> > +             strbuf_addstr(name, "worktree");
>
> I assume this means a user have passed a zero-sized worktree name? But
> zero-sized file/directory names are not possible anyway, would it make
> sense to just return an error in this case?

It could happen if you do "git worktree add .lock". The ".lock" part
will be stripped out, leaving us with an empty string.
-- 
Duy




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux