New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os/exec: Pipe of command's stdout to network connection not working on windows #22278

Closed
tiago4orion opened this Issue Oct 15, 2017 · 19 comments

Comments

Projects
None yet
7 participants
@tiago4orion

tiago4orion commented Oct 15, 2017

What did you do?

I'm trying to pipe the output of a program to a network connection. The code below works on Linux/android/osx:

package main

import (
	"log"
	"net"
	"os"
	"os/exec"
)

func main() {
	conn, err := net.Dial("tcp", "localhost:8089")
	if err != nil {
		log.Fatal(err)
	}

	defer func() {
		err := conn.Close()
		if err != nil {
			log.Fatal(err)
		}
	}()

	cmd := exec.Command("echo", "hello world")
	cmd.Stdout = conn
	cmd.Stderr = os.Stderr
	cmd.Stdin = os.Stdin

	err = cmd.Run()
	if err != nil {
		log.Fatal(err)
	}
}

What did you expect to see?

I expect the string "hello world" sent to tcp://localhost:8089, as works in linux,

What did you see instead?

Nothing is sent.

System details

go version go1.9.1 windows/amd64
GOARCH="amd64"
GOBIN=""
GOEXE=".exe"
GOHOSTARCH="amd64"
GOHOSTOS="windows"
GOOS="windows"
GOPATH="C:\Users\Tiago\go"
GORACE=""
GOROOT="C:\Go"
GOTOOLDIR="C:\Go\pkg\tool\windows_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-m64 -mthreads -fmessage-length=0"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOROOT/bin/go version: go version go1.9.1 windows/amd64
GOROOT/bin/go tool compile -V: compile version go1.9.1

@tiago4orion tiago4orion changed the title from os/exec: Windows StdoutPipe not working with network connection to os/exec: Pipe of command's stdout to network connection not working on windows Oct 15, 2017

@as

This comment has been minimized.

Contributor

as commented Oct 15, 2017

Echo is not a program, it is a shell built-in. Your other systems are overloaded with binary versions of echo. So they execute that.

In this rare case, Windows is simpler than Linux and OSX. It does not have an echo binary, so your example doesnt run.

You should see an error message indicating the echo program cant be found. Can you locate such a message in your console?

@tiago4orion

This comment has been minimized.

tiago4orion commented Oct 15, 2017

Sorry, but I do have an echo binary. You can experiment with other binaries, same result.

@as

This comment has been minimized.

Contributor

as commented Oct 15, 2017

Unable to reproduce. Where do you have an echo binary?

@tiago4orion

This comment has been minimized.

tiago4orion commented Oct 15, 2017

https://github.com/c0defellas/enzo/blob/master/cmd/echo/echo.go

This problem was found in the Nash shell when redirecting output of commands to network address:

$ ls >[1] tcp://localhost:6666

This works on Linux/osx/android but not in windows... The program used doesn't matter.

@as Did you ran the code and the output of the program was sent? What version of Go?

@elizar

This comment has been minimized.

elizar commented Oct 15, 2017

@tiago4orion have you tried cmd := exec.Command("cmd /c echo", "hello world")?

@tiago4orion

This comment has been minimized.

tiago4orion commented Oct 16, 2017

@elizar

2017/10/15 22:43:09 exec: "cmd /c echo": file does not exist

But using cmd := exec.Command("cmd", "/c", "echo", "hello world") does the interesting result below:

The process tried to write to a nonexistent pipe.

and nothing is sent.. This message apparently came from cmd.exe.

@as

This comment has been minimized.

Contributor

as commented Oct 16, 2017

This might be a poller issue, but we need to confirm a few things first.

Have you tested this with prior versions of Go?
If so, when did it start to break?
Does it break in a standard windows environment?

Although it's nice to see a shell written in Go, let's not use any more Go programs other than a short reproducible example to isolate the cause without confounding variables.

@tiago4orion

This comment has been minimized.

tiago4orion commented Oct 16, 2017

Have you tested this with prior versions of Go?

