From 10a273196b5aed2d72d50573bfc5c0bdb2e631a2 Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@golang.org>
Date: Thu, 10 Apr 2014 22:25:31 -0700
Subject: [PATCH] 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
---
 src/pkg/net/http/serve_test.go | 62 +++++++++++++++++++++++++++-------
 src/pkg/net/http/server.go     |  8 +----
 2 files changed, 50 insertions(+), 20 deletions(-)

diff --git a/src/pkg/net/http/serve_test.go b/src/pkg/net/http/serve_test.go
index c1ea537d85..625d379c26 100644
--- 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 273d5964f1..6b94167aef 100644
--- 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()
-- 
2.50.0