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()