net/http: don't reject 0-lengthed bodies with Expect 100-continue

I was implementing rules from RFC 2616. The rules are apparently useless,
ambiguous, and too strict for common software on the Internet. (e.g. curl)

Add more tests, including a test of a chunked request.

Fixes #7625

LGTM=dsymonds
R=golang-codereviews, dsymonds
CC=adg, golang-codereviews, rsc
https://golang.org/cl/84480045

diff --git a/src/pkg/net/http/serve_test.go b/src/pkg/net/http/serve_test.go
index c1ea537d85a02abd113bfdf68438b30c3c859723..625d379c26a20a7c3615b42bcfad5c74c69d9ebe 100644 (file)
--- a/src/pkg/net/http/serve_test.go
+++ b/src/pkg/net/http/serve_test.go
@@ -933,31 +933,50 @@ func TestTLSServer(t *testing.T) {
 }
 
 type serverExpectTest struct {
-       contentLength    int    // of request body
+       contentLength    int // of request body
+       chunked          bool
        expectation      string // e.g. "100-continue"
        readBody         bool   // whether handler should read the body (if false, sends StatusUnauthorized)
        expectedResponse string // expected substring in first line of http response
 }
 
+func expectTest(contentLength int, expectation string, readBody bool, expectedResponse string) serverExpectTest {
+       return serverExpectTest{
+               contentLength:    contentLength,
+               expectation:      expectation,
+               readBody:         readBody,
+               expectedResponse: expectedResponse,
+       }
+}
+
 var serverExpectTests = []serverExpectTest{
        // Normal 100-continues, case-insensitive.
-       {100, "100-continue", true, "100 Continue"},
-       {100, "100-cOntInUE", true, "100 Continue"},
+       expectTest(100, "100-continue", true, "100 Continue"),
+       expectTest(100, "100-cOntInUE", true, "100 Continue"),
 
        // No 100-continue.
-       {100, "", true, "200 OK"},
+       expectTest(100, "", true, "200 OK"),
 
        // 100-continue but requesting client to deny us,
        // so it never reads the body.
-       {100, "100-continue", false, "401 Unauthorized"},
+       expectTest(100, "100-continue", false, "401 Unauthorized"),
        // Likewise without 100-continue:
-       {100, "", false, "401 Unauthorized"},
+       expectTest(100, "", false, "401 Unauthorized"),
 
        // Non-standard expectations are failures
-       {0, "a-pony", false, "417 Expectation Failed"},
+       expectTest(0, "a-pony", false, "417 Expectation Failed"),
 
-       // Expect-100 requested but no body
-       {0, "100-continue", true, "400 Bad Request"},
+       // Expect-100 requested but no body (is apparently okay: Issue 7625)
+       expectTest(0, "100-continue", true, "200 OK"),
+       // Expect-100 requested but handler doesn't read the body
+       expectTest(0, "100-continue", false, "401 Unauthorized"),
+       // Expect-100 continue with no body, but a chunked body.
+       {
+               expectation:      "100-continue",
+               readBody:         true,
+               chunked:          true,
+               expectedResponse: "100 Continue",
+       },
 }
 
 // Tests that the server responds to the "Expect" request header
@@ -986,21 +1005,38 @@ func TestServerExpect(t *testing.T) {
 
                // Only send the body immediately if we're acting like an HTTP client
                // that doesn't send 100-continue expectations.
-               writeBody := test.contentLength > 0 && strings.ToLower(test.expectation) != "100-continue"
+               writeBody := test.contentLength != 0 && strings.ToLower(test.expectation) != "100-continue"
 
                go func() {
+                       contentLen := fmt.Sprintf("Content-Length: %d", test.contentLength)
+                       if test.chunked {
+                               contentLen = "Transfer-Encoding: chunked"
+                       }
                        _, err := fmt.Fprintf(conn, "POST /?readbody=%v HTTP/1.1\r\n"+
                                "Connection: close\r\n"+
-                               "Content-Length: %d\r\n"+
+                               "%s\r\n"+
                                "Expect: %s\r\nHost: foo\r\n\r\n",
-                               test.readBody, test.contentLength, test.expectation)
+                               test.readBody, contentLen, test.expectation)
                        if err != nil {
                                t.Errorf("On test %#v, error writing request headers: %v", test, err)
                                return
                        }
                        if writeBody {
+                               var targ io.WriteCloser = struct {
+                                       io.Writer
+                                       io.Closer
+                               }{
+                                       conn,
+                                       ioutil.NopCloser(nil),
+                               }
+                               if test.chunked {
+                                       targ = httputil.NewChunkedWriter(conn)
+                               }
                                body := strings.Repeat("A", test.contentLength)
-                               _, err = fmt.Fprint(conn, body)
+                               _, err = fmt.Fprint(targ, body)
+                               if err == nil {
+                                       err = targ.Close()
+                               }
                                if err != nil {
                                        if !test.readBody {
                                                // Server likely already hung up on us.

diff --git a/src/pkg/net/http/server.go b/src/pkg/net/http/server.go
index 273d5964f14e2c308649f28b662736c8f6a9970f..6b94167aef58a6d482b505798d58c895ab1dff93 100644 (file)
--- a/src/pkg/net/http/server.go
+++ b/src/pkg/net/http/server.go
@@ -1158,16 +1158,10 @@ func (c *conn) serve() {
                // Expect 100 Continue support
                req := w.req
                if req.expectsContinue() {
-                       if req.ProtoAtLeast(1, 1) {
+                       if req.ProtoAtLeast(1, 1) && req.ContentLength != 0 {
                                // Wrap the Body reader with one that replies on the connection
                                req.Body = &expectContinueReader{readCloser: req.Body, resp: w}
                        }
-                       if req.ContentLength == 0 {
-                               w.Header().Set("Connection", "close")
-                               w.WriteHeader(StatusBadRequest)
-                               w.finishRequest()
-                               break
-                       }
                        req.Header.Del("Expect")
                } else if req.Header.get("Expect") != "" {
                        w.sendExpectationFailed()