net/http: don't reuse conns after incomplete 100-continue requests

If we receive an HTTP request with "Expect: 100-continue" and the
Handler never read to EOF, the conn is in an unknown state.
Don't reuse that connection.

Fixes #11549

Change-Id: I5be93e7a54e899d615b05f72bdcf12b25304bc60
Reviewed-on: https://go-review.googlesource.com/12262
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index b28e4063c4010ad9a72df39c6e94711f043c9d5d..345bac5608a1c5ccfb8a8a672527630e009e3249 100644 (file)
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -2991,6 +2991,49 @@ func TestTolerateCRLFBeforeRequestLine(t *testing.T) {
        }
 }
 
+func TestIssue11549_Expect100(t *testing.T) {
+       req := reqBytes(`PUT /readbody HTTP/1.1
+User-Agent: PycURL/7.22.0
+Host: 127.0.0.1:9000
+Accept: */*
+Expect: 100-continue
+Content-Length: 10
+
+HelloWorldPUT /noreadbody HTTP/1.1
+User-Agent: PycURL/7.22.0
+Host: 127.0.0.1:9000
+Accept: */*
+Expect: 100-continue
+Content-Length: 10
+
+GET /should-be-ignored HTTP/1.1
+Host: foo
+
+`)
+       var buf bytes.Buffer
+       conn := &rwTestConn{
+               Reader: bytes.NewReader(req),
+               Writer: &buf,
+               closec: make(chan bool, 1),
+       }
+       ln := &oneConnListener{conn: conn}
+       numReq := 0
+       go Serve(ln, HandlerFunc(func(w ResponseWriter, r *Request) {
+               numReq++
+               if r.URL.Path == "/readbody" {
+                       ioutil.ReadAll(r.Body)
+               }
+               io.WriteString(w, "Hello world!")
+       }))
+       <-conn.closec
+       if numReq != 2 {
+               t.Errorf("num requests = %d; want 2", numReq)
+       }
+       if !strings.Contains(buf.String(), "Connection: close\r\n") {
+               t.Errorf("expected 'Connection: close' in response; got: %s", buf.String())
+       }
+}
+
 func BenchmarkClientServer(b *testing.B) {
        b.ReportAllocs()
        b.StopTimer()

diff --git a/src/net/http/server.go b/src/net/http/server.go
index 882c3521447fab467d225472e76a8367b035e6d2..fda26bad1da7956b59ecdb68c08744ec8fd0e6fb 100644 (file)
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -554,6 +554,7 @@ type expectContinueReader struct {
        resp       *response
        readCloser io.ReadCloser
        closed     bool
+       sawEOF     bool
 }
 
 func (ecr *expectContinueReader) Read(p []byte) (n int, err error) {
@@ -565,7 +566,11 @@ func (ecr *expectContinueReader) Read(p []byte) (n int, err error) {
                ecr.resp.conn.buf.WriteString("HTTP/1.1 100 Continue\r\n\r\n")
                ecr.resp.conn.buf.Flush()
        }
-       return ecr.readCloser.Read(p)
+       n, err = ecr.readCloser.Read(p)
+       if err == io.EOF {
+               ecr.sawEOF = true
+       }
+       return
 }
 
 func (ecr *expectContinueReader) Close() error {
@@ -846,6 +851,22 @@ func (cw *chunkWriter) writeHeader(p []byte) {
                w.closeAfterReply = true
        }
 
+       // If the client wanted a 100-continue but we never sent it to
+       // them (or, more strictly: we never finished reading their
+       // request body), don't reuse this connection because it's now
+       // in an unknown state: we might be sending this response at
+       // the same time the client is now sending its request body
+       // after a timeout.  (Some HTTP clients send Expect:
+       // 100-continue but knowing that some servers don't support
+       // it, the clients set a timer and send the body later anyway)
+       // If we haven't seen EOF, we can't skip over the unread body
+       // because we don't know if the next bytes on the wire will be
+       // the body-following-the-timer or the subsequent request.
+       // See Issue 11549.
+       if ecr, ok := w.req.Body.(*expectContinueReader); ok && !ecr.sawEOF {
+               w.closeAfterReply = true
+       }
+
        // Per RFC 2616, we should consume the request body before
        // replying, if the handler hasn't already done so.  But we
        // don't want to do an unbounded amount of reading here for