FAQ
Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
net/http: populate ContentLength in HEAD responses

Also fixes a necessary TODO in the process.

Fixes issue 4126

Please review this at https://codereview.appspot.com/6869053/

Affected files:
M src/pkg/net/http/client_test.go
M src/pkg/net/http/response.go
M src/pkg/net/http/response_test.go
M src/pkg/net/http/server.go
M src/pkg/net/http/transfer.go
M src/pkg/net/http/transport.go
M src/pkg/net/http/transport_test.go


Index: src/pkg/net/http/client_test.go
===================================================================
--- a/src/pkg/net/http/client_test.go
+++ b/src/pkg/net/http/client_test.go
@@ -527,3 +527,38 @@
t.Errorf("wanted error mentioning 127.0.0.1 and badserver; got
error: %v", err)
}
}
+
+// Verify Resposne.ContentLength is populated. http://golang.org/issue/4126
+func TestClientHeadContentLength(t *testing.T) {
+ ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
+ if v := r.FormValue("cl"); v != "" {
+ w.Header().Set("Content-Length", v)
+ }
+ }))
+ defer ts.Close()
+ tests := []struct {
+ suffix string
+ want int64
+ }{
+ {"/?cl=1234", 1234},
+ {"/?cl=0", 0},
+ {"", -1},
+ }
+ for _, tt := range tests {
+ req, _ := NewRequest("HEAD", ts.URL+tt.suffix, nil)
+ res, err := DefaultClient.Do(req)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if res.ContentLength != tt.want {
+ t.Errorf("Content-Length = %d; want %d", res.ContentLength, tt.want)
+ }
+ bs, err := ioutil.ReadAll(res.Body)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(bs) != 0 {
+ t.Errorf("Unexpected content: %q", bs)
+ }
+ }
+}
Index: src/pkg/net/http/response.go
===================================================================
--- a/src/pkg/net/http/response.go
+++ b/src/pkg/net/http/response.go
@@ -49,7 +49,7 @@
Body io.ReadCloser

// ContentLength records the length of the associated content. The
- // value -1 indicates that the length is unknown. Unless RequestMethod
+ // value -1 indicates that the length is unknown. Unless Request.Method
// is "HEAD", values >= 0 indicate that the given number of bytes may
// be read from Body.
ContentLength int64
@@ -178,7 +178,7 @@
// StatusCode
// ProtoMajor
// ProtoMinor
-// RequestMethod
+// Request.Method
// TransferEncoding
// Trailer
// Body
Index: src/pkg/net/http/response_test.go
===================================================================
--- a/src/pkg/net/http/response_test.go
+++ b/src/pkg/net/http/response_test.go
@@ -193,7 +193,7 @@
Request: dummyReq("HEAD"),
Header: Header{},
Close: true,
- ContentLength: 0,
+ ContentLength: -1,
},

"",
Index: src/pkg/net/http/server.go
===================================================================
--- a/src/pkg/net/http/server.go
+++ b/src/pkg/net/http/server.go
@@ -525,7 +525,7 @@
// HTTP/1.0 clients keep their "keep-alive" connections alive, and for
// HTTP/1.1 clients is just as good as the alternative: sending a
// chunked response and immediately sending the zero-length EOF chunk.
- if w.written == 0 && w.header.get("Content-Length") == "" {
+ if w.written == 0 && w.header.get("Content-Length") == "" &&
w.req.Method != "HEAD" {
w.header.Set("Content-Length", "0")
}
// If this was an HTTP/1.0 request with keep-alive and we sent a
Index: src/pkg/net/http/transfer.go
===================================================================
--- a/src/pkg/net/http/transfer.go
+++ b/src/pkg/net/http/transfer.go
@@ -294,10 +294,19 @@
return err
}

- t.ContentLength, err = fixLength(isResponse, t.StatusCode,
t.RequestMethod, t.Header, t.TransferEncoding)
+ realLength, err := fixLength(isResponse, t.StatusCode, t.RequestMethod,
t.Header, t.TransferEncoding)
if err != nil {
return err
}
+ if isResponse && t.RequestMethod == "HEAD" {
+ if n, err := parseContentLength(t.Header.get("Content-Length")); err !=
nil {
+ return err
+ } else {
+ t.ContentLength = n
+ }
+ } else {
+ t.ContentLength = realLength
+ }

// Trailer
t.Trailer, err = fixTrailer(t.Header, t.TransferEncoding)
@@ -310,7 +319,7 @@
// See RFC2616, section 4.4.
switch msg.(type) {
case *Response:
- if t.ContentLength == -1 &&
+ if realLength == -1 &&
!chunked(t.TransferEncoding) &&
bodyAllowedForStatus(t.StatusCode) {
// Unbounded body.
@@ -323,11 +332,11 @@
switch {
case chunked(t.TransferEncoding):
t.Body = &body{Reader: newChunkedReader(r), hdr: msg, r: r, closing:
t.Close}
- case t.ContentLength >= 0:
+ case realLength >= 0:
// TODO: limit the Content-Length. This is an easy DoS vector.
- t.Body = &body{Reader: io.LimitReader(r, t.ContentLength), closing:
t.Close}
+ t.Body = &body{Reader: io.LimitReader(r, realLength), closing: t.Close}
default:
- // t.ContentLength < 0, i.e. "Content-Length" not mentioned in header
+ // realLength < 0, i.e. "Content-Length" not mentioned in header
if t.Close {
// Close semantics (i.e. HTTP/1.0)
t.Body = &body{Reader: r, closing: t.Close}
@@ -434,9 +443,9 @@
// Logic based on Content-Length
cl := strings.TrimSpace(header.get("Content-Length"))
if cl != "" {
- n, err := strconv.ParseInt(cl, 10, 64)
- if err != nil || n < 0 {
- return -1, &badStringError{"bad Content-Length", cl}
+ n, err := parseContentLength(cl)
+ if err != nil {
+ return -1, err
}
return n, nil
} else {
@@ -641,3 +650,18 @@
}
return nil
}
+
+// parseContentLength trims whitespace from s and returns -1 if no value
+// is set, or the value if it's >= 0.
+func parseContentLength(cl string) (int64, error) {
+ cl = strings.TrimSpace(cl)
+ if cl == "" {
+ return -1, nil
+ }
+ n, err := strconv.ParseInt(cl, 10, 64)
+ if err != nil || n < 0 {
+ return 0, &badStringError{"bad Content-Length", cl}
+ }
+ return n, nil
+
+}
Index: src/pkg/net/http/transport.go
===================================================================
--- a/src/pkg/net/http/transport.go
+++ b/src/pkg/net/http/transport.go
@@ -604,10 +604,7 @@
alive = false
}

- // TODO(bradfitz): this hasBody conflicts with the defition
- // above which excludes HEAD requests. Is this one
- // incomplete?
- hasBody := resp != nil && resp.ContentLength != 0
+ hasBody := resp != nil && (rc.req.Method != "HEAD" &&
resp.ContentLength != 0)
var waitForBodyRead chan bool
if hasBody {
lastbody = resp.Body
Index: src/pkg/net/http/transport_test.go
===================================================================
--- a/src/pkg/net/http/transport_test.go
+++ b/src/pkg/net/http/transport_test.go
@@ -464,7 +464,7 @@
if e, g := "123", res.Header.Get("Content-Length"); e != g {
t.Errorf("loop %d: expected Content-Length header of %q, got %q", i, e,
g)
}
- if e, g := int64(0), res.ContentLength; e != g {
+ if e, g := int64(123), res.ContentLength; e != g {
t.Errorf("loop %d: expected res.ContentLength of %v, got %v", i, e, g)
}
}

Search Discussions

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedDec 4, '12 at 1:05a
activeDec 4, '12 at 1:05a
posts1
users1
websitegolang.org

1 user in discussion

Bradfitz: 1 post

People

Translate

site design / logo © 2022 Grokbase