Comment 1:

It seems a bit more complex than I had initially thought.
I managed to reproduce the issue by placing an hg.bat (not .exe) file in a directory on
Path, and a cmd.exe in the current directory. (I originally had hg.exe on Path; in that
case hg.exe is invoked directly, without involving cmd, so the issue is not triggered.)
I believe that always using %Comspec% /c hg should still resolve the issue, but ideally
there should be a solution that fixes just the problematic case (with .bat files), so
I'm looking into the mechanism in more detail.

Comment 2:

This seems mostly unrelated to cmd/go; apparently invoking a batch file with os/exec
will use cmd.exe in cwd if available. I have attached a simple example to reproduce.
1. ...\> go build cmdtest.go
   creates cmdtest.exe
2. ...\> cmdtest.exe
   observe output "hello" from hello.bat
3. ...\> go build cmd.go
   creates cmd.exe that returns error on invocation
4. ...\> cmdtest.exe
   observe error from newly created cmd.exe
I'm just unsure whether this is the "normal" Windows behavior or something specific to
how invoking commands is handled in Go. I will keep on digging.

Attachments:

1. 5716_1.zip (617 bytes)

Comment 3:

The environment I used for checking is:
XP/32bit with SP3
go version devel +56909cb770fe Fri Jun 21 18:07:57 2013 -0700 windows/386

Comment 4:

I found another potentially problematic case related to this issue: when ".PY" is in
%PATHEXT%, exec.Command (exec.LookPath) finds .PY files on %PATH%, but exec.Start cannot
execute them. While the docs don't explicitly guarantee that this should work, it seems
a reasonable assumption (cmd/go seems to assume this for example).
To summarize the symptoms:
(S1) exec.Start doesn't work with program paths found by exec.LookPath with non-standard
file extensions (e.g. .PY)
(S2) When invoking batch files with exec.Start, the first cmd.exe found in cwd / PATH is
used instead of %ComSpec%
My analysis:
Go on Windows uses the CreateProcess API [0] to invoke programs (via
syscall.StartProcess [1]). The behavior of CreateProcess for each file type seems to be:
- binary files: works
- batch files: the docs state that the command interpreter with /C should be specified,
but it seems to include a workaround to add cmd /C for batch files, so that it doesn't
fail even when the caller omits the command interpreter. Unfortunately this workaround
uses cmd instead of %ComSpec%, hence (S2)
- other files: (e.g. .PY) fails, hence (S1)
[0] http://msdn.microsoft.com/en-us/library/windows/desktop/ms682425.aspx
[1] http://tip.golang.org/src/pkg/syscall/exec_windows.go#L330
The solution that I believe should work in general is to prepend %ComSpec% /C when
invoking "non-binary" files, either in syscall.StartProcess, or in os/exec (Start or
maybe Command). Although I'm not quite sure how to decide whether the command is an
executable binary.
Do you think this is a reasonable approach to pursue? I have to add that I don't
normally program a lot on Windows, so I may be missing other important factors.

Comment 5:

speter.go1,
The issue says "cmd/go: use %Comspec% instead of `cmd` when executing go get". I don't
see that cmd/go uses `cmd` when executing go get. If not that, then what is the problem
we are trying to fix? Thank you.
Alex

Comment 6:

