code review 4523058: http: fix two Transport gzip+persist crashes

http: fix two Transport gzip+persist crashes

There were a couple issues:

-- HEAD requests were attempting to be ungzipped,
   despite having no content.  That was fixed in
   the previous patch version, but ultimately was
   fixed as a result of other refactoring:

-- persist.go's ClientConn "lastbody" field was
   remembering the wrong body, since we were
   mucking with it later. Instead, ditch
   ClientConn's readRes func field and add a new
   method passing it in, so we can use a closure
   and do all our bodyEOFSignal + gunzip stuff
   in one place, simplifying a lot of code and
   not requiring messing with ClientConn's innards.

-- closing the gzip reader didn't consume its
   contents.  if the caller wasn't done reading
   all the response body and ClientConn closed it
   (thinking it'd move past those bytes in the
   TCP stream), it actually wouldn't.  so introduce
   a new wrapper just for gzip reader to have its
   Close method do an ioutil.Discard on its body
   first, before the close.

Fixes issue 1725
Fixes issue 1804

[bradfitz, 2011-05-11 22:45:28]

Hello rsc, eivind@uggedal.com (cc: golang-dev@googlegroups.com),

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

[bradfitz, 2011-05-12 00:13:19]

Hello rsc, eivind@uggedal.com (cc: golang-dev@googlegroups.com),

Please take another look.

[bradfitz, 2011-05-12 00:15:08]

(I changed the commit message substantially (it ended up fixing two issues),
but codereview didn't remail that part....)

http: fix two Transport gzip+persist crashes

There were a couple issues:

-- HEAD requests were attempting to be ungzipped,
   despite having no content.  That was fixed in
   the previous patch version, but ultimately was
   fixed as a result of other refactoring:

-- persist.go's ClientConn "lastbody" field was
   remembering the wrong body, since we were
   mucking with it later. Instead, ditch
   ClientConn's readRes func field and add a new
   method passing it in, so we can use a closure
   and do all our bodyEOFSignal + gunzip stuff
   in one place, simplifying a lot of code and
   not requiring messing with ClientConn's innards.

-- closing the gzip reader didn't consume its
   contents.  if the caller wasn't done reading
   all the response body and ClientConn closed it
   (thinking it'd move past those bytes in the
   TCP stream), it actually wouldn't.  so introduce
   a new wrapper just for gzip reader to have its
   Close method do an ioutil.Discard on its body
   first, before the close.

Fixes issue 1725
Fixes issue 1804


On Wed, May 11, 2011 at 5:13 PM, <bradfitz@golang.org> wrote:

> Hello rsc, eivind@uggedal.com (cc: golang-dev@googlegroups.com),
>
> Please take another look.
>
>
> http://codereview.appspot.com/4523058/
>

[rsc, 2011-05-12 02:25:37]

LGTM

Please let me submit it.

[rsc, 2011-05-12 02:33:20]

*** Submitted as http://code.google.com/p/go/source/detail?r=ea6ff96a4c34 ***

http: fix two Transport gzip+persist crashes

There were a couple issues:

-- HEAD requests were attempting to be ungzipped,
   despite having no content.  That was fixed in
   the previous patch version, but ultimately was
   fixed as a result of other refactoring:

-- persist.go's ClientConn "lastbody" field was
   remembering the wrong body, since we were
   mucking with it later. Instead, ditch
   ClientConn's readRes func field and add a new
   method passing it in, so we can use a closure
   and do all our bodyEOFSignal + gunzip stuff
   in one place, simplifying a lot of code and
   not requiring messing with ClientConn's innards.

-- closing the gzip reader didn't consume its
   contents.  if the caller wasn't done reading
   all the response body and ClientConn closed it
   (thinking it'd move past those bytes in the
   TCP stream), it actually wouldn't.  so introduce
   a new wrapper just for gzip reader to have its
   Close method do an ioutil.Discard on its body
   first, before the close.

Fixes issue 1725
Fixes issue 1804

R=rsc, eivind
CC=golang-dev
http://codereview.appspot.com/4523058

Committer: Russ Cox <rsc@golang.org>