From e27702545ac2d1e5f545fa6b3e39dbcf36bdb023 Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@golang.org>
Date: Thu, 14 Apr 2011 10:40:23 -0700
Subject: [PATCH] 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
---
 src/pkg/http/serve_test.go | 48 +++++++++++++++++++++++++++++++++++++-
 src/pkg/http/server.go     | 15 ++++++++++++
 2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/src/pkg/http/serve_test.go b/src/pkg/http/serve_test.go
index 0142dead9f2..eb1ecfdd32c 100644
--- 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 3291de1017f..aa4dc294224 100644
--- 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 {