Thanks for looking at this. You're right, after all there doesn't seem to be anything
specifically wrong with cmd/go. (It was one symptom, not the cause; it just took me a
while to figure that out.) Sorry for the confusion.
The problem / symptom that was related to cmd/go is what I refer to as (S2) above in #4:
(S2) When invoking batch files with exec.Start, the first cmd.exe found in cwd / PATH is
used instead of %ComSpec%
More specifically, the problem was that go get was trying to invoke "cmd /C hg" using an
arbitrary cmd.exe in the PATH instead of %ComSpec%. I think invoking an arbitrary
cmd.exe in cwd or on the PATH can in the better case cause a great deal of confusion,
and in the worse case serious security issues.
And another problem / symptom that I think is related to this is:
(S1) exec.Start doesn't work with program paths found by exec.LookPath with non-standard
file extensions (e.g. .PY)
My interpretation is that there is an issue with how processes are invoked for
non-binary executables. The docs for CreateProcess [0] state that the command
interpreter with parameter "/C" should be specified when invoking batch files, but this
doesn't happen in the most common flow from exec.Command -> exec.Start ->
syscall.StartProcess. It appears that CreateProcess includes a flaky workaround for this
case, which can end up invoking an arbitrary cmd.exe instead of %ComSpec%, triggering
(S2).
[0] http://msdn.microsoft.com/en-us/library/windows/desktop/ms682425.aspx
In addition, CreateProcess doesn't support other types of executables (e.g. .PY) that
may be specified with PATHEXT and file type association, so even though exec.Command
"finds" the command, exec.Start fails to run it. This again is another case when
exec.Command -> exec.Start -> syscall.StartProcess attempts an unsupported kind of
invocation using CreateProcess (except that CreateProcess doesn't even have a flaky
workaround for this one).
I think for non-binary executables the command interpreter (%ComSpec%) should be
explicitly specified when invoking CreateProcess. This could be added to one of
syscall.StartProcess, os/exec.Start, or os/exec.Command (but the latter won't fix
programs that create exec.Cmd directly).

Comment 7:

speter.go1,
> ... there doesn't seem to be anything specifically wrong with cmd/go. ...
That is fine. I will close the issue then.
> ... the problem was that go get was trying to invoke "cmd /C hg" using an arbitrary
cmd.exe ...
You keep saying that. I am yet to see it. I don't believe "go get" command invokes
cmd.exe in any circumstances.
> ... exec.Start doesn't work with program paths found by exec.LookPath with
non-standard file extensions (e.g. .PY)
We do what Windows CreateProcess provides. I think it is inclusive enough. Someone who
needs to do something fancier, should provide appropriate command. For example, call
python interpreter (or whatever it is called) directly.
Alex

Status changed to Retracted.

Comment 8:

Alex,
Sorry for taking time to respond.
> > ... the problem was that go get was trying to invoke "cmd /C hg" using an arbitrary
cmd.exe ...
> 
> You keep saying that. I am yet to see it. I don't believe "go get" command invokes
cmd.exe in any circumstances.
"Trying to invoke" was not the right expression, sorry. It doesn't explicitly specify
"cmd" but apparently it ends up invoking it anyway in some cases.
It has happened for the OP in the golang-nuts thread linked at the top, and I too could
reproduce it when hg is installed as a batch file. (Note that for instance "pip install
mercurial" installs the hg command as a batch file; pip is one of the standard
installers for Python software.) It can be confirmed using ProcessManager, or by placing
a bogus cmd.exe in cwd or on the PATH before %ComSpec%.
In #2 I also provided a minimal example showing that invoking a .bat file using
exec.Start causes cmd.exe in cwd (rather than %ComSpec%) to be used for invoking the
batch file.
The same can happen in go get. My biggest concern is that it may be possible to create a
repository that exploits this and makes go get invoke arbitrary code in some cases. (I
can try to create one if that is what it takes for this to be accepted as an issue.)
Peter

Comment 9:

Peter,
> ... It doesn't explicitly specify "cmd" but apparently it ends up invoking it anyway
in some cases.
It happens because that is how Windows CreateProcess works. I said it before, I am
repeating it again. Whatever Windows CreateProcess does cannot be "wrong". Can it?
> ... as a batch file. ...
Sure. But that is how Windows CreateProcess treats batch files. If you have problem with
that, you should take it up with Microsoft.
I wouldn't use batch files to invoke Mercurial anyway. Batch files are trouble at times.
For example, can you determine if Mercurial command has failed or not, if you are
calling it via batch file?
> ... In #2 I also provided a minimal example showing that invoking a .bat file using
exec.Start causes cmd.exe in cwd (rather than %ComSpec%) to be used for invoking the
batch file.
Same as above.
> ... My biggest concern is that it may be possible to create a repository that exploits
this and makes go get invoke arbitrary code in some cases. (I can try to create one if
that is what it takes for this to be accepted as an issue.)
"go get" does not execute code that is not *installed* by you. Does it?
Feel free to create a new issue if you like. Alternatively, I am happy to reopen this
issue, if you insist. I just don't understand what you expect us to do. Perhaps other
gophers do. Sorry.
Alex