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.

Sign up for GitHub

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

Jump to bottom

net/http: API provides no way to follow 307 redirect for PUT or POST #7912

Closed
gopherbot opened this issue May 1, 2014 · 12 comments
Closed

net/http: API provides no way to follow 307 redirect for PUT or POST #7912

gopherbot opened this issue May 1, 2014 · 12 comments
Labels
FrozenDueToAge Thinking
Milestone
Unplanned

Comments

@gopherbot
Copy link

by mdriley:

307 means "try the same verb on this new URL".

Client allows callers to define a redirect policy (CheckRedirect) for whether a given
HTTP redirect should or shouldn't be followed. However, Client.Do() passes an additional
policy to doFollowingRedirects() for GET, HEAD, PUT, and POST that is called before
CheckRedirect. For PUT and POST, that policy only redirects on 302 and 303. Further,
doFollowingRedirects() explicitly changes the verb from POST or PUT to GET on redirect.
As a result, the API provides no way for callers to follow a 307 redirect and keep the
verb.

I imagine the code was written not to follow these redirects because the spec for this
case says "the user agent MUST NOT automatically redirect the request unless it can
be confirmed by the user". The API should allow the CheckRedirect policy to say
such redirects are okay.

wget and curl both support these redirects.
@ianlancetaylor
Copy link
Contributor

Comment 1:

Labels changed: added repo-main, release-go1.4.

@gopherbot
Copy link
Author

Comment 2 by mdriley:

.Net's HttpWebRequest also supports this behavior if AllowAutoRedirect is set.
http://msdn.microsoft.com/en-us/library/system.net.httpwebrequest.allowautoredirect.aspx

@bradfitz
Copy link
Contributor

bradfitz commented May 1, 2014

Comment 3:

You can do it by hand with by using http.Transport, which doesn't follow redirects.
How would we do this automatically if there's a Request.Body and we get the 307 after
we've written the body?  Or would this only be in the case where the 307 came in
response to a client Expect: 100-continue?

@gopherbot
Copy link
Author

Comment 4 by mdriley:

Sure, this is possible with layers under Client -- but it would be a shame to lose the
convenience Client offers.
The current interface (where body is an io.Reader) would require a 100-continue step.
Given the extra round-trip that shouldn't be the default, so whatever option enables
redirecting PUT/POST would also enable 100-continue. Enabling 100-continue independent
of redirects could be a useful optimization for some callers but is probably a separate
feature request.
An optimization where the body is an io.ReadSeeker is also possible, but leads to cases
that can only be tested based on runtime type and server responses, which seems  fragile.

@rsc
Copy link
Contributor

rsc commented Sep 16, 2014

Comment 5:

It doesn't sound like we know how to do this safely.

Labels changed: added release-none, removed release-go1.4.

Status changed to Thinking.

@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@juliusv
Copy link

juliusv commented May 5, 2015

Is there any intention of fixing this? Go rewriting POST and PUT to GET upon 307 sounds like a clear violation of the HTTP/1.1 RFC: https://tools.ietf.org/html/rfc7231#section-6.4.7

The relevant client.go code that changes the method is here:

go/src/net/http/client.go

Lines 353 to 355 in 339cf98

if ireq.Method == "POST" || ireq.Method == "PUT" {
nreq.Method = "GET"
}

@juliusv juliusv mentioned this issue May 5, 2015
Closed
@bradfitz
Copy link
Contributor

bradfitz commented May 5, 2015

@juliusv, I know where the code is but there is no proposal on how to fix this yet. See earlier comments.

@bradfitz bradfitz mentioned this issue Jan 18, 2016
Closed
@odeke-em odeke-em mentioned this issue Feb 27, 2016
Closed
@cookingkode cookingkode mentioned this issue Jun 13, 2016
Closed
@romeovs romeovs mentioned this issue Oct 21, 2016
Closed
@hairyhenderson
Copy link

Have there been any further updates on this? Is there a timeline or proposal for fixing this? This bug makes it particularly easy for inexperienced Go developers (such as myself) to write buggy HTTP clients. 😞

@bradfitz
Copy link
Contributor

This was fixed recently. It will be in Go 1.8. On phone now. Will close this later with references to fixes. This bug was a lost duplicate.

@hairyhenderson
Copy link

Thank you @bradfitz! This made my day 😁

@drmdrew drmdrew mentioned this issue Nov 19, 2016
Merged
2 tasks
@odeke-em
Copy link
Member

Fixed by CL https://go-review.googlesource.com/29852. @bradfitz I realize you might be tracking/triaging fixed bugs for Go1.8 so I'll let you drop the close hammer on this bug.

@bradfitz
Copy link
Contributor

Yup. Thanks!

@vadmeste vadmeste mentioned this issue Apr 7, 2017
Merged
8 tasks
@golang golang locked and limited conversation to collaborators Nov 23, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Thinking
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
7 participants
@bradfitz @rsc @juliusv @hairyhenderson @ianlancetaylor @odeke-em @gopherbot