Revert: handle single req bytes per Write() call

Partial revert of 8758db0e and b8e2e481

psiphon/common/transforms/httpTransformer.go

@@ -51,9 +51,9 @@ type HTTPTransformerParameters struct {
 }
 
 const (
-	// httpTransformerReadHeader HTTPTransformer is waiting to finish reading
-	// the next HTTP request header.
-	httpTransformerReadHeader = 0
+	// httpTransformerReadWriteHeader HTTPTransformer is waiting to finish
+	// reading and writing the next HTTP request header.
+	httpTransformerReadWriteHeader = 0
 	// httpTransformerReadWriteBody HTTPTransformer is waiting to finish reading
 	// and writing the current HTTP request body.
 	httpTransformerReadWriteBody = 1
@@ -82,205 +82,171 @@ type HTTPTransformer struct {
 	net.Conn
 }
 
-// Warning: Does not handle chunked encoding. Must be called synchronously.
+// Write implements the net.Conn interface.
+//
+// Note: it is assumed that the underlying transport, net.Conn, is a reliable
+// stream transport, i.e. TCP, therefore it is required that the caller stop
+// calling Write() on an instance of HTTPTransformer after an error is returned
+// because, following this assumption, the connection will have failed when a
+// Write() call to the underlying net.Conn fails; a new connection must be
+// established, net.Conn, and wrapped with a new HTTPTransformer. For this
+// reason, the return value may be the number of bytes buffered internally
+// and not the number of bytes written to the underlying net.Conn when a non-nil
+// error is returned.
+//
+// Warning: Does not handle chunked encoding and multiple HTTP requests written
+// in a single Write(). Must be called synchronously.
 func (t *HTTPTransformer) Write(b []byte) (int, error) {
 
-	if t.state == httpTransformerReadHeader {
+	if t.state == httpTransformerReadWriteHeader {
 
 		t.b = append(t.b, b...)
 
 		// Wait until the entire HTTP request header has been read. Must check
 		// all accumulated bytes incase the "\r\n\r\n" separator is written over
-		// multiple Write() calls; from reading the net/http code the entire
-		// HTTP request is written in a single Write() call.
+		// multiple Write() calls; from reading the go1.19.5 net/http code the
+		// entire HTTP request is written in a single Write() call.
 
 		sep := []byte("\r\n\r\n")
 
 		headerBodyLines := bytes.SplitN(t.b, sep, 2) // split header and body
 
-		if len(headerBodyLines) > 1 {
+		if len(headerBodyLines) <= 1 {
+			// b buffered in t.b and the entire HTTP request header has not been
+			// recieved so another Write() call is expected.
+			return len(b), nil
+		} // else: HTTP request header has been read
 
-			// read Content-Length before applying transform
+		// read Content-Length before applying transform
 
-			var headerLines [][]byte
-
-			lines := bytes.Split(headerBodyLines[0], []byte("\r\n"))
-			if len(lines) > 1 {
-				// skip request line, e.g. "GET /foo HTTP/1.1"
-				headerLines = lines[1:]
-			}
+		var headerLines [][]byte
 
-			var cl []byte
-			contentLengthHeader := []byte("Content-Length:")
+		lines := bytes.Split(headerBodyLines[0], []byte("\r\n"))
+		if len(lines) > 1 {
+			// skip request line, e.g. "GET /foo HTTP/1.1"
+			headerLines = lines[1:]
+		}
 
-			for _, header := range headerLines {
+		var cl []byte
+		contentLengthHeader := []byte("Content-Length:")
 
-				if bytes.HasPrefix(header, contentLengthHeader) {
+		for _, header := range headerLines {
 
-					cl = textproto.TrimBytes(header[len(contentLengthHeader):])
-					break
-				}
-			}
-			if len(cl) == 0 {
-				// Irrecoverable error because either Content-Length header
-				// missing, or Content-Length header value is empty, e.g.
-				// "Content-Length: ", and request body length cannot be
-				// determined.
-				//
-				// b buffered in t.b, return len(b) in an attempt to get
-				// through the current Write() sequence instead of getting
-				// stuck.
-				return len(b), errors.TraceNew("Content-Length missing")
-			}
+			if bytes.HasPrefix(header, contentLengthHeader) {
 
-			contentLength, err := strconv.ParseUint(string(cl), 10, 63)
-			if err != nil {
-				// Irrecoverable error because Content-Length is malformed and
-				// request body length cannot be determined.
-				//
-				// b buffered in t.b, return len(b) in an attempt to get
-				// through the current Write() sequence instead of getting
-				// stuck.
-				return len(b), errors.Trace(err)
+				cl = textproto.TrimBytes(header[len(contentLengthHeader):])
+				break
 			}
+		}
+		if len(cl) == 0 {
+			// Irrecoverable error because either Content-Length header
+			// missing, or Content-Length header value is empty, e.g.
+			// "Content-Length: ", and request body length cannot be
+			// determined.
+			return len(b), errors.TraceNew("Content-Length missing")
+		}
 
-			t.remain = contentLength
+		contentLength, err := strconv.ParseUint(string(cl), 10, 63)
+		if err != nil {
+			// Irrecoverable error because Content-Length is malformed and
+			// request body length cannot be determined.
+			return len(b), errors.Trace(err)
+		}
 
-			// transform and write header
+		t.remain = contentLength
 
-			headerLen := len(headerBodyLines[0]) + len(sep)
-			header := t.b[:headerLen]
+		// transform and write header
 
-			if t.transform != nil {
-				newHeaderS, err := t.transform.Apply(t.seed, string(header))
-				if err != nil {
-					// TODO: consider logging an error and skiping transform
-					// instead of returning an error, if the transform is broken
-					// then all subsequent applications may fail.
-					//
-					// b buffered in t.b, return len(b) in an attempt to get
-					// through the current Write() sequence instead of getting
-					// stuck.
-					return len(b), errors.Trace(err)
-				}
+		headerLen := len(headerBodyLines[0]) + len(sep)
+		header := t.b[:headerLen]
 
-				newHeader := []byte(newHeaderS)
+		if t.transform != nil {
+			newHeaderS, err := t.transform.Apply(t.seed, string(header))
+			if err != nil {
+				// TODO: consider logging an error and skiping transform
+				// instead of returning an error, if the transform is broken
+				// then all subsequent applications may fail.
+				return len(b), errors.Trace(err)
+			}
 
-				// only allocate new slice if header length changed
-				if len(newHeader) == len(header) {
-					copy(t.b[:len(header)], newHeader)
-				} else {
-					t.b = append(newHeader, t.b[len(header):]...)
-				}
+			newHeader := []byte(newHeaderS)
 
-				header = newHeader
+			// only allocate new slice if header length changed
+			if len(newHeader) == len(header) {
+				copy(t.b[:len(header)], newHeader)
+			} else {
+				t.b = append(newHeader, t.b[len(header):]...)
 			}
 
-			if math.MaxUint64-t.remain < uint64(len(header)) {
-				// Irrecoverable error because request is malformed:
-				// Content-Length + len(header) > math.MaxUint64.
-				//
-				// b buffered in t.b, return len(b) in an attempt to get
-				// through the current Write() sequence instead of getting
-				// stuck.
-				return len(b), errors.TraceNew("t.remain + uint64(len(header)) overflows")
-			}
-			t.remain += uint64(len(header))
-
-			n, err := t.writeBuffer()
-
-			written := len(b) // all bytes of b buffered in t.b
-
-			if n < len(header) ||
-				len(t.b) > 0 && t.remain == 0 {
-				// All bytes of b were not written, but all bytes of b have been
-				// buffered in t.b. Drop 1 byte of b from t.b to pretend 1 byte
-				// of b was not written to trigger another Write() call. This
-				// handles the scenario where all request bytes have been
-				// received but writing to the underlying net.Conn fails and
-				// another Write() call cannot be expected unless a value
-				// less than len(b) is returned. An alternative solution would
-				// be to retry writes, or spawn a goroutine which writes t.b,
-				// but we want to return the error to the caller immediately so
-				// it can act accordingly.
-				written = len(b) - 1
-				t.b = t.b[:len(t.b)-1]
-			}
+			header = newHeader
+		}
 
-			if t.remain > 0 {
-				t.state = httpTransformerReadWriteBody
-			}
+		if math.MaxUint64-t.remain < uint64(len(header)) {
+			// Irrecoverable error because request is malformed:
+			// Content-Length + len(header) > math.MaxUint64.
+			return len(b), errors.TraceNew("t.remain + uint64(len(header)) overflows")
+		}
+		t.remain += uint64(len(header))
 
-			return written, err
+		err = t.writeBuffer()
+
+		if t.remain > 0 {
+			t.state = httpTransformerReadWriteBody
 		}
 
-		// b buffered in t.b and the entire HTTP request header has not been
-		// recieved so another Write() call is expected.
-		return len(b), nil
+		return len(b), err
 	}
 
 	// HTTP request header has been transformed. Write any remaining bytes of
 	// HTTP request header and then write HTTP request body.
 
 	// Must write buffered bytes first, in-order, to write bytes to underlying
-	// Conn in the same order they were received in.
-	_, err := t.writeBuffer()
+	// net.Conn in the same order they were received in.
+	//
+	// In practise the buffer will be empty by this point because its entire
+	// contents will have been written in the first call to t.writeBuffer()
+	// when the header is received, parsed, and transformed; otherwise the
+	// underlying transport will have failed and the caller will not invoke
+	// Write() again on this instance. See HTTPTransformer.Write() comment.
+	err := t.writeBuffer()
 	if err != nil {
 		// b not written or buffered
-		return 0, errors.Trace(err)
+		return 0, err
 	}
 
-	// Only write bytes of current request
-	writeN := uint64(len(b))
-	if writeN > t.remain {
-		writeN = t.remain
+	if uint64(len(b)) > t.remain {
+		return len(b), errors.TraceNew("multiple HTTP requests in single Write() not supported")
 	}
 
-	n, err := t.Conn.Write(b[:writeN])
+	n, err := t.Conn.Write(b)
 
-	// Do not need to check for underflow because n <= t.remain
 	t.remain -= uint64(n)
 
 	if t.remain <= 0 {
 		// Entire request, header and body, has been written. Return to
 		// waiting for next HTTP request header to arrive.
-		//
-		// Return the number of bytes written to the underlying Conn instead of
-		// calling t.Write() with any remaining bytes of b which were not
-		// written or buffered, i.e. when n < len(b). The caller must call
-		// Write() again with the unwritten, and unbuffered, bytes of b.
-		t.state = httpTransformerReadHeader
+		t.state = httpTransformerReadWriteHeader
 		t.remain = 0
 	}
 
 	return n, err
 }
 
-func (t *HTTPTransformer) writeBuffer() (written int, err error) {
-
-	// Continue writing buffered bytes until either all buffered bytes have
-	// been written or all remaining bytes of the current HTTP request have
-	// been written.
-	for len(t.b) > 0 && t.remain > 0 {
+func (t *HTTPTransformer) writeBuffer() error {
 
-		// Write all buffered bytes of the current request
-		writeN := uint64(len(t.b))
-		if writeN > t.remain {
-			// t.b contains bytes of the next request(s), only write current
-			// request bytes.
-			writeN = t.remain
-		}
+	if uint64(len(t.b)) > t.remain {
+		// Should never happen, multiple requests written in a single
+		// Write() are not supported.
+		return errors.TraceNew("multiple HTTP requests in single Write() not supported")
+	}
 
-		// Check for potential overflow before Write() call
-		if math.MaxInt-written < int(writeN) {
-			return written, errors.TraceNew("written + bytesToWrite overflows")
-		}
+	// Continue to Write() buffered bytes to underlying net.Conn until Write()
+	// fails or all buffered bytes are written.
+	for len(t.b) > 0 {
 
 		var n int
-		n, err = t.Conn.Write(t.b[:writeN])
-		written += n
+		n, err := t.Conn.Write(t.b)
 
-		// Do not need to check for underflow because n <= t.remain
 		t.remain -= uint64(n)
 
 		if n == len(t.b) {
@@ -291,11 +257,11 @@ func (t *HTTPTransformer) writeBuffer() (written int, err error) {
 
 		// Stop writing and return if there was an error
 		if err != nil {
-			return
+			return err
 		}
 	}
 
-	return
+	return nil
 }
 
 func WrapDialerWithHTTPTransformer(dialer common.Dialer, params *HTTPTransformerParameters) common.Dialer {

psiphon/common/transforms/httpTransformer_test.go

@@ -56,6 +56,20 @@ func TestHTTPTransformerHTTPRequest(t *testing.T) {
 			wantOutput: "POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\nabcd",
 			chunkSize:  1,
 		},
+		{
+			name:       "write header then body", // behaviour of net/http code
+			input:      "POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\nabcd",
+			wantOutput: "POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\nabcd",
+			chunkSize:  38,
+		},
+		{
+			name:          "write header then body with error",
+			input:         "POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\nabcd",
+			wantOutput:    "POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\nabcd",
+			chunkSize:     38,
+			connWriteErrs: []error{nil, errors.New("err1")},
+			wantError:     errors.New("err1"),
+		},
 		{
 			name:       "written in a single write",
 			input:      "POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\nabcd",
@@ -68,6 +82,7 @@ func TestHTTPTransformerHTTPRequest(t *testing.T) {
 			wantOutput:    "POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\nabcd",
 			chunkSize:     999,
 			connWriteErrs: []error{errors.New("err1")},
+			wantError:     errors.New("err1"),
 		},
 		{
 			name:           "written with partial write and errors",
@@ -75,7 +90,8 @@ func TestHTTPTransformerHTTPRequest(t *testing.T) {
 			wantOutput:     "POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\nabcd",
 			chunkSize:      1,
 			connWriteLimit: 1,
-			connWriteErrs:  []error{errors.New("err1"), errors.New("err2")},
+			connWriteErrs:  []error{nil, errors.New("err1"), errors.New("err2")},
+			wantError:      errors.New("err1"),
 		},
 		{
 			name:       "transform not applied to body",
@@ -102,24 +118,6 @@ func TestHTTPTransformerHTTPRequest(t *testing.T) {
 			wantOutput: "POST / HTTP/1.1\r\nContent-Length: 3\r\n\r\nabc",
 			chunkSize:  1,
 		},
-		{
-			name:          "written in a single write with errors and partial writes",
-			input:         "POST / HTTP/1.1\r\nHost: example.com\r\nContent-Length: 0\r\n\r\n",
-			wantOutput:    "POST / HTTP/1.1\r\nContent-Length: 0\r\n\r\n",
-			chunkSize:     999,
-			transform:     Spec{[2]string{"Host: example.com\r\n", ""}},
-			connWriteErrs: []error{errors.New("err1"), nil, errors.New("err2"), nil, nil, errors.New("err3")},
-			connWriteLens: []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 10},
-		},
-		{
-			name:          "written in a single write with error and partial write",
-			input:         "POST / HTTP/1.1\r\nHost: example.com\r\nContent-Length: 4\r\n\r\nabcd",
-			wantOutput:    "POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\nabcd",
-			chunkSize:     999,
-			transform:     Spec{[2]string{"Host: example.com\r\n", ""}},
-			connWriteErrs: []error{errors.New("err1")},
-			connWriteLens: []int{28}, // write lands mid "\r\n\r\n"
-		},
 		{
 			name:       "transform",
 			input:      "POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\nabcd",
@@ -135,6 +133,7 @@ func TestHTTPTransformerHTTPRequest(t *testing.T) {
 			transform:      Spec{[2]string{"4", "100"}},
 			connWriteLimit: 1,
 			connWriteErrs:  []error{errors.New("err1"), errors.New("err2")},
+			wantError:      errors.New("err1"),
 		},
 		{
 			name:           "transform with chunk write and errors in body write",
@@ -144,13 +143,45 @@ func TestHTTPTransformerHTTPRequest(t *testing.T) {
 			transform:      Spec{[2]string{"4", "100"}},
 			connWriteLimit: 1,
 			connWriteErrs:  []error{errors.New("err1"), errors.New("err2"), errors.New("err3")},
+			wantError:      errors.New("err1"),
+		},
+		//
+		// Below tests document unsupported behavior.
+		//
+		{
+			name:          "written in a single write with errors and partial writes",
+			input:         "POST / HTTP/1.1\r\nHost: example.com\r\nContent-Length: 0\r\n\r\n",
+			wantOutput:    "POST / HTTP/1.1\r\nContent-Length: 0\r\n\r\n",
+			chunkSize:     999,
+			transform:     Spec{[2]string{"Host: example.com\r\n", ""}},
+			connWriteErrs: []error{errors.New("err1"), nil, errors.New("err2"), nil, nil, errors.New("err3")},
+			connWriteLens: []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 10},
+			wantError:     errors.New("err1"),
+		},
+		{
+			name:          "written in a single write with error and partial write",
+			input:         "POST / HTTP/1.1\r\nHost: example.com\r\nContent-Length: 4\r\n\r\nabcd",
+			wantOutput:    "POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\nabcd",
+			chunkSize:     999,
+			transform:     Spec{[2]string{"Host: example.com\r\n", ""}},
+			connWriteErrs: []error{errors.New("err1")},
+			connWriteLens: []int{28}, // write lands mid "\r\n\r\n"
+			wantError:     errors.New("err1"),
 		},
-		// Multiple HTTP requests written in a single write.
 		{
 			name:       "multiple HTTP requests written in a single write",
 			input:      "POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\nabcdPOST / HTTP/1.1\r\nContent-Length: 2\r\n\r\n12",
 			wantOutput: "POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\nabcdPOST / HTTP/1.1\r\nContent-Length: 2\r\n\r\n12",
 			chunkSize:  999,
+			wantError:  errors.New("multiple HTTP requests in single Write() not supported"),
+		},
+		{
+			name:       "multiple HTTP requests written in a single write with transform",
+			input:      "POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\nabcdPOST / HTTP/1.1\r\nContent-Length: 4\r\n\r\n12POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\n34",
+			wantOutput: "POST / HTTP/1.1\r\nContent-Length: 100\r\n\r\nabcdPOST / HTTP/1.1\r\nContent-Length: 100\r\n\r\n12POST / HTTP/1.1\r\nContent-Length: 100\r\n\r\n34",
+			chunkSize:  999,
+			transform:  Spec{[2]string{"4", "100"}},
+			wantError:  errors.New("multiple HTTP requests in single Write() not supported"),
 		},
 		// Multiple HTTP requests written in a single write. A write will occur
 		// where it contains both the end of the previous HTTP request and the
@@ -159,15 +190,8 @@ func TestHTTPTransformerHTTPRequest(t *testing.T) {
 			name:       "multiple HTTP requests written in chunks",
 			input:      "POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\nabcdPOST / HTTP/1.1\r\nContent-Length: 2\r\n\r\n12",
 			wantOutput: "POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\nabcdPOST / HTTP/1.1\r\nContent-Length: 2\r\n\r\n12",
-			chunkSize:  3,
-		},
-		// Multiple HTTP requests written in a single write with transform.
-		{
-			name:       "multiple HTTP requests written in a single write with transform",
-			input:      "POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\nabcdPOST / HTTP/1.1\r\nContent-Length: 4\r\n\r\n12POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\n34",
-			wantOutput: "POST / HTTP/1.1\r\nContent-Length: 100\r\n\r\nabcdPOST / HTTP/1.1\r\nContent-Length: 100\r\n\r\n12POST / HTTP/1.1\r\nContent-Length: 100\r\n\r\n34",
-			chunkSize:  999,
-			transform:  Spec{[2]string{"4", "100"}},
+			chunkSize:  4,
+			wantError:  errors.New("multiple HTTP requests in single Write() not supported"),
 		},
 		// Multiple HTTP requests written in a single write with transform. A
 		// write will occur where it contains both the end of the previous HTTP
@@ -178,6 +202,7 @@ func TestHTTPTransformerHTTPRequest(t *testing.T) {
 			wantOutput: "POST / HTTP/1.1\r\nContent-Length: 100\r\n\r\nabcdPOST / HTTP/1.1\r\nContent-Length: 100\r\n\r\n12",
 			chunkSize:  4, // ensure one write contains bytes from both reqs
 			transform:  Spec{[2]string{"4", "100"}},
+			wantError:  errors.New("multiple HTTP requests in single Write() not supported"),
 		},
 	}
 
@@ -211,24 +236,19 @@ func TestHTTPTransformerHTTPRequest(t *testing.T) {
 				}
 
 				var b []byte
-				if len(remain) < tt.chunkSize {
+				if len(remain) < tt.chunkSize || tt.chunkSize == 0 {
 					b = remain
 				} else {
 					b = remain[:tt.chunkSize]
 				}
 
-				expectedErr := len(conn.writeErrs) > 0
-
 				var n int
 				n, err = transformer.Write(b)
 				if err != nil {
-					if expectedErr {
-						// reset err
-						err = nil
-					} else {
-						// err checked outside loop
-						break
-					}
+					// The underlying transport will be a reliable stream
+					// transport, i.e. TCP, and we expect the caller to stop
+					// writing after an error is returned.
+					break
 				}
 
 				remain = remain[n:]
@@ -255,10 +275,14 @@ func TestHTTPTransformerHTTPRequest(t *testing.T) {
 func TestHTTPTransformerHTTPServer(t *testing.T) {
 
 	type test struct {
-		name      string
-		request   func(string) *http.Request
-		wantBody  string
-		transform Spec
+		name           string
+		request        func(string) *http.Request
+		wantBody       string
+		transform      Spec
+		connWriteLimit int
+		connWriteLens  []int
+		connWriteErrs  []error
+		wantError      error
 	}
 
 	tests := []test{
@@ -278,6 +302,26 @@ func TestHTTPTransformerHTTPServer(t *testing.T) {
 			},
 			wantBody: "abc",
 		},
+		// Expect HTTP request to abort after a single Write() call on
+		// underlying net.Conn fails.
+		{
+			name:      "transport fails",
+			transform: Spec{[2]string{"", ""}},
+			request: func(addr string) *http.Request {
+
+				body := bytes.NewReader([]byte("abcd"))
+
+				req, err := http.NewRequest("POST", "http://"+addr, body)
+				if err != nil {
+					panic(err)
+				}
+
+				return req
+			},
+			wantBody:      "abc",
+			connWriteErrs: []error{errors.New("test error")},
+			wantError:     errors.New("test error"),
+		},
 	}
 
 	for _, tt := range tests {
@@ -295,7 +339,24 @@ func TestHTTPTransformerHTTPServer(t *testing.T) {
 			}
 
 			dialer := func(ctx context.Context, network, address string) (net.Conn, error) {
-				return net.Dial(network, address)
+
+				if network != "tcp" {
+					return nil, errors.New("expected network tcp")
+				}
+
+				conn, err := net.Dial(network, address)
+				if err != nil {
+					return nil, err
+				}
+
+				wrappedConn := testConn{
+					Conn:       conn,
+					writeLimit: tt.connWriteLimit,
+					writeLens:  tt.connWriteLens,
+					writeErrs:  tt.connWriteErrs,
+				}
+
+				return &wrappedConn, nil
 			}
 
 			httpTransport := &http.Transport{
@@ -309,7 +370,9 @@ func TestHTTPTransformerHTTPServer(t *testing.T) {
 
 			serverReq := make(chan *serverRequest, 1)
 
-			http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+			mux := http.NewServeMux()
+
+			mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
 				b, err := io.ReadAll(r.Body)
 				if err != nil {
 					w.WriteHeader(http.StatusInternalServerError)
@@ -324,7 +387,8 @@ func TestHTTPTransformerHTTPServer(t *testing.T) {
 			})
 
 			s := &http.Server{
-				Addr: "127.0.0.1:8080",
+				Addr:    "127.0.0.1:8080",
+				Handler: mux,
 			}
 
 			go func() {
@@ -346,18 +410,26 @@ func TestHTTPTransformerHTTPServer(t *testing.T) {
 				t.Fatalf("s.Shutdown failed %v", shutdownErr)
 			}
 
-			if err != nil {
-				t.Fatalf("client.Do failed %v", err)
-			}
-
-			if resp.StatusCode != http.StatusOK {
-				t.Fatalf("expected 200 but got %d", resp.StatusCode)
-			}
+			if tt.wantError == nil {
+				if err != nil {
+					t.Fatalf("client.Do failed %v", err)
+				}
+				if resp.StatusCode != http.StatusOK {
+					t.Fatalf("expected 200 but got %d", resp.StatusCode)
+				}
 
-			r := <-serverReq
+				r := <-serverReq
 
-			if tt.wantBody != string(r.body) {
-				t.Fatalf("expected body %s but got %s", tt.wantBody, string(r.body))
+				if tt.wantBody != string(r.body) {
+					t.Fatalf("expected body %s but got %s", tt.wantBody, string(r.body))
+				}
+			} else {
+				// tt.wantError != nil
+				if err == nil {
+					t.Fatalf("expected error %v", tt.wantError)
+				} else if !strings.Contains(err.Error(), tt.wantError.Error()) {
+					t.Fatalf("expected error %v got %v", tt.wantError, err)
+				}
 			}
 		})
 	}
@@ -382,10 +454,12 @@ type testConn struct {
 	// writeErrs are returned from Write() calls in order. If empty, then a nil
 	// error is returned.
 	writeErrs []error
+
+	net.Conn
 }
 
 func (c *testConn) Read(b []byte) (n int, err error) {
-	return 0, nil
+	return c.Conn.Read(b)
 }
 
 func (c *testConn) Write(b []byte) (n int, err error) {
@@ -412,29 +486,34 @@ func (c *testConn) Write(b []byte) (n int, err error) {
 		n = len(b)
 	}
 
+	// Only write to net.Conn if set
+	if c.Conn != nil && n > 0 {
+		c.Conn.Write(b[:n])
+	}
+
 	return
 }
 
 func (c *testConn) Close() error {
-	return nil
+	return c.Conn.Close()
 }
 
 func (c *testConn) LocalAddr() net.Addr {
-	return nil
+	return c.Conn.LocalAddr()
 }
 
 func (c *testConn) RemoteAddr() net.Addr {
-	return nil
+	return c.Conn.RemoteAddr()
 }
 
 func (c *testConn) SetDeadline(t time.Time) error {
-	return nil
+	return c.Conn.SetDeadline(t)
 }
 
 func (c *testConn) SetReadDeadline(t time.Time) error {
-	return nil
+	return c.Conn.SetReadDeadline(t)
 }
 
 func (c *testConn) SetWriteDeadline(t time.Time) error {
-	return nil
+	return c.Conn.SetWriteDeadline(t)
 }