qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-12 07:40:23 -07:00
Code Releases Activity

net/textproto: reject all headers with a leading space

Browse Source 
Previously, golang.org/cl/75350 updated ReadMIMEHeader to ignore the
first header line when it begins with a leading space, as in the
following example:

GET / HTTP/1.1
  Host: foo.com
Accept-Encoding: gzip

However, golang.org/cl/75350 changed ReadMIMEHeader's behavior for the
following example: before the CL it returned an error, but after the
CL it ignored the first line.

GET / HTTP/1.1
  Host foo.com
Accept-Encoding: gzip

This change updates ReadMIMEHeader to always fail when the first header
line starts with a space. During the discussion for golang.org/cl/75350,
we realized we had three competing needs:

1. HTTP clients should accept malformed response headers when possible
   (ignoring the malformed lines).

2. HTTP servers should reject all malformed request headers.

3. The net/textproto package is used by multiple protocols (most notably,
   HTTP and SMTP) which have slightly different parsing semantics. This
   complicates changes to net/textproto.

We weren't sure how to best fix net/textproto without an API change, but
it is too late for API changes in Go 1.10. We decided to ignore initial
lines that begin with spaces, thinking that would have the least impact on
existing users -- malformed headers would continue to parse, but the
initial lines would be ignored. Instead, golang.org/cl/75350 actually
changed ReadMIMEHeader to succeed in cases where it previously failed
(as in the above example).

Reconsidering the above two examples, there does not seem to be a good
argument to silently ignore ` Host: foo.com` but fail on ` Host foo.com`.
Hence, this change fails for *all* headers where the initial line begins
with a space.

Updates #22464

Change-Id: I68d3d190489c350b0bc1549735bf6593fe11a94c
Reviewed-on: https://go-review.googlesource.com/80055
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This commit is contained in:
Tom Bergan 2017-11-27 11:25:14 -08:00
parent 671cf92c32
commit 1c69384da4
5 changed files with 54 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
src/net/http/readrequest_test.go
View File

@ -401,26 +401,6 @@ var reqTests = []reqTest{
noTrailer,
noError,
},
// leading whitespace in the first header. golang.org/issue/22464
{
"GET / HTTP/1.1\r\n Foobar: ignored\r\nConnection: close\r\n\r\n",
&Request{
Method: "GET",
URL: &url.URL{
Path: "/",
},
Header: Header{"Connection": {"close"}},
Proto: "HTTP/1.1",
ProtoMajor: 1,
ProtoMinor: 1,
RequestURI: "/",
Close: true,
},
noBodyStr,
noTrailer,
noError,
},
}
func TestReadRequest(t *testing.T) {
@ -473,6 +453,14 @@ Content-Length: 4
abc`)},
{"smuggle_content_len_head", reqBytes(`HEAD / HTTP/1.1
Host: foo
Content-Length: 5`)},
// golang.org/issue/22464
{"leading_space_in_header", reqBytes(`HEAD / HTTP/1.1
Host: foo
Content-Length: 5`)},
{"leading_tab_in_header", reqBytes(`HEAD / HTTP/1.1
\tHost: foo
Content-Length: 5`)},
}

58
src/net/http/response_test.go
View File

@ -555,28 +555,6 @@ some body`,
},
"Your Authentication failed.\r\n",
},
// leading whitespace in the first header. golang.org/issue/22464
{
"HTTP/1.1 200 OK\r\n" +
" Content-type: text/html\r\n" +
"\tIgnore: foobar\r\n" +
"Foo: bar\r\n\r\n",
Response{
Status: "200 OK",
StatusCode: 200,
Proto: "HTTP/1.1",
ProtoMajor: 1,
ProtoMinor: 1,
Request: dummyReq("GET"),
Header: Header{
"Foo": {"bar"},
},
Close: true,
ContentLength: -1,
},
"",
},
}
// tests successful calls to ReadResponse, and inspects the returned Response.
@ -838,7 +816,6 @@ func TestReadResponseErrors(t *testing.T) {
type testCase struct {
name string // optional, defaults to in
in string
header Header
wantErr interface{} // nil, err value, or string substring
}
@ -864,22 +841,21 @@ func TestReadResponseErrors(t *testing.T) {
}
}
contentLength := func(status, body string, wantErr interface{}, header Header) testCase {
contentLength := func(status, body string, wantErr interface{}) 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{
{"", "", 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"},
{"", "", 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"},
status("20X Unknown", true),
status("abcd Unknown", true),
status("二百/两百 OK", true),
@ -905,18 +881,22 @@ func TestReadResponseErrors(t *testing.T) {
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),
contentLength("200 OK", "Content-Length: 10\r\nContent-Length: 7\r\n\r\nGopher hey\r\n", errMultiCL),
contentLength("200 OK", "Content-Length: 7\r\nContent-Length: 7\r\n\r\nGophers\r\n", nil),
contentLength("201 OK", "Content-Length: 0\r\nContent-Length: 7\r\n\r\nGophers\r\n", errMultiCL),
contentLength("300 OK", "Content-Length: 0\r\nContent-Length: 0 \r\n\r\nGophers\r\n", nil),
contentLength("200 OK", "Content-Length:\r\nContent-Length:\r\n\r\nGophers\r\n", nil),
contentLength("206 OK", "Content-Length:\r\nContent-Length: 0 \r\nConnection: close\r\n\r\nGophers\r\n", errMultiCL),
// 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),
contentLength("204 OK", "Content-Length: 7\r\nContent-Length: 8\r\n\r\n", errMultiCL),
contentLength("204 OK", "Content-Length: 3\r\nContent-Length: 3\r\n\r\n", nil),
contentLength("304 OK", "Content-Length: 880\r\nContent-Length: 1\r\n\r\n", errMultiCL),
contentLength("304 OK", "Content-Length: 961\r\nContent-Length: 961\r\n\r\n", nil),
// golang.org/issue/22464
{"leading space in header", "HTTP/1.1 200 OK\r\n Content-type: text/html\r\nFoo: bar\r\n\r\n", "malformed MIME"},
{"leading tab in header", "HTTP/1.1 200 OK\r\n\tContent-type: text/html\r\nFoo: bar\r\n\r\n", "malformed MIME"},
}
for i, tt := range tests {

13
src/net/http/transport_test.go
View File

@ -4281,12 +4281,13 @@ func TestMissingStatusNoPanic(t *testing.T) {
shutdown := make(chan bool, 1)
done := make(chan bool)
fullAddrURL := fmt.Sprintf("http://%s", addr)
raw := `HTTP/1.1 400
Date: Wed, 30 Aug 2017 19:09:27 GMT
Content-Type: text/html; charset=utf-8
Content-Length: 10
Last-Modified: Wed, 30 Aug 2017 19:02:02 GMT
Vary: Accept-Encoding` + "\r\n\r\nAloha Olaa"
raw := "HTTP/1.1 400\r\n" +
"Date: Wed, 30 Aug 2017 19:09:27 GMT\r\n" +
"Content-Type: text/html; charset=utf-8\r\n" +
"Content-Length: 10\r\n" +
"Last-Modified: Wed, 30 Aug 2017 19:02:02 GMT\r\n" +
"Vary: Accept-Encoding\r\n\r\n" +
"Aloha Olaa"
go func() {
defer func() {

12
src/net/textproto/reader.go
View File

@ -477,11 +477,13 @@ func (r *Reader) ReadMIMEHeader() (MIMEHeader, error) {
m := make(MIMEHeader, hint)
for r.skipSpace() > 0 {
// The first line cannot start with a leading space.
if buf, err := r.R.Peek(1); err == nil && (buf[0] == ' ' || buf[0] == '\t') {
line, err := r.readLineSlice()
if len(line) == 0 || err != nil {
if err != nil {
return m, err
}
return m, ProtocolError("malformed MIME header initial line: " + string(line))
}
for {
@ -490,9 +492,9 @@ func (r *Reader) ReadMIMEHeader() (MIMEHeader, error) {
return m, err
}
// Key ends at first colon; should not have spaces but
// they appear in the wild, violating specs, so we
// remove them if present.
// Key ends at first colon; should not have trailing spaces
// but they appear in the wild, violating specs, so we remove
// them if present.
i := bytes.IndexByte(kv, ':')
if i < 0 {
return m, ProtocolError("malformed MIME header line: " + string(kv))

27
src/net/textproto/reader_test.go
View File

@ -211,21 +211,20 @@ func TestReadMIMEHeaderNonCompliant(t *testing.T) {
}
}
func TestReadMIMEHeaderLeadingSpace(t *testing.T) {
tests := []struct {
input string
want MIMEHeader
}{
{" Ignore: ignore\r\nFoo: foo\r\n\r\n", MIMEHeader{"Foo": {"foo"}}},
{"\tIgnore: ignore\r\nFoo: foo\r\n\r\n", MIMEHeader{"Foo": {"foo"}}},
{" Ignore1: ignore\r\n Ignore2: ignore\r\nFoo: foo\r\n\r\n", MIMEHeader{"Foo": {"foo"}}},
{" Ignore1: ignore\r\n\r\n", MIMEHeader{}},
func TestReadMIMEHeaderMalformed(t *testing.T) {
inputs := []string{
"No colon first line\r\nFoo: foo\r\n\r\n",
" No colon first line with leading space\r\nFoo: foo\r\n\r\n",
"\tNo colon first line with leading tab\r\nFoo: foo\r\n\r\n",
" First: line with leading space\r\nFoo: foo\r\n\r\n",
"\tFirst: line with leading tab\r\nFoo: foo\r\n\r\n",
"Foo: foo\r\nNo colon second line\r\n\r\n",
}
for _, tt := range tests {
r := reader(tt.input)
m, err := r.ReadMIMEHeader()
if !reflect.DeepEqual(m, tt.want) || err != nil {
t.Errorf("ReadMIMEHeader(%q) = %v, %v; want %v", tt.input, m, err, tt.want)
for _, input := range inputs {
r := reader(input)
if m, err := r.ReadMIMEHeader(); err == nil {
t.Errorf("ReadMIMEHeader(%q) = %v, %v; want nil, err", input, m, err)
}
}
}