From d6b0833209e9b45921b20dfd4c6e1f56e9ba88fa Mon Sep 17 00:00:00 2001 From: chungthuang Date: Fri, 9 Aug 2024 14:43:35 -0500 Subject: [PATCH] TUN-8592: Use metadata from the edge to determine if request body is empty for QUIC transport If the metadata is missing, fallback to decide based on protocol, http method, transferring and content length --- connection/quic.go | 53 +++++++++++++++--- connection/quic_test.go | 119 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+), 8 deletions(-) diff --git a/connection/quic.go b/connection/quic.go index c5e218f3..e1048e3a 100644 --- a/connection/quic.go +++ b/connection/quic.go @@ -42,12 +42,26 @@ 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 @@ -474,7 +488,6 @@ 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 { @@ -499,13 +512,8 @@ func buildHTTPRequest( return nil, fmt.Errorf("Error setting content-length: %w", err) } - // 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 { + if shouldSetRequestBodyToEmpty(connectRequest, metadata, req) { + log.Debug().Str("host", req.Host).Str("method", req.Method).Msg("Set request to have no body") req.Body = http.NoBody } stripWebsocketUpgradeHeader(req) @@ -530,6 +538,35 @@ 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 c81d53fb..302cb7f9 100644 --- a/connection/quic_test.go +++ b/connection/quic_test.go @@ -484,6 +484,125 @@ 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()