From a0e6f03add91af28cce2f2f5a8ec5d2e859de6f4 Mon Sep 17 00:00:00 2001 From: Petar Maymounkov Date: Thu, 28 Jan 2010 15:13:26 -0800 Subject: [PATCH] Cosmetic bug or compliance fixes in http.Response. (1) http.Response must close resp.Body after writing. (2) Case when resp.Body != nil and resp.ContentLength = 0 should not be treated as an error in Response.Write, because this is what ReadResponse often returns. (3) Changed body.th to body.hdr for readability. R=rsc CC=golang-dev https://golang.org/cl/194084 --- src/pkg/http/request.go | 2 +- src/pkg/http/response.go | 30 ++++++++++++++++-------------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/pkg/http/request.go b/src/pkg/http/request.go index 5842afa61bd..57ea6d0aced 100644 --- a/src/pkg/http/request.go +++ b/src/pkg/http/request.go @@ -574,7 +574,7 @@ func ReadRequest(b *bufio.Reader) (req *Request, err os.Error) { // A message body exists when either Content-Length or Transfer-Encoding // headers are present. Transfer-Encoding trumps Content-Length. if v, present := req.Header["Transfer-Encoding"]; present && v == "chunked" { - req.Body = &body{Reader: newChunkedReader(b), th: req, r: b, closing: req.Close} + req.Body = &body{Reader: newChunkedReader(b), hdr: req, r: b, closing: req.Close} } else if v, present := req.Header["Content-Length"]; present { length, err := strconv.Btoi64(v, 10) if err != nil { diff --git a/src/pkg/http/response.go b/src/pkg/http/response.go index 9a2355ff4a1..ba7e95ee28e 100644 --- a/src/pkg/http/response.go +++ b/src/pkg/http/response.go @@ -112,6 +112,7 @@ func ReadResponse(r *bufio.Reader, requestMethod string) (resp *Response, err os fixPragmaCacheControl(resp.Header) + // Transfer encoding, content length resp.TransferEncoding, err = fixTransferEncoding(resp.Header) if err != nil { return nil, err @@ -123,8 +124,10 @@ func ReadResponse(r *bufio.Reader, requestMethod string) (resp *Response, err os return nil, err } + // Closing resp.Close = shouldClose(resp.ProtoMajor, resp.ProtoMinor, resp.Header) + // Trailer resp.Trailer, err = fixTrailer(resp.Header, resp.TransferEncoding) if err != nil { return nil, err @@ -134,7 +137,7 @@ func ReadResponse(r *bufio.Reader, requestMethod string) (resp *Response, err os // or close connection when finished, since multipart is not supported yet switch { case chunked(resp.TransferEncoding): - resp.Body = &body{Reader: newChunkedReader(r), th: resp, r: r, closing: resp.Close} + resp.Body = &body{Reader: newChunkedReader(r), hdr: resp, r: r, closing: resp.Close} case resp.ContentLength >= 0: resp.Body = &body{Reader: io.LimitReader(r, resp.ContentLength), closing: resp.Close} default: @@ -149,13 +152,13 @@ func ReadResponse(r *bufio.Reader, requestMethod string) (resp *Response, err os // and then reads the trailer if necessary. type body struct { io.Reader - th interface{} // non-nil (Response or Request) value means read trailer + hdr interface{} // non-nil (Response or Request) value means read trailer r *bufio.Reader // underlying wire-format reader for the trailer closing bool // is the connection to be closed after reading body? } func (b *body) Close() os.Error { - if b.th == nil && b.closing { + if b.hdr == nil && b.closing { // no trailer and closing the connection next. // no point in reading to EOF. return nil @@ -172,7 +175,7 @@ func (b *body) Close() os.Error { } return err } - if b.th == nil { // not reading trailer + if b.hdr == nil { // not reading trailer return nil } @@ -378,7 +381,7 @@ func (r *Response) ProtoAtLeast(major, minor int) bool { // func (resp *Response) Write(w io.Writer) os.Error { - // RequestMethod should be lower-case + // RequestMethod should be upper-case resp.RequestMethod = strings.ToUpper(resp.RequestMethod) // Status line @@ -404,13 +407,7 @@ func (resp *Response) Write(w io.Writer) os.Error { } if chunked(resp.TransferEncoding) { resp.ContentLength = -1 - } else if resp.Body != nil { - // For safety, consider sending a 0-length body an - // error - if resp.ContentLength <= 0 { - return &ProtocolError{"zero body length"} - } - } else { // no chunking, no body + } else if resp.Body == nil { // no chunking, no body resp.ContentLength = 0 } } @@ -421,8 +418,10 @@ func (resp *Response) Write(w io.Writer) os.Error { if chunked(resp.TransferEncoding) { io.WriteString(w, "Transfer-Encoding: chunked\r\n") } else { - io.WriteString(w, "Content-Length: ") - io.WriteString(w, strconv.Itoa64(resp.ContentLength)+"\r\n") + if resp.ContentLength > 0 || resp.RequestMethod == "HEAD" { + io.WriteString(w, "Content-Length: ") + io.WriteString(w, strconv.Itoa64(resp.ContentLength)+"\r\n") + } } if resp.Header != nil { resp.Header["Content-Length"] = "", false @@ -478,6 +477,9 @@ func (resp *Response) Write(w io.Writer) os.Error { if err != nil { return err } + if err = resp.Body.Close(); err != nil { + return err + } } // TODO(petar): Place trailer writer code here.