Revert "TUN-8592: Use metadata from the edge to determine if request …

…body is empty for QUIC transport" This reverts commit e2064c8.
cloudflare · Oct 21, 2024 · f407dbb · f407dbb
1 parent 92e0f5f
commit f407dbb
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 164 deletions.
diff --git a/connection/quic.go b/connection/quic.go
@@ -42,26 +42,12 @@ const (
 	HTTPMethodKey = "HttpMethod"
 	// HTTPHostKey is used to get or set http Method in QUIC ALPN if the underlying proxy connection type is HTTP.
 	HTTPHostKey = "HttpHost"
-	// HTTPRequestBodyHintKey is used in ConnectRequest metadata to indicate if the request has body
-	HTTPRequestBodyHintKey = "HttpReqBodyHint"
 
 	QUICMetadataFlowID = "FlowID"
 	// emperically this capacity has been working well
 	demuxChanCapacity = 16
 )
 
-type RequestBodyHint uint64
-
-const (
-	RequestBodyHintMissing RequestBodyHint = iota
-	RequestBodyHintEmpty
-	RequestBodyHintHasData
-)
-
-func (rbh RequestBodyHint) String() string {
-	return [...]string{"missing", "empty", "data"}[rbh]
-}
-
 var (
 	portForConnIndex = make(map[uint8]int, 0)
 	portMapMutex     sync.Mutex
@@ -500,6 +486,7 @@ func buildHTTPRequest(
 	dest := connectRequest.Dest
 	method := metadata[HTTPMethodKey]
 	host := metadata[HTTPHostKey]
+	isWebsocket := connectRequest.Type == pogs.ConnectionTypeWebsocket
 
 	req, err := http.NewRequestWithContext(ctx, method, dest, body)
 	if err != nil {
@@ -524,8 +511,13 @@ func buildHTTPRequest(
 		return nil, fmt.Errorf("Error setting content-length: %w", err)
 	}
 
-	if shouldSetRequestBodyToEmpty(connectRequest, metadata, req) {
-		log.Debug().Str("host", req.Host).Str("method", req.Method).Msg("Set request to have no body")
+	// Go's client defaults to chunked encoding after a 200ms delay if the following cases are true:
+	//   * the request body blocks
+	//   * the content length is not set (or set to -1)
+	//   * the method doesn't usually have a body (GET, HEAD, DELETE, ...)
+	//   * there is no transfer-encoding=chunked already set.
+	// So, if transfer cannot be chunked and content length is 0, we dont set a request body.
+	if !isWebsocket && !isTransferEncodingChunked(req) && req.ContentLength == 0 {
 		req.Body = http.NoBody
 	}
 	stripWebsocketUpgradeHeader(req)
@@ -550,35 +542,6 @@ func isTransferEncodingChunked(req *http.Request) bool {
 	return strings.Contains(strings.ToLower(transferEncodingVal), "chunked")
 }
 
-// Borrowed from https://github.com/golang/go/blob/go1.22.6/src/net/http/request.go#L1541
-func requestMethodUsuallyLacksBody(req *http.Request) bool {
-	switch strings.ToUpper(req.Method) {
-	case "GET", "HEAD", "DELETE", "OPTIONS", "PROPFIND", "SEARCH":
-		return true
-	}
-	return false
-}
-
-func shouldSetRequestBodyToEmpty(connectRequest *pogs.ConnectRequest, metadata map[string]string, req *http.Request) bool {
-	switch metadata[HTTPRequestBodyHintKey] {
-	case RequestBodyHintEmpty.String():
-		return true
-	case RequestBodyHintHasData.String():
-		return false
-	default:
-	}
-
-	isWebsocket := connectRequest.Type == pogs.ConnectionTypeWebsocket
-	// Go's client defaults to chunked encoding after a 200ms delay if the following cases are true:
-	//   * the request body blocks
-	//   * the content length is not set (or set to -1)
-	//   * the method doesn't usually have a body (GET, HEAD, DELETE, ...)
-	//   * there is no transfer-encoding=chunked already set.
-	// So, if transfer cannot be chunked and content length is 0, we dont set a request body.
-	// Reference: https://github.com/golang/go/blob/go1.22.2/src/net/http/transfer.go#L192-L206
-	return !isWebsocket && requestMethodUsuallyLacksBody(req) && !isTransferEncodingChunked(req) && req.ContentLength == 0
-}
-
 // A helper struct that guarantees a call to close only affects read side, but not write side.
 type nopCloserReadWriter struct {
 	io.ReadWriteCloser

diff --git a/connection/quic_test.go b/connection/quic_test.go
@@ -486,125 +486,6 @@ func TestBuildHTTPRequest(t *testing.T) {
 			},
 			body: io.NopCloser(&bytes.Buffer{}),
 		},
-		{
-			name: "if edge sends the body is empty hint, set body to empty",
-			connectRequest: &pogs.ConnectRequest{
-				Dest: "http://test.com",
-				Metadata: []pogs.Metadata{
-					{
-						Key: "HttpHeader:Another-Header",
-						Val: "Misc",
-					},
-					{
-						Key: "HttpHost",
-						Val: "cf.host",
-					},
-					{
-						Key: "HttpMethod",
-						Val: "put",
-					},
-					{
-						Key: HTTPRequestBodyHintKey,
-						Val: RequestBodyHintEmpty.String(),
-					},
-				},
-			},
-			req: &http.Request{
-				Method: "put",
-				URL: &url.URL{
-					Scheme: "http",
-					Host:   "test.com",
-				},
-				Proto:      "HTTP/1.1",
-				ProtoMajor: 1,
-				ProtoMinor: 1,
-				Header: http.Header{
-					"Another-Header": []string{"Misc"},
-				},
-				ContentLength: 0,
-				Host:          "cf.host",
-				Body:          http.NoBody,
-			},
-			body: io.NopCloser(&bytes.Buffer{}),
-		},
-		{
-			name: "if edge sends the body has data hint, don't set body to empty",
-			connectRequest: &pogs.ConnectRequest{
-				Dest: "http://test.com",
-				Metadata: []pogs.Metadata{
-					{
-						Key: "HttpHeader:Another-Header",
-						Val: "Misc",
-					},
-					{
-						Key: "HttpHost",
-						Val: "cf.host",
-					},
-					{
-						Key: "HttpMethod",
-						Val: "put",
-					},
-					{
-						Key: HTTPRequestBodyHintKey,
-						Val: RequestBodyHintHasData.String(),
-					},
-				},
-			},
-			req: &http.Request{
-				Method: "put",
-				URL: &url.URL{
-					Scheme: "http",
-					Host:   "test.com",
-				},
-				Proto:      "HTTP/1.1",
-				ProtoMajor: 1,
-				ProtoMinor: 1,
-				Header: http.Header{
-					"Another-Header": []string{"Misc"},
-				},
-				ContentLength: 0,
-				Host:          "cf.host",
-				Body:          io.NopCloser(&bytes.Buffer{}),
-			},
-			body: io.NopCloser(&bytes.Buffer{}),
-		},
-		{
-			name: "if the http method usually has body, don't set body to empty",
-			connectRequest: &pogs.ConnectRequest{
-				Dest: "http://test.com",
-				Metadata: []pogs.Metadata{
-					{
-						Key: "HttpHeader:Another-Header",
-						Val: "Misc",
-					},
-					{
-						Key: "HttpHost",
-						Val: "cf.host",
-					},
-					{
-						Key: "HttpMethod",
-						Val: "post",
-					},
-				},
-			},
-			req: &http.Request{
-				Method: "post",
-				URL: &url.URL{
-					Scheme: "http",
-					Host:   "test.com",
-				},
-				Proto:      "HTTP/1.1",
-				ProtoMajor: 1,
-				ProtoMinor: 1,
-				Header: http.Header{
-					"Another-Header": []string{"Misc"},
-				},
-				ContentLength: 0,
-				Host:          "cf.host",
-				Body:          io.NopCloser(&bytes.Buffer{}),
-			},
-			body: io.NopCloser(&bytes.Buffer{}),
-		},
 	}
 
 	log := zerolog.Nop()