Re: GIT_ASKPASS absolute path detection bug on Windows

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

 



On Sun, Mar 22, 2020 at 04:59:15PM +0000, brian m. carlson wrote:
> On 2020-03-22 at 11:44:33, András Kucsma wrote:
> > My proposal patch is to take advantage of find_last_dir_sep function's
> > OS specific directory separator knowledge.
> > I posted the diff below, which is also available on github here:
> > https://github.com/git/git/compare/maint...r0mai:fix-prepare_cmd-windows-maint
> >
> > diff --git a/run-command.c b/run-command.c
> > index f5e1149f9b..9fcc12ebf9 100644
> > --- a/run-command.c
> > +++ b/run-command.c
> > @@ -421,12 +421,12 @@ static int prepare_cmd(struct argv_array *out,
> > const struct child_process *cmd)
> >      }
> >
> >      /*
> > -     * If there are no '/' characters in the command then perform a path
> > -     * lookup and use the resolved path as the command to exec.  If there
> > -     * are '/' characters, we have exec attempt to invoke the command
> > -     * directly.
> > +     * If there are no dir separator characters in the command then perform
> > +     * a path lookup and use the resolved path as the command to exec. If
> > +     * there are dir separator characters, we have exec attempt to invoke
> > +     * the command directly.
> >       */
> > -    if (!strchr(out->argv[1], '/')) {
> > +    if (find_last_dir_sep(out->argv[1]) == NULL) {
> >          char *program = locate_in_PATH(out->argv[1]);
>
> This function (locate_in_PATH) specifically says it is not to be used on
> Windows because it doesn't work properly there due to file extensions.

My reading is, that it dows work if you specify "foo.exe", "foo.bat".
And when you specify "foo" it may use "foo.exe", but there may be
a shell script called "foo".
But I may be wrong.

> I'm pretty sure a proper solution would involve touching that as well,
> although your solution does indeed fix the issue you reported.  That
> function also uses a colon-separated PATH, which I'm not sure will work
> in all cases on Windows (although maybe it will).
>
> From looking at this earlier, I think the problem here is that we're
> trying to use the Unix codepaths (on Cygwin) and then expecting those to
> handle Windows-style paths, which they aren't intended to do.  This is
> likely one of many problems on Cygwin.

Yes and no.
C:/MyTool.BAT is a valid Windows file name even under Windows.
Cygwin preferres /cygdrive/cMyTool.BAT
Git under Cygwin should handle C:/MyTool.BAT correctly, and to my
understanding it does.

Some interesting reading can be found here:
https://docs.microsoft.com/en-us/dotnet/standard/io/file-path-formats#path-normalization

> --
> brian m. carlson: Houston, Texas, US
> OpenPGP: https://keybase.io/bk2204






[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