src/net/http/readrequest_test.go | 35 ++++++++++++++++++++++++++++++----- src/net/http/transfer.go | 21 ++++++++++++++------- diff --git a/src/net/http/readrequest_test.go b/src/net/http/readrequest_test.go index 1a3cf913bb39ecaedc50211044b59d2151c52fe1..60e2be41d177f9d0d874627ecc28f43d5cd4ffd7 100644 --- a/src/net/http/readrequest_test.go +++ b/src/net/http/readrequest_test.go @@ -178,6 +178,36 @@ }, noError, }, + // Tests chunked body and a bogus Content-Length which should be deleted. + { + "POST / HTTP/1.1\r\n" + + "Host: foo.com\r\n" + + "Transfer-Encoding: chunked\r\n" + + "Content-Length: 9999\r\n\r\n" + // to be removed. + "3\r\nfoo\r\n" + + "3\r\nbar\r\n" + + "0\r\n" + + "\r\n", + &Request{ + Method: "POST", + URL: &url.URL{ + Path: "/", + }, + TransferEncoding: []string{"chunked"}, + Proto: "HTTP/1.1", + ProtoMajor: 1, + ProtoMinor: 1, + Header: Header{}, + ContentLength: -1, + Host: "foo.com", + RequestURI: "/", + }, + + "foobar", + noTrailer, + noError, + }, + // CONNECT request with domain name: { "CONNECT www.google.com:443 HTTP/1.1\r\n\r\n", @@ -398,11 +428,6 @@ {"bad_connect_host", reqBytes("CONNECT []%20%48%54%54%50%2f%31%2e%31%0a%4d%79%48%65%61%64%65%72%3a%20%31%32%33%0a%0a HTTP/1.0")}, {"smuggle_two_contentlen", reqBytes(`POST / HTTP/1.1 Content-Length: 3 Content-Length: 4 - -abc`)}, - {"smuggle_chunked_and_len", reqBytes(`POST / HTTP/1.1 -Transfer-Encoding: chunked -Content-Length: 3 abc`)}, {"smuggle_content_len_head", reqBytes(`HEAD / HTTP/1.1 diff --git a/src/net/http/transfer.go b/src/net/http/transfer.go index 3c868bd1320ef6c9bfae38bb7a87f17d98c1295c..fbbbf2417ace3a88245601ca61034f7f956d1671 100644 --- a/src/net/http/transfer.go +++ b/src/net/http/transfer.go @@ -430,7 +430,6 @@ raw, present := header["Transfer-Encoding"] if !present { return nil, nil } - isRequest := !isResponse delete(header, "Transfer-Encoding") encodings := strings.Split(raw[0], ",") @@ -458,12 +457,20 @@ if len(te) > 0 { // RFC 7230 3.3.2 says "A sender MUST NOT send a // Content-Length header field in any message that // contains a Transfer-Encoding header field." - if len(header["Content-Length"]) > 0 { - if isRequest { - return nil, errors.New("http: invalid Content-Length with Transfer-Encoding") - } - delete(header, "Content-Length") - } + // + // but also: + // "If a message is received with both a + // Transfer-Encoding and a Content-Length header + // field, the Transfer-Encoding overrides the + // Content-Length. Such a message might indicate an + // attempt to perform request smuggling (Section 9.5) + // or response splitting (Section 9.4) and ought to be + // handled as an error. A sender MUST remove the + // received Content-Length field prior to forwarding + // such a message downstream." + // + // Reportedly, these appear in the wild. + delete(header, "Content-Length") return te, nil }