net/http: Transport should err on EOF before declared Content-Length

[gopherbot]

by zarcardfly:

Before filing a bug, please check whether it has been fixed since the
latest release. Search the issue tracker and check that you're running the
latest version of Go:

Run "go version" and compare against
http://golang.org/doc/devel/release.html  If a newer version of Go exists,
install it and retry what you did to reproduce the problem.

Thanks.

What steps will reproduce the problem?
If possible, include a link to a program on play.golang.org.
1. Run http://play.golang.org/p/YMV_AvNz-5  as server
2. Run http://play.golang.org/p/3fx3P_T82B  as client
3. Shutdown the server before response fully transfer
4. Retry again but remove line 29 from  http://play.golang.org/p/YMV_AvNz-5

What is the expected output?
The behaviours of two trials should be homologous, both io.Copy should return a none nil
err.

What do you see instead?
Step 3 io.Copy return a nil err, while Step 4 return errTrailerEOF.
In my opinion it's http package's responsibility to detect a half-baked read error, just
as Step 4 return errTrailerEOF.

Which compiler are you using (5g, 6g, 8g, gccgo)?
6g

Which operating system are you using?
OS X 10.8.3

Which version are you using?  (run 'go version')
[08:02:09]~/git/go1.1/src $ go version
go version go1.1 darwin/amd64

Please provide any additional information below.

[gopherbot]

Comment 2 by zarcardfly:

Step 1,2,3 the server respond with specified Content-Length, Step 4 the server respond
with chunked semantic.
In either case we  have the approach to check a half-baked read.
For first case, a half-baked read is detected when the total readed bytes is less than
Content-Length, but can't read anymore(Read return 0, nil).
For second case, a half-baked read is detected when the trailer is missed, but can't
read anymore.
Tell me if I misunderstand something, thank you.
It seems my previous comment was missed, so I send it again. It it's reduplicative,
please ignore it

[robpike]

Comment 3:

Labels changed: added priority-later, removed priority-triage.

Owner changed to @bradfitz.

Status changed to Accepted.

[bradfitz]

Comment 4:

Changed summary.
The bug is that the HTTP client (type *http.Transport) gets an HTTP response from the
server saying "Content-Length: 400000" and then proceeds to get reads:
2013/06/21 10:58:46 Read = 633, <nil>
2013/06/21 10:58:47 Read = 8192, <nil>
2013/06/21 10:58:47 Read = 8192, <nil>
2013/06/21 10:58:47 Read = 8192, <nil>
2013/06/21 10:58:47 Read = 8192, <nil>
2013/06/21 10:58:48 Read = 8192, <nil>
2013/06/21 10:58:48 Read = 8192, <nil>
We then kill the server, which closes the TCP connection, and the client receives:
2013/06/21 10:58:52 Read = 0, EOF
But because we're not de-chunking, we see (0, EOF) from the TCPConn and we assume that's
just the end of the body and pass that through.
We already do count to make sure we don't over-read past the declared Content-Length
(with an io.LimitReader), but don't count that we under-read.
We should return an io.ErrUnexpectedEOF when we see EOF from the server and we haven't
gotten all the bytes.

[bradfitz]

Comment 5:

Mailed https://golang.org/cl/10237050/

Status changed to Started.

[bradfitz]

Comment 6:

This issue was closed by revision a054028.

Status changed to Fixed.

[alberts]

Comment 7:

Is this worthy of Go 1.1.x?

[ianlancetaylor]

Comment 8:

Brad: add this fix to 1.1.2 (if there is one) or not?  Your call.

[alberts]

Comment 9:

The reason I ask: we've massive increased our use of net/http in client and server code
in the past few months, and we'd usually be happy to get this fix from tip, but due to
the current state of the runtime, we've had to hold at 1.1.x.

[bradfitz]

Comment 10:

While a safe fix, this bug has existed from day 1.  It wasn't urgent the past few years,
so it's probably not urgent for 1.1.2.
There isn't a work-around, though.
I'm neutral on 1.1.2.
fullung, worst case, if you're actually hitting this problem, you could cherry-pick it
into your local build.

[rsc]

Comment 11:

I'm inclined to leave this out of 1.1.2. We've been burned before by
changing net/http semantics in a bug fix release without realizing it.

[davecheney]

Comment 12:

I agree.