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>

diff --git a/src/net/http/clientserver_test.go b/src/net/http/clientserver_test.go
index ac25a04c0da270eff8224299d7a3f52567b6976c..14b0783d3a435e4ec9db4f1a375ebb0912851799 100644 (file)
--- 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 f6428bcf18d6ccbe95514771e06317ceb426f049..8a854f03b923491c06d57bae657c0a7461638197 100644 (file)
--- 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).