From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Wed, 08 Jun 2011 10:45:28 GMT
Subject: Re: [PATCH 3/3] verify_path: consider dos drive prefix
Message-ID: <BANLkTinTJh9oOTHi5js7n-sy8+pmMa_vtQ@mail.gmail.com>
In-Reply-To: <DAFDEB13CE4944C18AAF3F20994AEF2E@martinic.local>

On Wed, Jun 8, 2011 at 11:55 AM, Theo Niessink <niessink@martinic.com> wrote:
> Junio C Hamano wrote:
>> Here is what I queued last night. If it looks Ok then I'll merge it down
>> to 'next'.
>
> I have run a couple of quick tests, and everything seems OK, except the
> following backslashed paths, which are verified OK while they should be
> rejected:
>
> foo\.\bar
> foo\..\bar
>
> This is caused by verify_dotfile(), which doesn't use is_dir_sep(). So I
> propose this patch on verify_dotfile():
>
> diff --git a/read-cache.c b/read-cache.c
> index 282c0c1..72be7cd 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -726,11 +726,12 @@ static int verify_dotfile(const char *rest)
>         * has already been discarded, we now test
>         * the rest.
>         */
> -       switch (*rest) {
> +
>        /* "." is not allowed */
> -       case '\0': case '/':
> +       if (*rest == '\0' || is_dir_sep(*rest))
>                return 0;
>
> +       switch (*rest) {
>        /*
>         * ".git" followed by  NUL or slash is bad. This
>         * shares the path end test with the ".." case.
> @@ -743,7 +744,7 @@ static int verify_dotfile(const char *rest)
>                rest += 2;
>        /* fallthrough */
>        case '.':
> -               if (rest[1] == '\0' || rest[1] == '/')
> +               if (rest[1] == '\0' || is_dir_sep(rest[1]))
>                        return 0;
>        }
>        return 1;
>
>
>

This looks obviously correct to me. Thanks for spotting the problem.

Would you mind writing up a commit-message and supply a sign-off?