From 143822585e32449860e624cace9d2e521deee62e Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 7 Jul 2015 13:19:44 -0600 Subject: [PATCH] net/http: revert overly-strict part of earlier smuggling defense The recent https://golang.org/cl/11810 is reportedly a bit too aggressive. Apparently some HTTP requests in the wild do contain both a Transfer-Encoding along with a bogus Content-Length. Instead of returning a 400 Bad Request error, we should just ignore the Content-Length like we did before. Change-Id: I0001be90d09f8293a34f04691f608342875ff5c4 Reviewed-on: https://go-review.googlesource.com/11962 Reviewed-by: Andrew Gerrand Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/net/http/readrequest_test.go | 35 ++++++++++++++++++++++++++++++----- src/net/http/transfer.go | 21 ++++++++++++++------- 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/src/net/http/transfer.go b/src/net/http/transfer.go index 3c868bd..fbbbf24 100644 --- a/src/net/http/transfer.go +++ b/src/net/http/transfer.go @@ -430,7 +430,6 @@ func fixTransferEncoding(isResponse bool, requestMethod string, header Header) ( if !present { return nil, nil } - isRequest := !isResponse delete(header, "Transfer-Encoding") encodings := strings.Split(raw[0], ",") @@ -458,12 +457,20 @@ func fixTransferEncoding(isResponse bool, requestMethod string, header Header) ( // 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 }