# RE: [PATCH 3/3] verify_path: consider dos drive prefix 4 messages from 2011-06-08 to 2011-06-08. Participants: Theo Niessink, Erik Faye-Lund. Thread: https://gitlist.dev/t/27576 ## Theo Niessink, 2011-06-08 09:55 Subject: RE: [PATCH 3/3] verify_path: consider dos drive prefix Message-ID: URL: https://gitlist.dev/e/DAFDEB13CE4944C18AAF3F20994AEF2E%40martinic.local ``` 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; ``` ## Erik Faye-Lund, 2011-06-08 10:45 Subject: Re: [PATCH 3/3] verify_path: consider dos drive prefix Message-ID: URL: https://gitlist.dev/e/BANLkTinTJh9oOTHi5js7n-sy8%2BpmMa_vtQ%40mail.gmail.com In-Reply-To: ``` On Wed, Jun 8, 2011 at 11:55 AM, Theo Niessink 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? ``` ## Theo Niessink, 2011-06-08 12:04 Subject: RE: [PATCH 3/3] verify_path: consider dos drive prefix Message-ID: <187961B7EDE84635BD7B3C9A4EF688C3@martinic.local> URL: https://gitlist.dev/e/187961B7EDE84635BD7B3C9A4EF688C3%40martinic.local In-Reply-To: ``` Erik Faye-Lund wrote: > This looks obviously correct to me. Thanks for spotting the problem. > > Would you mind writing up a commit-message and supply a sign-off? Like this you mean? -- >8 -- Subject: [PATCH] verify_dotfile(): do not assume '/' is the path seperator verify_dotfile() currently assumes that the path seperator is '/', but on Windows it can also be '\\', so use is_dir_sep() instead. Signed-off-by: Theo Niessink --- read-cache.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) 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; -- 1.7.5.3776.g5dcaf.dirty ``` ## Erik Faye-Lund, 2011-06-08 12:15 Subject: Re: [PATCH 3/3] verify_path: consider dos drive prefix Message-ID: URL: https://gitlist.dev/e/BANLkTik6mi002mWArJiLcC4rnvmotf5%2B2A%40mail.gmail.com In-Reply-To: <187961B7EDE84635BD7B3C9A4EF688C3@martinic.local> ``` On Wed, Jun 8, 2011 at 2:04 PM, Theo Niessink wrote: > Erik Faye-Lund wrote: >> This looks obviously correct to me. Thanks for spotting the problem. >> >> Would you mind writing up a commit-message and supply a sign-off? > > Like this you mean? Just like that, indeed! Junio, what do you think, is this OK? I'd be great if we could have this applied upstream, and then rebase-merge our branch on top for the next Git for Windows release. ```