From f407dbb71253a3320b2e54e0ce33a18895d673f9 Mon Sep 17 00:00:00 2001 From: GoncaloGarcia Date: Mon, 21 Oct 2024 16:07:52 +0100 Subject: [PATCH] Revert "TUN-8592: Use metadata from the edge to determine if request body is empty for QUIC transport" This reverts commit e2064c820f32802f58baa027903043fe0ed052e0. --- connection/quic.go | 53 +++--------------- connection/quic_test.go | 119 ---------------------------------------- 2 files changed, 8 insertions(+), 164 deletions(-) diff --git a/connection/quic.go b/connection/quic.go index 34855b3f..cbf5b186 100644 --- 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 index 3504168f..c073b850 100644 --- 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()