no

Does it break in a standard windows environment?

yes: https://ci.appveyor.com/project/tiago4orion/nash/build/1.0.0.44#L244

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Oct 19, 2017

@tiago4orion I can reproduce your problem, thank you very much.

The problem is that os/exec.Cmd passes socket to a child process stdin. We create copy of the socket handle before passing it into CreateProcess Windows API with DuplicateHandle Windows API. But DuplicateHandle is not allowed to be used with sockets - see https://msdn.microsoft.com/en-us/library/windows/desktop/ms724251(v=vs.85).aspx:

You should not use DuplicateHandle to duplicate handles to the following objects:
I/O completion ports. No error is returned, but the duplicate handle cannot be used.
Sockets. No error is returned, but the duplicate handle may not be recognized by Winsock at the target process. Also, using DuplicateHandle interferes with internal reference counting on the underlying object. To duplicate a socket handle, use the WSADuplicateSocket function.

The simplest way to fix this issue would be to insert pipe in between child and parent, and have goroutine copy data from one end of the pipe to other. In fact this is already happening in os/exec.Cmd.stdin, but that code is not used if Cmd.Stdin is a *os.File (sockets are *os.File). So if we can separate sockets from other os.File there, we should be able to fix this easy.

@ianlancetaylor (or anyone else) what is the easiest way to determine if *os.File is a socket?

Thank you

Alex

@mattn

This comment has been minimized.

Member

mattn commented Oct 19, 2017

AFAIK, WSAEnumNetworkEvents return error when passing non-socket handle.

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Oct 20, 2017

I've lost track of where the actual failing code is. How did we get a socket that is a *os.File?

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Oct 20, 2017

I've lost track of where the actual failing code is. How did we get a socket that is a *os.File?

I am talking about this code https://github.com/golang/go/blob/master/src/os/exec/exec.go#L207

When c.Stdin is a socket, we return it on line 208 as is. That makes our code pass socket handle to child process (that is not allowed, because Windows DuplicateHandle does not work for sockets). What we want to do instead is to continue to line 211 that creates a pipe to pass to the child instead and start goroutine that feeds that pipe.

How should I change the expression on line 207 to achieve that?

Alex

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Oct 20, 2017

But a socket is not normally a *os.File. It's normally a net.TCPConn or something like that. In order to (try to) answer your question, I'm trying to understand how we got a socket with type *os.File.

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Oct 20, 2017

I'm trying to understand how we got a socket with type *os.File.

I have no computer in front of me. I will try to debug this again later and report back. Thank you.

Alex

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Oct 22, 2017

I was completely wrong here #22278 (comment) - I misread the original issue. The problem is that we pass pipe (returned by os.Pipe) into net.sendFile. On linux this call fails with "invalid argument", so our code just does slow version of TCPConn.readFrom. On windows net.sendFile calls TransmitFile Windows API, and TransmitFile actually copies some data from the pipe into tcp connection, but then TransmitFile returns success after some timeout or something - I could not find a way to control how long TransmitFile waits before returning. I suspect that TransmitFile is to be used with files only, not with pipes. So we need to find a way to not to call TransmitFile with pipes. Suggestions are welcome.

Alex

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Oct 23, 2017

Is there any way that internal/poll/sendfile_windows.go can determine what src is? I see that Windows has a GetFileType function, which can return FILE_TYPE_PIPE. Would that be sufficient?

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Oct 24, 2017

I see that Windows has a GetFileType function, which can return FILE_TYPE_PIPE

That should probably work, thank you very much. @ianlancetaylor you could work at Microsoft now. I will report back with my findings.

Alex

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Nov 24, 2017

@tiago4orion can you try https://go-review.googlesource.com/#/c/go/+/79775 to see if it fixes your problem?

Thank you

Alex

@gopherbot

This comment has been minimized.

gopherbot commented Nov 24, 2017

Change https://golang.org/cl/79775 mentions this issue: internal/poll: do not use Windows TransmitFile with pipes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment