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

net/http: inconsistent results using os/exec in handler #5660

Closed
gopherbot opened this issue Jun 7, 2013 · 6 comments
Closed

net/http: inconsistent results using os/exec in handler #5660

gopherbot opened this issue Jun 7, 2013 · 6 comments
Labels
FrozenDueToAge Suggested Issues that may be good for new contributors looking for work to do.
Milestone

Comments

@gopherbot
Copy link

by cgmurray:

What steps will reproduce the problem?
Run the attached program. See comment in main.go for instructions.

What is the expected output?
Consistent md5-checksum result

What do you see instead?
Random md5-checksum result

Which operating system are you using?
OSX 10.8.3

Which version are you using?  (run 'go version')
go version go1.1 darwin/amd64

Please provide any additional information below.
I've found two workarounds:

I) Change "Content-Type" to "_ontent-Type"
II) Set "Connection: close" header

Attachments:

  1. main.go (770 bytes)
@rsc
Copy link
Contributor

rsc commented Jun 10, 2013

Comment 1:

Labels changed: added priority-later, removed priority-triage.

Status changed to Accepted.

@bradfitz
Copy link
Contributor

Comment 2:

I can reproduce on a Mac.
If I either buffer the input or buffer the output, it passes.  (See attached x.go)   It
only produces inconsistent results if it streams in and streams out, which the net/http
package (nor HTTP, really) permits.
But in this case, the md5 binary shouldn't produce any output until its input's EOF, so
I think this should work.
I suspected maybe that the exec package's io.Copy to the rw from its internal os.Pipe
was causing a 0-byte write or something which forced the HTTP package to shut down its
input, but that doesn't appear to be the case.

Labels changed: added suggested.

Owner changed to @bradfitz.

Attachments:

  1. x.go (1225 bytes)

@rsc
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 3:

Labels changed: added go1.2.

@bradfitz
Copy link
Contributor

bradfitz commented Aug 7, 2013

Comment 4:

The problem is os/exec's io.Copy(ResponseWriter, os.Pipe) and finding that the
ResponseWriter implements io.ReaderFrom.
The net/http (*response).ReadFrom immediately then calls WriteHeader and flushes any
buffered writes, in prep to do a sendfile. 
func (w *response) ReadFrom(src io.Reader) (n int64, err error) {
        if !w.wroteHeader {
                w.WriteHeader(StatusOK)
        }
  ...
        w.w.Flush()  // get rid of any previous writes                                                                                             
        w.cw.flush() // make sure Header is written; flush data to rwc                                                                             
Writing the Header & sending it to the client causes the request.Body to be closed
(which md5 is still reading from).

@bradfitz
Copy link
Contributor

bradfitz commented Aug 7, 2013

Comment 5:

Frowned-upon patch at https://golang.org/cl/12632043/

@bradfitz
Copy link
Contributor

bradfitz commented Aug 8, 2013

Comment 6:

This issue was closed by revision de04bf2.

Status changed to Fixed.

@gopherbot gopherbot added fixed Suggested Issues that may be good for new contributors looking for work to do. labels Aug 8, 2013
@rsc rsc added this to the Go1.2 milestone Apr 14, 2015
@rsc rsc removed the go1.2 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Suggested Issues that may be good for new contributors looking for work to do.
Projects
None yet
Development

No branches or pull requests

3 participants