FAQ
Reviewers: golang-dev1,

Message:
Hello golang-dev@googlegroups.com,

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


Description:
net/http: Transport should return an error when response body ends early

If a server response contains a Content-Length and the body is short,
the Transport should end in io.ErrUnexpectedEOF, not io.EOF.

Fixes issue 5738

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

Affected files:
    M src/pkg/net/http/response_test.go
    M src/pkg/net/http/transfer.go


Index: src/pkg/net/http/response_test.go
===================================================================
--- a/src/pkg/net/http/response_test.go
+++ b/src/pkg/net/http/response_test.go
@@ -565,3 +565,29 @@
     t.Errorf("stutter in status: %s", buf.String())
    }
   }
+
+func TestResponseContentLengthShortBody(t *testing.T) {
+ const shortBody = "Short body, not 123 bytes."
+ br := bufio.NewReader(strings.NewReader("HTTP/1.1 200 OK\r\n" +
+ "Content-Length: 123\r\n" +
+ "\r\n" +
+ shortBody))
+ res, err := ReadResponse(br, &Request{Method: "GET"})
+ if err != nil {
+ t.Fatal(err)
+ }
+ if res.ContentLength != 123 {
+ t.Fatalf("Content-Length = %d; want 123", res.ContentLength)
+ }
+ var buf bytes.Buffer
+ n, err := io.Copy(&buf, res.Body)
+ if n != int64(len(shortBody)) {
+ t.Errorf("Copied %d bytes; want %d, len(%q)", n, len(shortBody),
shortBody)
+ }
+ if buf.String() != shortBody {
+ t.Errorf("Read body %q; want %q", buf.String(), shortBody)
+ }
+ if err != io.ErrUnexpectedEOF {
+ t.Errorf("io.Copy error = %#v; want io.ErrUnexpectedEOF", err)
+ }
+}
Index: src/pkg/net/http/transfer.go
===================================================================
--- a/src/pkg/net/http/transfer.go
+++ b/src/pkg/net/http/transfer.go
@@ -532,13 +532,22 @@
    }
    n, err = b.Reader.Read(p)

- // Read the final trailer once we hit EOF.
- if err == io.EOF && b.hdr != nil {
- if e := b.readTrailer(); e != nil {
- err = e
+ if err == io.EOF {
+ // Chunked case. Read the trailer.
+ if b.hdr != nil {
+ if e := b.readTrailer(); e != nil {
+ err = e
+ }
+ b.hdr = nil
+ } else {
+ // If the server declared the Content-Length, our body is a
LimitedReader
+ // and we need to check whether this EOF arrived early.
+ if lr, ok := b.Reader.(*io.LimitedReader); ok && lr.N > 0 {
+ err = io.ErrUnexpectedEOF
+ }
     }
- b.hdr = nil
    }
+
    return n, err
   }



--

---
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Search Discussions

  • Brad Fitzpatrick at Jun 24, 2013 at 7:55 pm
    ping.

    Will take a review from anybody.


    On Fri, Jun 21, 2013 at 11:43 AM, wrote:

    Reviewers: golang-dev1,

    Message:
    Hello golang-dev@googlegroups.com,

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


    Description:
    net/http: Transport should return an error when response body ends early

    If a server response contains a Content-Length and the body is short,
    the Transport should end in io.ErrUnexpectedEOF, not io.EOF.

    Fixes issue 5738

    Please review this at https://codereview.appspot.**com/10237050/<https://codereview.appspot.com/10237050/>

    Affected files:
    M src/pkg/net/http/response_**test.go
    M src/pkg/net/http/transfer.go


    Index: src/pkg/net/http/response_**test.go
    ==============================**==============================**=======
    --- a/src/pkg/net/http/response_**test.go
    +++ b/src/pkg/net/http/response_**test.go
    @@ -565,3 +565,29 @@
    t.Errorf("stutter in status: %s", buf.String())
    }
    }
    +
    +func TestResponseContentLengthShort**Body(t *testing.T) {
    + const shortBody = "Short body, not 123 bytes."
    + br := bufio.NewReader(strings.**NewReader("HTTP/1.1 200 OK\r\n" +
    + "Content-Length: 123\r\n" +
    + "\r\n" +
    + shortBody))
    + res, err := ReadResponse(br, &Request{Method: "GET"})
    + if err != nil {
    + t.Fatal(err)
    + }
    + if res.ContentLength != 123 {
    + t.Fatalf("Content-Length = %d; want 123",
    res.ContentLength)
    + }
    + var buf bytes.Buffer
    + n, err := io.Copy(&buf, res.Body)
    + if n != int64(len(shortBody)) {
    + t.Errorf("Copied %d bytes; want %d, len(%q)", n,
    len(shortBody), shortBody)
    + }
    + if buf.String() != shortBody {
    + t.Errorf("Read body %q; want %q", buf.String(), shortBody)
    + }
    + if err != io.ErrUnexpectedEOF {
    + t.Errorf("io.Copy error = %#v; want io.ErrUnexpectedEOF",
    err)
    + }
    +}
    Index: src/pkg/net/http/transfer.go
    ==============================**==============================**=======
    --- a/src/pkg/net/http/transfer.go
    +++ b/src/pkg/net/http/transfer.go
    @@ -532,13 +532,22 @@
    }
    n, err = b.Reader.Read(p)

    - // Read the final trailer once we hit EOF.
    - if err == io.EOF && b.hdr != nil {
    - if e := b.readTrailer(); e != nil {
    - err = e
    + if err == io.EOF {
    + // Chunked case. Read the trailer.
    + if b.hdr != nil {
    + if e := b.readTrailer(); e != nil {
    + err = e
    + }
    + b.hdr = nil
    + } else {
    + // If the server declared the Content-Length, our
    body is a LimitedReader
    + // and we need to check whether this EOF arrived
    early.
    + if lr, ok := b.Reader.(*io.LimitedReader); ok &&
    lr.N > 0 {
    + err = io.ErrUnexpectedEOF
    + }
    }
    - b.hdr = nil
    }
    +
    return n, err
    }


    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Kevlar at Jun 24, 2013 at 8:18 pm

    On 2013/06/21 18:43:35, bradfitz wrote:
    Hello mailto:golang-dev@googlegroups.com,
    I'd like you to review this change to
    https://go.googlecode.com/hg/
    LGTM

    Nice.

    https://codereview.appspot.com/10237050/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • R at Jun 24, 2013 at 8:21 pm
    LGTM

    https://codereview.appspot.com/10237050/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Bradfitz at Jun 24, 2013 at 8:28 pm
    *** Submitted as
    https://code.google.com/p/go/source/detail?r=41134e67106d ***

    net/http: Transport should return an error when response body ends early

    If a server response contains a Content-Length and the body is short,
    the Transport should end in io.ErrUnexpectedEOF, not io.EOF.

    Fixes issue 5738

    R=golang-dev, kevlar, r
    CC=golang-dev
    https://codereview.appspot.com/10237050


    https://codereview.appspot.com/10237050/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedJun 21, '13 at 6:43p
activeJun 24, '13 at 8:28p
posts5
users3
websitegolang.org

3 users in discussion

Bradfitz: 3 posts R: 1 post Kevlar: 1 post

People

Translate

site design / logo © 2022 Grokbase