var readRequestErrorTests = []struct {
in string
- err error
+ err string
+
+ header Header
}{
- {"GET / HTTP/1.1\r\nheader:foo\r\n\r\n", nil},
- {"GET / HTTP/1.1\r\nheader:foo\r\n", io.ErrUnexpectedEOF},
- {"", io.EOF},
+ 0: {"GET / HTTP/1.1\r\nheader:foo\r\n\r\n", "", Header{"Header": {"foo"}}},
+ 1: {"GET / HTTP/1.1\r\nheader:foo\r\n", io.ErrUnexpectedEOF.Error(), nil},
+ 2: {"", io.EOF.Error(), nil},
+ 3: {
+ in: "HEAD / HTTP/1.1\r\nContent-Length:4\r\n\r\n",
+ err: "http: method cannot contain a Content-Length",
+ },
+ 4: {
+ in: "HEAD / HTTP/1.1\r\n\r\n",
+ header: Header{},
+ },
+
+ // Multiple Content-Length values should either be
+ // deduplicated if same or reject otherwise
+ // See Issue 16490.
+ 5: {
+ in: "POST / HTTP/1.1\r\nContent-Length: 10\r\nContent-Length: 0\r\n\r\nGopher hey\r\n",
+ err: "cannot contain multiple Content-Length headers",
+ },
+ 6: {
+ in: "POST / HTTP/1.1\r\nContent-Length: 10\r\nContent-Length: 6\r\n\r\nGopher\r\n",
+ err: "cannot contain multiple Content-Length headers",
+ },
+ 7: {
+ in: "PUT / HTTP/1.1\r\nContent-Length: 6 \r\nContent-Length: 6\r\nContent-Length:6\r\n\r\nGopher\r\n",
+ err: "",
+ header: Header{"Content-Length": {"6"}},
+ },
+ 8: {
+ in: "PUT / HTTP/1.1\r\nContent-Length: 1\r\nContent-Length: 6 \r\n\r\n",
+ err: "cannot contain multiple Content-Length headers",
+ },
+ 9: {
+ in: "POST / HTTP/1.1\r\nContent-Length:\r\nContent-Length: 3\r\n\r\n",
+ err: "cannot contain multiple Content-Length headers",
+ },
+ 10: {
+ in: "HEAD / HTTP/1.1\r\nContent-Length:0\r\nContent-Length: 0\r\n\r\n",
+ header: Header{"Content-Length": {"0"}},
+ },
}
func TestReadRequestErrors(t *testing.T) {
for i, tt := range readRequestErrorTests {
- _, err := ReadRequest(bufio.NewReader(strings.NewReader(tt.in)))
- if err != tt.err {
- t.Errorf("%d. got error = %v; want %v", i, err, tt.err)
+ req, err := ReadRequest(bufio.NewReader(strings.NewReader(tt.in)))
+ if err == nil {
+ if tt.err != "" {
+ t.Errorf("#%d: got nil err; want %q", i, tt.err)
+ }
+
+ if !reflect.DeepEqual(tt.header, req.Header) {
+ t.Errorf("#%d: gotHeader: %q wantHeader: %q", i, req.Header, tt.header)
+ }
+ continue
+ }
+
+ if tt.err == "" || !strings.Contains(err.Error(), tt.err) {
+ t.Errorf("%d: got error = %v; want %v", i, err, tt.err)
}
}
}
type testCase struct {
name string // optional, defaults to in
in string
+ header Header
wantErr interface{} // nil, err value, or string substring
}
}
}
+ contentLength := func(status, body string, wantErr interface{}, header Header) testCase {
+ return testCase{
+ name: fmt.Sprintf("status %q %q", status, body),
+ in: fmt.Sprintf("HTTP/1.1 %s\r\n%s", status, body),
+ wantErr: wantErr,
+ header: header,
+ }
+ }
+
+ errMultiCL := "message cannot contain multiple Content-Length headers"
+
tests := []testCase{
- {"", "", io.ErrUnexpectedEOF},
- {"", "HTTP/1.1 301 Moved Permanently\r\nFoo: bar", io.ErrUnexpectedEOF},
- {"", "HTTP/1.1", "malformed HTTP response"},
- {"", "HTTP/2.0", "malformed HTTP response"},
+ {"", "", nil, io.ErrUnexpectedEOF},
+ {"", "HTTP/1.1 301 Moved Permanently\r\nFoo: bar", nil, io.ErrUnexpectedEOF},
+ {"", "HTTP/1.1", nil, "malformed HTTP response"},
+ {"", "HTTP/2.0", nil, "malformed HTTP response"},
status("20X Unknown", true),
status("abcd Unknown", true),
status("二百/两百 OK", true),
version("HTTP/A.B", true),
version("HTTP/1", true),
version("http/1.1", true),
+
+ contentLength("200 OK", "Content-Length: 10\r\nContent-Length: 7\r\n\r\nGopher hey\r\n", errMultiCL, nil),
+ contentLength("200 OK", "Content-Length: 7\r\nContent-Length: 7\r\n\r\nGophers\r\n", nil, Header{"Content-Length": {"7"}}),
+ contentLength("201 OK", "Content-Length: 0\r\nContent-Length: 7\r\n\r\nGophers\r\n", errMultiCL, nil),
+ contentLength("300 OK", "Content-Length: 0\r\nContent-Length: 0 \r\n\r\nGophers\r\n", nil, Header{"Content-Length": {"0"}}),
+ contentLength("200 OK", "Content-Length:\r\nContent-Length:\r\n\r\nGophers\r\n", nil, nil),
+ contentLength("206 OK", "Content-Length:\r\nContent-Length: 0 \r\nConnection: close\r\n\r\nGophers\r\n", errMultiCL, nil),
+
+ // multiple content-length headers for 204 and 304 should still be checked
+ contentLength("204 OK", "Content-Length: 7\r\nContent-Length: 8\r\n\r\n", errMultiCL, nil),
+ contentLength("204 OK", "Content-Length: 3\r\nContent-Length: 3\r\n\r\n", nil, nil),
+ contentLength("304 OK", "Content-Length: 880\r\nContent-Length: 1\r\n\r\n", errMultiCL, nil),
+ contentLength("304 OK", "Content-Length: 961\r\nContent-Length: 961\r\n\r\n", nil, nil),
}
+
for i, tt := range tests {
br := bufio.NewReader(strings.NewReader(tt.in))
_, rerr := ReadResponse(br, nil)
// function is not a method, because ultimately it should be shared by
// ReadResponse and ReadRequest.
func fixLength(isResponse bool, status int, requestMethod string, header Header, te []string) (int64, error) {
- contentLens := header["Content-Length"]
isRequest := !isResponse
+ contentLens := header["Content-Length"]
+
+ // Hardening against HTTP request smuggling
+ if len(contentLens) > 1 {
+ // Per RFC 7230 Section 3.3.2, prevent multiple
+ // Content-Length headers if they differ in value.
+ // If there are dups of the value, remove the dups.
+ // See Issue 16490.
+ first := strings.TrimSpace(contentLens[0])
+ for _, ct := range contentLens[1:] {
+ if first != strings.TrimSpace(ct) {
+ return 0, fmt.Errorf("http: message cannot contain multiple Content-Length headers; got %q", contentLens)
+ }
+ }
+
+ // deduplicate Content-Length
+ header.Del("Content-Length")
+ header.Add("Content-Length", first)
+
+ contentLens = header["Content-Length"]
+ }
+
// Logic based on response type or status
if noBodyExpected(requestMethod) {
// For HTTP requests, as part of hardening against request
return 0, nil
}
- if len(contentLens) > 1 {
- // harden against HTTP request smuggling. See RFC 7230.
- return 0, errors.New("http: message cannot contain multiple Content-Length headers")
- }
-
// Logic based on Transfer-Encoding
if chunked(te) {
return -1, nil
header.Del("Content-Length")
}
- if !isResponse {
+ if isRequest {
// RFC 2616 neither explicitly permits nor forbids an
// entity-body on a GET request so we permit one if
// declared, but we default to 0 here (not -1 below)