Commit 087b708f authored by John Graham-Cumming's avatar John Graham-Cumming Committed by Brad Fitzpatrick

net/http: fix handling of HEAD in ReadResponse and (*http.Response).Write

The test suite for ReadResponse was not checking the error return on the io.Copy
on the body. This was masking two errors: the handling of chunked responses to
HEAD requests and the handling of Content-Length > 0 to HEAD.

The former manifested itself as an 'unexpected EOF' when doing the io.Copy
because a chunked reader was assigned but there were no chunks to read. The
latter cause (*http.Response).Write to report an error on HEAD requests
because it saw a Content-Length > 0 and expected a body.

There was also a missing \r\n in one chunked test that meant that the chunked
encoding was malformed. This does not appear to have been intentional.

R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/7407046
parent 6ecb39fc
...@@ -157,7 +157,7 @@ var respTests = []respTest{ ...@@ -157,7 +157,7 @@ var respTests = []respTest{
"Content-Length: 10\r\n" + "Content-Length: 10\r\n" +
"\r\n" + "\r\n" +
"0a\r\n" + "0a\r\n" +
"Body here\n" + "Body here\n\r\n" +
"0\r\n" + "0\r\n" +
"\r\n", "\r\n",
...@@ -327,13 +327,10 @@ var respTests = []respTest{ ...@@ -327,13 +327,10 @@ var respTests = []respTest{
} }
func TestReadResponse(t *testing.T) { func TestReadResponse(t *testing.T) {
for i := range respTests { for i, tt := range respTests {
tt := &respTests[i] resp, err := ReadResponse(bufio.NewReader(strings.NewReader(tt.Raw)), tt.Resp.Request)
var braw bytes.Buffer
braw.WriteString(tt.Raw)
resp, err := ReadResponse(bufio.NewReader(&braw), tt.Resp.Request)
if err != nil { if err != nil {
t.Errorf("#%d: %s", i, err) t.Errorf("#%d: %v", i, err)
continue continue
} }
rbody := resp.Body rbody := resp.Body
...@@ -341,7 +338,11 @@ func TestReadResponse(t *testing.T) { ...@@ -341,7 +338,11 @@ func TestReadResponse(t *testing.T) {
diff(t, fmt.Sprintf("#%d Response", i), resp, &tt.Resp) diff(t, fmt.Sprintf("#%d Response", i), resp, &tt.Resp)
var bout bytes.Buffer var bout bytes.Buffer
if rbody != nil { if rbody != nil {
io.Copy(&bout, rbody) _, err = io.Copy(&bout, rbody)
if err != nil {
t.Errorf("#%d: %v", i, err)
continue
}
rbody.Close() rbody.Close()
} }
body := bout.String() body := bout.String()
...@@ -351,6 +352,22 @@ func TestReadResponse(t *testing.T) { ...@@ -351,6 +352,22 @@ func TestReadResponse(t *testing.T) {
} }
} }
func TestWriteResponse(t *testing.T) {
for i, tt := range respTests {
resp, err := ReadResponse(bufio.NewReader(strings.NewReader(tt.Raw)), tt.Resp.Request)
if err != nil {
t.Errorf("#%d: %v", i, err)
continue
}
bout := bytes.NewBuffer(nil)
err = resp.Write(bout)
if err != nil {
t.Errorf("#%d: %v", i, err)
continue
}
}
}
var readResponseCloseInMiddleTests = []struct { var readResponseCloseInMiddleTests = []struct {
chunked, compressed bool chunked, compressed bool
}{ }{
......
...@@ -209,7 +209,7 @@ func (t *transferWriter) WriteBody(w io.Writer) (err error) { ...@@ -209,7 +209,7 @@ func (t *transferWriter) WriteBody(w io.Writer) (err error) {
} }
} }
if t.ContentLength != -1 && t.ContentLength != ncopy { if !t.ResponseToHEAD && t.ContentLength != -1 && t.ContentLength != ncopy {
return fmt.Errorf("http: Request.ContentLength=%d with Body length %d", return fmt.Errorf("http: Request.ContentLength=%d with Body length %d",
t.ContentLength, ncopy) t.ContentLength, ncopy)
} }
...@@ -327,7 +327,11 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err error) { ...@@ -327,7 +327,11 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err error) {
// or close connection when finished, since multipart is not supported yet // or close connection when finished, since multipart is not supported yet
switch { switch {
case chunked(t.TransferEncoding): case chunked(t.TransferEncoding):
t.Body = &body{Reader: newChunkedReader(r), hdr: msg, r: r, closing: t.Close} if noBodyExpected(t.RequestMethod) {
t.Body = &body{Reader: io.LimitReader(r, 0), closing: t.Close}
} else {
t.Body = &body{Reader: newChunkedReader(r), hdr: msg, r: r, closing: t.Close}
}
case realLength >= 0: case realLength >= 0:
// TODO: limit the Content-Length. This is an easy DoS vector. // TODO: limit the Content-Length. This is an easy DoS vector.
t.Body = &body{Reader: io.LimitReader(r, realLength), closing: t.Close} t.Body = &body{Reader: io.LimitReader(r, realLength), closing: t.Close}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment