code review 6013049: net/http: add cookies from jar to POST request.

net/http: add cookies from jar to POST request.

The main content of this CL is a test case checking the reported
issue 3511 and a tiny fix for it.  A subsequent CL will refactor
the fix as proposed issue 3511.

Fixes issue 3511.

[volker.dobler, 2012-04-13 00:05:02]

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

I'd like you to review this change to
https://code.google.com/p/go

[smh, 2012-04-13 00:44:38]

While less lines of change but doesn't eliminate the multiple paths with the
same code which doing the small re-factor like the original patch, which results
in all cookie handling being done in one location. The multiple locations /
paths where likely the cause for the initial bug in the first place, so I'd
argue the initial patch is a better overall fix.

http://code.google.com/p/go/issues/attachmentText?id=3511&aid=35110000000&nam...

[volker.dobler, 2012-04-13 06:44:27]

On 2012/04/13 00:44:38, steven.hartland wrote:
> While less lines of change but doesn't eliminate the multiple paths with the
> same code which doing the small re-factor like the original patch, which
results
> in all cookie handling being done in one location. The multiple locations /
> paths where likely the cause for the initial bug in the first place, so I'd
> argue the initial patch is a better overall fix.
> 
>
http://code.google.com/p/go/issues/attachmentText?id=3511&aid=35110000000&nam...

I agree that collecting all this stuff in send() is
a superior solution.  How about proceeding like this
to reach this goal?
 - Add a testcase to this CL first
 - Refactor common cookie code to send (your patch)
 - Remove blackHoleJar which is then useless

V.

[volker.dobler, 2012-04-13 13:31:48]

Hello golang-dev@googlegroups.com, steven.hartland@multiplay.co.uk (cc:
golang-dev@googlegroups.com),

Please take another look.

[smh, 2012-04-13 13:57:15]

On 2012/04/13 06:44:27, volker.dobler wrote:
> I agree that collecting all this stuff in send() is
> a superior solution.  How about proceeding like this
> to reach this goal?
>  - Add a testcase to this CL first
>  - Refactor common cookie code to send (your patch)
>  - Remove blackHoleJar which is then useless

That sounds good to me :)

[bradfitz, 2012-05-21 17:57:08]

LGTM

[bradfitz, 2012-05-21 17:57:23]

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

net/http: add cookies from jar to POST request.

The main content of this CL is a test case checking the reported
issue 3511 and a tiny fix for it.  A subsequent CL will refactor
the fix as proposed issue 3511.

Fixes issue 3511.

R=golang-dev, steven.hartland, bradfitz
CC=golang-dev
http://codereview.appspot.com/6013049

Committer: Brad Fitzpatrick <bradfitz@golang.org>

[volker.dobler, 2012-05-21 21:12:42]

Hi Steven,

Brad committed the CL which includes a test for the bug you found
and the stopgap fix.  I'd like to suggest that you submit your
patch now as a CL (of course after 'hg sync'-ing and running
the tests :-)

Regards,

Volker

On Fri, Apr 13, 2012 at 3:57 PM,  <steven.hartland@multiplay.co.uk> wrote:
> On 2012/04/13 06:44:27, volker.dobler wrote:
>>
>> I agree that collecting all this stuff in send() is
>> a superior solution.  How about proceeding like this
>> to reach this goal?
>>  - Add a testcase to this CL first
>>  - Refactor common cookie code to send (your patch)
>>  - Remove blackHoleJar which is then useless
>
>
> That sounds good to me :)
>
> http://codereview.appspot.com/6013049/



-- 
Dr. Volker Dobler