code review 5629061: net/http: Handle 100 Continue responses from servers, i...

net/http: Handle 100 Continue responses from servers, including the
unsolicited 100 Continue that precedes POST/PUT responses from IIS/5.1.
(This is valid for compatability with RFC 2068 according to RFC 2616,
8.2.3.)

Fixes issue 2184.

[pmylund, 2012-02-05 17:09:46]

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

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

[pmylund, 2012-02-05 17:12:08]

I'm wondering: This also handles 100 Continue in general. (The response
"leaking" would be an issue if you set the Expect in a request anyway.) Perhaps
it should only skip the 100 Continue if req.Header doesn't have "Expect" (to
solve issue 2184), but not when somebody explicitly Expects 100-continue? Then
that should be handled another way to avoid causing the same problem.

[rsc1, 2012-02-10 16:17:21]

I'd like to leave this until after Go 1, unless 
Brad feels strongly that it should go in.

I think it should only read at most one continue
header.  That might mean renaming ReadResponse to
readResponse and then making the new ReadResponse
call readResponse, and then if the status is 100,
call it again (but not check for another continue).

Please remind us about this CL after Go 1 is out.
Thanks.

[pmylund, 2012-06-29 02:34:25]

Hello golang-dev@googlegroups.com (cc: bradfitz@golang.org,
golang-dev@googlegroups.com, rsc@golang.org),

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

[pmylund, 2012-06-29 02:41:49]

Hello golang-dev@googlegroups.com (cc: bradfitz@golang.org,
golang-dev@googlegroups.com, rsc@golang.org),

Please take another look.

[pmylund, 2012-06-29 03:47:31]

Hello golang-dev@googlegroups.com (cc: bradfitz@golang.org,
golang-dev@googlegroups.com, rsc@golang.org),

Please take another look.

[pmylund, 2012-06-29 03:53:47]

Ugh. I need to learn to restrain my 'hg mail' trigger finger.

Only major changes from the CL is that I've moved the test from transport_test
where it made no sense to readresponse_test.go (need to be in package http_test
for http/httptest), and I've implemented Russ' suggestion to only skip one 100
Continue header.

[pmylund, 2012-07-02 21:53:56]

Something to consider: The RFC states that 100 Continue responses are allowed
only in response to requests with an Expect: 100-continue header, with the
exception of POST/PUT requests (but only for HTTP/1.1)

Perhaps it's more correct to check the request type and/or presence of that
header, but is it worth the overhead?

[bradfitz, 2012-08-07 04:13:56]

This CL might fix Issue 2184, but any CL should probably also fix Issue 3665, or
at least not be incompatible with its eventual fix.

[pmylund, 2012-08-14 22:00:38]

Just to let you know, I'm planning on doing the former when I have some time,
unless you are already working on it.

[bradfitz, 2012-08-20 05:47:10]

I'm not working on it.  All yours.  :)

On Wed, Aug 15, 2012 at 8:00 AM, <patrick@patrickmn.com> wrote:

> Just to let you know, I'm planning on doing the former when I have some
> time, unless you are already working on it.
>
>
http://codereview.appspot.com/**5629061/<http://codereview.appspot.com/5629061/>
>

[bradfitz, 2013-04-02 02:54:29]

This was fixed by https://code.google.com/p/go/source/detail?r=466040fd273e a
different way.  Doing it here would've broken packages that read a 1xx directly,
like websocket does for 101 upgrades.

[pmylund, 2013-04-02 09:24:23]

On 2013/04/02 02:54:29, bradfitz wrote:
> This was fixed by https://code.google.com/p/go/source/detail?r=466040fd273e a
> different way.  Doing it here would've broken packages that read a 1xx
directly,
> like websocket does for 101 upgrades.

cool, thanks

[pmylund, 2013-04-02 09:32:12]

*** Abandoned ***