From 3761c4ae98ddc23554d61813dbd688c7318737bf Mon Sep 17 00:00:00 2001
From: Will McCutchen <will@mccutch.org>
Date: Mon, 10 Jul 2023 14:07:15 -0400
Subject: [PATCH] Add tests for correct handling of Expect: 100-continue (#138)

Issue #129 prompted me to look into how go-httpbin actually handles
incoming requests that specify an `Expect: 100-continue` header. Here we
add additional unit tests to explicitly codify the, uh, _expected_
behavior for those requests.

Note that the Go stdlib http client and server implementations have
automagical `Expect: 100-continue` handling, which means
these particular tests need to be written at a slightly lower level, by
directly writing to and reading from the underlying TCP connection.

Now, it may well be that the the moving parts (e.g. load balancers)
between a client and the instances of go-httpbin deployed at
https://httpbingo.org intervene and change this behavior, but these
tests give me some confidence that, at least when such requests are made
directly to go-httpbin itself, we are correctly handling them.
---
 httpbin/handlers_test.go | 171 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 169 insertions(+), 2 deletions(-)

diff --git a/httpbin/handlers_test.go b/httpbin/handlers_test.go
index 56e3b35..bef3d5f 100644
--- a/httpbin/handlers_test.go
+++ b/httpbin/handlers_test.go
@@ -451,6 +451,7 @@ func testRequestWithBody(t *testing.T, verb, path string) {
 		testRequestWithBodyBinaryBody,
 		testRequestWithBodyBodyTooBig,
 		testRequestWithBodyEmptyBody,
+		testRequestWithBodyExpect100Continue,
 		testRequestWithBodyFormEncodedBody,
 		testRequestWithBodyFormEncodedBodyNoContentType,
 		testRequestWithBodyHTML,
@@ -570,6 +571,172 @@ func testRequestWithBodyHTML(t *testing.T, verb, path string) {
 	assert.BodyContains(t, resp, data)
 }
 
+func testRequestWithBodyExpect100Continue(t *testing.T, verb, path string) {
+	// The stdlib http client automagically handles 100 Continue responses
+	// by continuing the request until a "final" 200 OK response is
+	// received, which prevents us from confirming that a 100 Continue
+	// response is sent when using the http client directly.
+	//
+	// So, here we instead manally write the request to the wire in two
+	// steps, confirming that we receive a 100 Continue response before
+	// sending the body and getting the normal expected response.
+
+	t.Run("non-zero content-length okay", func(t *testing.T) {
+		t.Parallel()
+
+		conn, err := net.Dial("tcp", srv.Listener.Addr().String())
+		assert.NilError(t, err)
+		defer conn.Close()
+
+		body := []byte("test body")
+
+		req := newTestRequestWithBody(t, verb, path, bytes.NewReader(body))
+		req.Header.Set("Expect", "100-continue")
+		req.Header.Set("Content-Type", "text/plain")
+
+		reqBytes, _ := httputil.DumpRequestOut(req, false)
+		t.Logf("raw request:\n%q", reqBytes)
+
+		if !strings.Contains(string(reqBytes), "Content-Length: 9") {
+			t.Fatalf("expected request to contain Content-Length header")
+		}
+
+		// first, we write the request line and headers -- but NOT the body --
+		// which should cause the server to respond with a 100 Continue
+		// response.
+		{
+			n, err := conn.Write(reqBytes)
+			assert.NilError(t, err)
+			assert.Equal(t, n, len(reqBytes), "incorrect number of bytes written")
+
+			resp, err := http.ReadResponse(bufio.NewReader(conn), req)
+			assert.NilError(t, err)
+			assert.StatusCode(t, resp, http.StatusContinue)
+		}
+
+		// Once we've gotten the 100 Continue response, we write the body. After
+		// that, we should get a normal 200 OK response along with the expected
+		// result.
+		{
+			n, err := conn.Write(body)
+			assert.NilError(t, err)
+			assert.Equal(t, n, len(body), "incorrect number of bytes written")
+
+			resp, err := http.ReadResponse(bufio.NewReader(conn), req)
+			assert.NilError(t, err)
+			assert.StatusCode(t, resp, http.StatusOK)
+
+			got := must.Unmarshal[bodyResponse](t, resp.Body)
+			assert.Equal(t, got.Data, string(body), "incorrect body")
+		}
+	})
+
+	t.Run("transfer-encoding:chunked okay", func(t *testing.T) {
+		t.Parallel()
+
+		conn, err := net.Dial("tcp", srv.Listener.Addr().String())
+		assert.NilError(t, err)
+		defer conn.Close()
+
+		body := []byte("test body")
+
+		reqParts := []string{
+			fmt.Sprintf("%s %s HTTP/1.1", verb, path),
+			"Host: test",
+			"Content-Type: text/plain",
+			"Expect: 100-continue",
+			"Transfer-Encoding: chunked",
+		}
+		reqBytes := []byte(strings.Join(reqParts, "\r\n") + "\r\n\r\n")
+		t.Logf("raw request:\n%q", reqBytes)
+
+		// first, we write the request line and headers -- but NOT the body --
+		// which should cause the server to respond with a 100 Continue
+		// response.
+		{
+			n, err := conn.Write(reqBytes)
+			assert.NilError(t, err)
+			assert.Equal(t, n, len(reqBytes), "incorrect number of bytes written")
+
+			resp, err := http.ReadResponse(bufio.NewReader(conn), nil)
+			assert.NilError(t, err)
+			assert.StatusCode(t, resp, http.StatusContinue)
+		}
+
+		// Once we've gotten the 100 Continue response, we write the body. After
+		// that, we should get a normal 200 OK response along with the expected
+		// result.
+		{
+			// write chunk size
+			_, err := conn.Write([]byte("9\r\n"))
+			assert.NilError(t, err)
+
+			// write chunk data
+			n, err := conn.Write(append(body, "\r\n"...))
+			assert.NilError(t, err)
+			assert.Equal(t, n, len(body)+2, "incorrect number of bytes written")
+
+			// write empty terminating chunk
+			_, err = conn.Write([]byte("0\r\n\r\n"))
+			assert.NilError(t, err)
+
+			resp, err := http.ReadResponse(bufio.NewReader(conn), nil)
+			assert.NilError(t, err)
+			assert.StatusCode(t, resp, http.StatusOK)
+
+			got := must.Unmarshal[bodyResponse](t, resp.Body)
+			assert.Equal(t, got.Data, string(body), "incorrect body")
+		}
+	})
+
+	t.Run("zero content-length ignored", func(t *testing.T) {
+		// The Go stdlib's Expect:100-continue handling requires either a a)
+		// non-zero Content-Length header or b) Transfer-Encoding:chunked
+		// header to be present.  Otherwise, the Expect header is ignored and
+		// the request is processed normally.
+		t.Parallel()
+
+		conn, err := net.Dial("tcp", srv.Listener.Addr().String())
+		assert.NilError(t, err)
+		defer conn.Close()
+
+		req := newTestRequest(t, verb, path)
+		req.Header.Set("Expect", "100-continue")
+
+		reqBytes, _ := httputil.DumpRequestOut(req, false)
+		t.Logf("raw request:\n%q", reqBytes)
+
+		// For GET and DELETE requests, it appears the Go stdlib does not
+		// include a Content-Length:0 header, so we ensure that the header is
+		// either missing or has a value of 0.
+		switch verb {
+		case "GET", "DELETE":
+			if strings.Contains(string(reqBytes), "Content-Length:") {
+				t.Fatalf("expected no Content-Length header for %s request", verb)
+			}
+		default:
+			if !strings.Contains(string(reqBytes), "Content-Length: 0") {
+				t.Fatalf("expected Content-Length:0 header for %s request", verb)
+			}
+		}
+
+		n, err := conn.Write(reqBytes)
+		assert.NilError(t, err)
+		assert.Equal(t, n, len(reqBytes), "incorrect number of bytes written")
+
+		resp, err := http.ReadResponse(bufio.NewReader(conn), req)
+		assert.NilError(t, err)
+
+		// Note: the server should NOT send a 100 Continue response here,
+		// because we send a request without a Content-Length header or with a
+		// Content-Length: 0 header.
+		assert.StatusCode(t, resp, http.StatusOK)
+
+		got := must.Unmarshal[bodyResponse](t, resp.Body)
+		assert.Equal(t, got.Data, "", "incorrect body")
+	})
+}
+
 func testRequestWithBodyFormEncodedBodyNoContentType(t *testing.T, verb, path string) {
 	params := url.Values{}
 	params.Set("foo", "foo")
@@ -1745,9 +1912,9 @@ func TestDrip(t *testing.T) {
 		assert.NilError(t, err)
 		defer conn.Close()
 
-		n, err := conn.Write(append(reqBytes, []byte("\r\n\r\n")...))
+		n, err := conn.Write(reqBytes)
 		assert.NilError(t, err)
-		assert.Equal(t, n, len(reqBytes)+4, "incorrect number of bytes written")
+		assert.Equal(t, n, len(reqBytes), "incorrect number of bytes written")
 
 		resp, err := http.ReadResponse(bufio.NewReader(conn), req)
 		assert.NilError(t, err)
-- 
GitLab