Skip to content
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: Stdin is broken in some cases on windows #15388

Open
yekimov opened this issue Apr 20, 2016 · 12 comments
Open

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

yekimov opened this issue Apr 20, 2016 · 12 comments
Milestone

Comments

@yekimov
Copy link

yekimov commented Apr 20, 2016

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.

@ianlancetaylor ianlancetaylor changed the title os.Stdin is broken in some cases on windows os: Stdin is broken in some cases on windows Apr 20, 2016
@ianlancetaylor ianlancetaylor modified the milestones: Go1.7, Unplanned Apr 20, 2016
@yekimov
Copy link
Author

yekimov commented Apr 21, 2016

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
Copy link
Member

@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
Copy link
Member

mattn commented Apr 26, 2016

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
Copy link
Author

yekimov commented Apr 26, 2016

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
Copy link
Member

mattn commented Apr 26, 2016

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
Copy link
Author

yekimov commented Apr 27, 2016

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

@mattn
Copy link
Member

mattn commented Apr 27, 2016

What mean of 0x40000000?

@bradfitz
Copy link
Contributor

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
Copy link
Author

yekimov commented Apr 27, 2016

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
Copy link
Member

mattn commented Apr 27, 2016

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

@mattn
Copy link
Member

mattn commented Apr 27, 2016

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
Copy link
Author

yekimov commented Apr 27, 2016

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

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

No branches or pull requests

5 participants