net/http: document ResponseWriter and Handler more; add test

Update docs on ResponseWriter and Handler around concurrency.

Also add a test.

The Handler docs were old and used "object" a lot. It was also too
ServeMux-centric.

Fixes #13050
Updates #13659 (new issue found in http2 while writing the test)

Change-Id: I25f53d5fa54f1c9d579d3d0f191bf3d94b1a251b
Reviewed-on: https://go-review.googlesource.com/17982
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>

2 files changed, 71 insertions, 8 deletions

diff --git a/src/net/http/clientserver_test.go b/src/net/http/clientserver_test.go
index ac25a04c0d..14b0783d3a 100644
--- a/src/net/http/clientserver_test.go
+++ b/src/net/http/clientserver_test.go
@@ -619,3 +619,59 @@ func testResponseBodyReadAfterClose(t *testing.T, h2 bool) {
 		t.Fatalf("ReadAll returned %q, %v; want error", data, err)
 	}
 }
+
+func TestConcurrentReadWriteReqBody_h1(t *testing.T) { testConcurrentReadWriteReqBody(t, h1Mode) }
+func TestConcurrentReadWriteReqBody_h2(t *testing.T) {
+	t.Skip("known failing; golang.org/issue/13659")
+	testConcurrentReadWriteReqBody(t, h2Mode)
+}
+func testConcurrentReadWriteReqBody(t *testing.T, h2 bool) {
+	defer afterTest(t)
+	const reqBody = "some request body"
+	const resBody = "some response body"
+	cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) {
+		var wg sync.WaitGroup
+		wg.Add(2)
+		didRead := make(chan bool, 1)
+		// Read in one goroutine.
+		go func() {
+			defer wg.Done()
+			data, err := ioutil.ReadAll(r.Body)
+			if string(data) != reqBody {
+				t.Errorf("Handler read %q; want %q", data, reqBody)
+			}
+			if err != nil {
+				t.Errorf("Handler Read: %v", err)
+			}
+			didRead <- true
+		}()
+		// Write in another goroutine.
+		go func() {
+			defer wg.Done()
+			if !h2 {
+				// our HTTP/1 implementation intentionally
+				// doesn't permit writes during read (mostly
+				// due to it being undefined); if that is ever
+				// relaxed, fix this.
+				<-didRead
+			}
+			io.WriteString(w, resBody)
+		}()
+		wg.Wait()
+	}))
+	defer cst.close()
+	req, _ := NewRequest("POST", cst.ts.URL, strings.NewReader(reqBody))
+	req.Header.Add("Expect", "100-continue") // just to complicate things
+	res, err := cst.c.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	data, err := ioutil.ReadAll(res.Body)
+	defer res.Body.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(data) != resBody {
+		t.Errorf("read %q; want %q", data, resBody)
+	}
+}
diff --git a/src/net/http/server.go b/src/net/http/server.go
index f6428bcf18..8a854f03b9 100644
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -36,26 +36,33 @@ var (
 	ErrContentLength   = errors.New("Conn.Write wrote more than the declared Content-Length")
 )
 
-// Objects implementing the Handler interface can be
-// registered to serve a particular path or subtree
-// in the HTTP server.
+// A Handler responds to an HTTP request.
 //
 // ServeHTTP should write reply headers and data to the ResponseWriter
-// and then return.  Returning signals that the request is finished
-// and that the HTTP server can move on to the next request on
-// the connection.
+// and then return. Returning signals that the request is finished; it
+// is not valid to use the ResponseWriter or read from the
+// Request.Body after or concurrently with the completion of the
+// ServeHTTP call.
+//
+// Depending on the HTTP client software, HTTP protocol version, and
+// any intermediaries between the client and the Go server, it may not
+// be possible to read from the Request.Body after writing to the
+// ResponseWriter. Cautious handlers should read the Request.Body
+// first, and then reply.
 //
 // If ServeHTTP panics, the server (the caller of ServeHTTP) assumes
 // that the effect of the panic was isolated to the active request.
 // It recovers the panic, logs a stack trace to the server error log,
 // and hangs up the connection.
-//
 type Handler interface {
 	ServeHTTP(ResponseWriter, *Request)
 }
 
 // A ResponseWriter interface is used by an HTTP handler to
 // construct an HTTP response.
+//
+// A ResponseWriter may not be used after the Handler.ServeHTTP method
+// has returned.
 type ResponseWriter interface {
 	// Header returns the header map that will be sent by
 	// WriteHeader. Changing the header after a call to
@@ -1546,7 +1553,7 @@ func (w *response) CloseNotify() <-chan bool {
 // The HandlerFunc type is an adapter to allow the use of
 // ordinary functions as HTTP handlers.  If f is a function
 // with the appropriate signature, HandlerFunc(f) is a
-// Handler object that calls f.
+// Handler that calls f.
 type HandlerFunc func(ResponseWriter, *Request)
 
 // ServeHTTP calls f(w, r).