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: Request.ParseForm doesn't support PATCH requests #7454

Closed
mattetti opened this issue Mar 4, 2014 · 6 comments
Closed

net/http: Request.ParseForm doesn't support PATCH requests #7454

mattetti opened this issue Mar 4, 2014 · 6 comments
Labels
FrozenDueToAge
Milestone
Go1.3

Comments

@mattetti
Copy link
Contributor

mattetti commented Mar 4, 2014 

HTTP PATCH requests are defined in this rfc: https://tools.ietf.org/html/rfc5789

The problem is that the current implementation for ParseForm is hardcoded to only parse
forms coming from POST or PUT requests, ignoring PATCH requests:

https://code.google.com/p/go/source/browse/src/pkg/net/http/request.go#731

The PostForm field should also be populated when making PATCH requests, from what I
could see, the underlying parsePostForm function wouldn't have an issue parsing a PATCH
request but we have a condition check that is preventing the parsing:

if r.Method == "POST" || r.Method == "PUT" {
    r.PostForm, err = parsePostForm(r)
}

I would gladly submit a patch but I have yet to understand the process to create a
codereview request.
@robfig
Copy link
Contributor

robfig commented Mar 4, 2014

Comment 1:

Discussion on golang-nuts
https://groups.google.com/d/msg/golang-nuts/GjngdEKsUXA/TIMHxkQkSp8J

@rsc
Copy link
Contributor

rsc commented Mar 4, 2014

Comment 2:

At the very least, as a workaround, you can change r.Method before calling ParseForm and
then change it back. I'd like to see real uses of PATCH before changing the standard
package.

Labels changed: added release-go1.4.

Status changed to Thinking.

@mattetti
Copy link
Contributor Author

mattetti commented Mar 4, 2014

Comment 3:

Thanks for work around suggestion, it's probably better than adding a PUT route.
> I'd like to see real uses of PATCH before changing the standard package.
Certainly, at Splice we track music producer saves. The "saves" are stored as resources
and can be partially updated (set a description for instance) by sending a PATCH
request. A PUT request would not be appropriate since we only make a partial change to
the resource.
Would you like me to explain more in details? Or did I miss the point of your question?

@mattetti
Copy link
Contributor Author

mattetti commented Mar 4, 2014

Comment 4:

I added a Code Review request with a fix: https://golang.org/cl/70990044 
(thanks @nathany for walking me through the process)

@bradfitz
Copy link
Contributor

bradfitz commented Mar 4, 2014

Comment 5:

Chatted with Russ. This is fine for now. I still want to clean a lot of this up and
maybe as part of that we'll just always call parsePostForm if that Content-Type is URL
encoded. But the test in this CL will prevent us from regressing in Go 1.4.

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

Status changed to Accepted.

@bradfitz
Copy link
Contributor

bradfitz commented Mar 4, 2014

Comment 6:

This issue was closed by revision b38320b.

Status changed to Fixed.

@rsc rsc added this to the Go1.3 milestone Apr 14, 2015
@rsc rsc removed the release-go1.3 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.3
Development

No branches or pull requests
5 participants
@mattetti @bradfitz @rsc @robfig @gopherbot