golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.13k stars 17.68k forks source link

os: Stdin is broken in some cases on windows #15388

Open yekimov opened 8 years ago

yekimov commented 8 years ago

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)? go version go1.6 windows/amd64
  2. What operating system and processor architecture are you using (go env)? set GOARCH=amd64 set GOOS=windows
  3. What did you do? I tried to make a Squid (a proxy server) auth helper with Go. It's a simple program that needs to read it's stdin, parse it and send the response to stdout. See http://wiki.squid-cache.org/Features/AddonHelpers for details. Here is the link to my code: https://play.golang.org/p/NR9PkX0fu7
  4. What did you expect to see? My squid instance need to ask credentials upon http request. No error messages related to my program need to be in squid log.
  5. What did you see instead? Every thing went fine. I tested the program from console, I tested it with echo, sending login and pass by pipe, like 'echo vasya 1 | sqauth'. No errors detected. Then, I configured Squid to run the program as an auth helper. Squid didn't start. In log file I saw this record: 'read /dev/stdin: The parameter is incorrect.', thre record from line 26 of my code. I spent few hours investigating the problem, and that's what I got. It seems that Squid starts its child processes, and sets its stdin to async socket handlers. It's not pretty good documented in WINAPI doc, but when they run ReadFile on async IO handler with NULL lpOverlapped parameter (it's the last parameter to function), the error 87 happens (The parameter is incorrect). I looked go source code, and I saw that 'File.read' function calls syscall.ReadFile with lpOverlapped == nil in any case.

Thus that the case, when I start go program as children process with stdin as async socket, os.Stdin is always broken and I have no normal way to get data from stdin. Instead I need to use platform dependend syscall. It would be pretty good to use standard os.Stdin in any case. It's interesting that C fgets works fine with normal stdin in the case.

The working C programm: fake.cc.zip.

yekimov commented 8 years ago

os.Stdout has the same problem with the corresponding call syscall.WriteFile when the handle is represented by socked. This works for me when stdin and stdout are sockets, and when they are not: https://play.golang.org/p/OiFtFkhBgp

alexbrainman commented 8 years ago

@yekimov

Thank you very much for your report. I suspect we can fix this.

But before we decide how to fix this, we need a new test. This test will, probably, live in "os" package. The test will demonstrate the problem - so it will create async socket (or whatever other media that work similarly), then start a child process with stdin / stdout set to the socket, and then get child process read and write its stdin / stdout.

The test will, obviously, have to fail on current tip. But then we could make whatever adjustments to standard library to make that test pass.

If you want to take on that task, this https://golang.org/doc/contribute.html is how to contribute. You could put your new test in $GOROOT/src/os/os_windows_test.go, because I think you would have to use Windows APIs for the test.

Alex

mattn commented 8 years ago

I don't understand what happen to go.

package main

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

func main() {
    l, err := net.Listen("tcp", ":8888")
    if err != nil {
        log.Fatal(err)
    }
    conn, err := l.Accept()
    if err != nil {
        log.Fatal(err)
    }
    cmd := exec.Command("sqauth")
    cmd.Stdin = conn
    cmd.Stdout = os.Stdout
    cmd.Run()
}

This works fine for me. Could you show me your simple code for the producer?

yekimov commented 8 years ago

Sure, take it. https://play.golang.org/p/CqC5BI8V8G The clue word is not "socket", but "ASYNC socket". The sixth arg of WINAPI CreateFile is the source of troubles with os.File on windows.

mattn commented 8 years ago

I got below

got input: vasya 0
case l == 2
PASED
got input: vasya 0
case l == 2
FORBIDEN
got input: vasya 0
case l == 2
FORBIDEN
got input: vasya 0
case l == 2
FORBIDEN
got input: vasya 0
case l == 2
FORBIDEN
...

First line seems okay. And second or later line seems getting EOF.

UPDATE

My result is generated by following code.

package main

import (
    "os"
    "os/exec"
    "syscall"
)

func main() {
    // Don't forget to create some file with content, 'vasya 0' should work fine.
    ptr, err := syscall.UTF16PtrFromString("some_file_with_content.txt")
    if err != nil {
        panic(err)
    }
    h, err := syscall.CreateFile(
        ptr,
        syscall.GENERIC_READ, /*|syscall.GENERIC_WRITE*/
        syscall.FILE_SHARE_READ,
        nil,
        syscall.OPEN_EXISTING,
        syscall.FILE_ATTRIBUTE_NORMAL, /*0x40000000*/ // Here is the rouch! Strike him with zero value =)
        0)
    if err != nil {
        panic(err)
    }
    cmd := exec.Command("sqauth")
    cmd.Stdin = os.NewFile(uintptr(h), "my cool async file")
    cmd.Stderr = os.Stderr
    cmd.Run()
    err = syscall.CloseHandle(h)
    if err != nil {
        panic(err)
    }
}
yekimov commented 8 years ago

Are you tolling me? Why did you comment argument 0x40000000? It's a key of the trouble.

mattn commented 8 years ago

What mean of 0x40000000?

bradfitz commented 8 years ago

MSDN CreateFile ... https://msdn.microsoft.com/en-us/library/windows/desktop/aa363858(v=vs.85).aspx searching that page for "0x40000000" finds:

FILE_FLAG_OVERLAPPED ... 0x40000000 ... "The file or device is being opened or created for asynchronous I/O."

Why does the example have that comment? FILE_ATTRIBUTE_NORMAL is defined as 0x00000080.

yekimov commented 8 years ago

FILE_FLAG_OVERLAPPED 0x40000000 The file or device is being opened or created for asynchronous I/O. When subsequent I/O operations are completed on this handle, the event specified in the OVERLAPPED structure will be set to the signaled state. If this flag is specified, the file can be used for simultaneous read and write operations. If this flag is not specified, then I/O operations are serialized, even if the calls to the read and write functions specify an OVERLAPPED structure. For information about considerations when using a file handle created with this flag, see the Synchronous and Asynchronous I/O Handles section of this topic.

Source: https://msdn.microsoft.com/en-us/library/windows/desktop/aa363858(v=vs.85).aspx

mattn commented 8 years ago

Ah, sorry. I've commmet bad. I'll try this again.

mattn commented 8 years ago

I could reproduce read /dev/stdin: The parameter is incorrect. And it may fix with bufio.

cmd.Stdin = bufio.NewReader(os.NewFile(uintptr(h), "my cool async file"))

I don't mean that this is not an issue.

yekimov commented 8 years ago

Do I need to make tests, as @alexbrainman recommends? Actualy I already starded, but not sure when I can finish.