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 ac25a04..14b0783 100644
--- a/src/net/http/clientserver_test.go
+++ b/src/net/http/clientserver_test.go
@@ -619,3 +619,59 @@
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 f6428bc..8a854f0 100644
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -36,26 +36,33 @@
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 @@
// 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).
