113 lines
3.5 KiB
Diff
113 lines
3.5 KiB
Diff
commit 143822585e32449860e624cace9d2e521deee62e
|
|
Author: Brad Fitzpatrick <bradfitz@golang.org>
|
|
Date: Tue Jul 7 13:19:44 2015 -0600
|
|
|
|
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 <adg@golang.org>
|
|
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
|
|
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
|
|
diff --git a/src/net/http/readrequest_test.go b/src/net/http/readrequest_test.go
|
|
index 1a3cf91..60e2be4 100644
|
|
--- a/src/net/http/readrequest_test.go
|
|
+++ b/src/net/http/readrequest_test.go
|
|
@@ -178,6 +178,36 @@ var reqTests = []reqTest{
|
|
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",
|
|
@@ -400,11 +430,6 @@ 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
|
|
Host: foo
|
|
Content-Length: 5`)},
|
|
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
|
|
}
|
|
|