Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/http: Client doesn't correctly set cookies for all requests #3985

Closed
gopherbot opened this issue Aug 21, 2012 · 5 comments
Closed

net/http: Client doesn't correctly set cookies for all requests #3985

gopherbot opened this issue Aug 21, 2012 · 5 comments
Milestone

Comments

@gopherbot
Copy link

by alexandru@mosoi.ro:

What steps will reproduce the problem?
If possible, include a link to a program on play.golang.org.

1. This bug is a duplicate of bug 3511 which was not correctly fixed
https://golang.org/issue/3511

If req.Method == 'POST' (anything other than GET and HEAD) the bug can be still
reproduced.


2. Here is how to patch test to reproduce the failure.

diff -r 1e84edee3397 src/pkg/net/http/client_test.go
--- a/src/pkg/net/http/client_test.go   Tue Aug 21 20:59:02 2012 +1000
+++ b/src/pkg/net/http/client_test.go   Tue Aug 21 22:45:37 2012 +0200
@@ -284,6 +284,10 @@
    req, _ := NewRequest("GET", us, nil)
    client.Do(req) // Note: doesn't hit network
    matchReturnedCookies(t, expectedCookies, tr.req.Cookies())
+
+   req, _ = NewRequest("POST", us, nil)
+   client.Do(req) // Note: doesn't hit network
+   matchReturnedCookies(t, expectedCookies, tr.req.Cookies())
 }
 
 // Just enough correctness for our redirect tests. Uses the URL.Host as the


3. Currently if req.Method == 'POST', send() is invoked which doesn't set the cookies.
The fix to bug 3511 did not set the cookies on all possible code paths.



What is the expected output?

HTTP cookies set for all methods.

What do you see instead?

HTTP cookies are not set for POST, PUT, etc.

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

6g

Which operating system are you using?

Ubuntu 12.04 LTS (with custom go installation).

Which version are you using?  (run 'go version')

go version weekly.2012-03-27 +1e84edee3397
@rsc
Copy link
Contributor

rsc commented Sep 12, 2012

Comment 1:

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

Status changed to Accepted.

@orian
Copy link

orian commented Sep 12, 2012

Comment 2:

https://groups.google.com/forum/?fromgroups=#!searchin/golang-nuts/patch/golang-nuts/OABhPILjjuU/3B_6D39OQDAJ

@bradfitz
Copy link
Contributor

Comment 3:

Thanks for looking into this. Please submit the CLA and submit a patch via codereview,
per the contributing guidelines.

@rsc
Copy link
Contributor

rsc commented Sep 12, 2012

Comment 4:

Labels changed: added go1.1.

@bradfitz
Copy link
Contributor

Comment 5:

This issue was closed by revision 4918e3a.

Status changed to Fixed.

@rsc rsc added this to the Go1.1 milestone Apr 14, 2015
@rsc rsc removed the go1.1 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants