http: consume request bodies before replying

This fixes our http behavior (even if Handlers forget to
consume a request body, we do it for them before we send
their response header), fixes the racy TestServerExpect,
and adds TestServerConsumesRequestBody.

With GOMAXPROCS>1, the http tests now seem race-free.

R=rsc
CC=golang-dev
https://golang.org/cl/4419042

diff --git a/src/pkg/http/serve_test.go b/src/pkg/http/serve_test.go
index 0142dead9f21b57f136b677b372e9da3d86d286e..eb1ecfdd32c3322b173233b9a2fec4f69539b4ba 100644 (file)
--- a/src/pkg/http/serve_test.go
+++ b/src/pkg/http/serve_test.go
@@ -588,7 +588,7 @@ func TestServerExpect(t *testing.T) {
                sendf := func(format string, args ...interface{}) {
                        _, err := fmt.Fprintf(conn, format, args...)
                        if err != nil {
-                               t.Fatalf("Error writing %q: %v", format, err)
+                               t.Fatalf("On test %#v, error writing %q: %v", test, format, err)
                        }
                }
                go func() {
@@ -616,3 +616,49 @@ func TestServerExpect(t *testing.T) {
                runTest(test)
        }
 }
+
+func TestServerConsumesRequestBody(t *testing.T) {
+       log := make(chan string, 100)
+
+       ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
+               log <- "got_request"
+               w.WriteHeader(StatusOK)
+               log <- "wrote_header"
+       }))
+       defer ts.Close()
+
+       conn, err := net.Dial("tcp", ts.Listener.Addr().String())
+       if err != nil {
+               t.Fatalf("Dial: %v", err)
+       }
+       defer conn.Close()
+
+       bufr := bufio.NewReader(conn)
+       gotres := make(chan bool)
+       go func() {
+               line, err := bufr.ReadString('\n')
+               if err != nil {
+                       t.Fatal(err)
+               }
+               log <- line
+               gotres <- true
+       }()
+
+       size := 1 << 20
+       log <- "writing_request"
+       fmt.Fprintf(conn, "POST / HTTP/1.0\r\nContent-Length: %d\r\n\r\n", size)
+       time.Sleep(25e6) // give server chance to misbehave & speak out of turn
+       log <- "slept_after_req_headers"
+       conn.Write([]byte(strings.Repeat("a", size)))
+
+       <-gotres
+       expected := []string{
+               "writing_request", "got_request",
+               "slept_after_req_headers", "wrote_header",
+               "HTTP/1.0 200 OK\r\n"}
+       for step, e := range expected {
+               if g := <-log; e != g {
+                       t.Errorf("on step %d expected %q, got %q", step, e, g)
+               }
+       }
+}

diff --git a/src/pkg/http/server.go b/src/pkg/http/server.go
index 3291de1017fcc8d0c7d9d2c3c46b8388706c0516..aa4dc294224c04436ee0eab47c59a06f715dbf6f 100644 (file)
--- a/src/pkg/http/server.go
+++ b/src/pkg/http/server.go
@@ -141,9 +141,13 @@ func newConn(rwc net.Conn, handler Handler) (c *conn, err os.Error) {
 type expectContinueReader struct {
        resp       *response
        readCloser io.ReadCloser
+       closed     bool
 }
 
 func (ecr *expectContinueReader) Read(p []byte) (n int, err os.Error) {
+       if ecr.closed {
+               return 0, os.NewError("http: Read after Close on request Body")
+       }
        if !ecr.resp.wroteContinue && !ecr.resp.conn.hijacked {
                ecr.resp.wroteContinue = true
                io.WriteString(ecr.resp.conn.buf, "HTTP/1.1 100 Continue\r\n\r\n")
@@ -153,6 +157,7 @@ func (ecr *expectContinueReader) Read(p []byte) (n int, err os.Error) {
 }
 
 func (ecr *expectContinueReader) Close() os.Error {
+       ecr.closed = true
        return ecr.readCloser.Close()
 }
 
@@ -196,6 +201,16 @@ func (w *response) WriteHeader(code int) {
                log.Print("http: multiple response.WriteHeader calls")
                return
        }
+
+       // Per RFC 2616, we should consume the request body before
+       // replying, if the handler hasn't already done so.
+       if w.req.ContentLength != 0 {
+               ecr, isExpecter := w.req.Body.(*expectContinueReader)
+               if !isExpecter || ecr.resp.wroteContinue {
+                       w.req.Body.Close()
+               }
+       }
+
        w.wroteHeader = true
        w.status = code
        if code == StatusNotModified {