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

proposal: rename http.Request.PostForm and http.Request.PostFormValue #25509

Closed
dchenk opened this issue May 23, 2018 · 3 comments
Closed

proposal: rename http.Request.PostForm and http.Request.PostFormValue #25509

dchenk opened this issue May 23, 2018 · 3 comments
Labels
FrozenDueToAge Proposal v2 An incompatible library change
Milestone

Comments

@dchenk
Copy link
Contributor

dchenk commented May 23, 2018

It looks like the PostForm field and the PostFormValue method of the http.Request struct were at first intended only for the form body of a POST request, as is obvious from the names (and this: https://sourcegraph.com/github.com/golang/go/-/commit/abb3c0618b658a41bf91a087f1737412e93ff6d9). Later on, PATCH requests were included along with POST and PUT requests to be dealt with the same way.

This also explains why the doc comment for PostFormValue (https://godoc.org/net/http#Request.PostFormValue) is wrong: form values sent with the PATCH method are now also included.

I suggest renaming the PostForm field to something like BodyForm, and the PostFormValue function to something like BodyFormValue — the point being that the data is coming from the request body, not form the URI.

(A fix can involve simply adding a BodyFormValue to the Request type and deprecating PostFormValue, and something similar can be done with the PostForm field.)

@gopherbot gopherbot added this to the Proposal milestone May 23, 2018
@davecheney davecheney added the v2 An incompatible library change label May 23, 2018
@meirf
Copy link
Contributor

meirf commented Jun 1, 2018

Aside from this being something for Go2 (we don't want "deprecating" + adding api which adds clutter), I don't see anything wrong with this proposal.

Tagging dislikers (@cznic, @wardenlym, @ucirello) to see if I missing anything. Maybe y'all don't think this change is enough - overhaul of form api? (Maybe you think Parse*Form methods aren't full featured enough or should return the form instead of parse into other fields.) But that would be a separate proposal.


This also explains why the doc comment for PostFormValue is wrong: form values sent with the POST method are now also included.

nit: POST is included in that doc - I assume you mean PATCH is missing.

@dchenk
Copy link
Contributor Author

dchenk commented Jun 1, 2018

@meirf thank you. You’re right, I meant to say that PostFormValue also returns values from PATCH requests. I too don’t understand the downvotes, since the proposal intends only to clean up the API (to make it less confusing) in accordance with changes that have been made to the Request type.

@ianlancetaylor
Copy link
Member

Folding into #5465.

@golang golang locked and limited conversation to collaborators Sep 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Proposal v2 An incompatible library change
Projects
None yet
Development

No branches or pull requests

5 participants