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

http: POST ignores query params #985

Closed
rsc opened this issue Aug 1, 2010 · 18 comments
Closed

http: POST ignores query params #985

rsc opened this issue Aug 1, 2010 · 18 comments

Comments

@rsc
Copy link
Contributor

rsc commented Aug 1, 2010

What steps will reproduce the problem?
1. Send an HTTP POST with form data in the post but with ?a=b in the URL.
2. Look at req.FormValue("a")

What is the expected output? What do you see instead?
It's empty; I think it should be "b", but I'm not sure.
Would have to check the spec and see what other implementations do.
@dsymonds
Copy link
Contributor

dsymonds commented Aug 1, 2010

Comment 1:

Yes, I'm pretty sure that both the URL and body args should come through. See, for
instance, section 4.3.2 of RFC 3875 (on CGI).

@garyburd
Copy link
Contributor

garyburd commented Aug 6, 2010

Comment 2:

It is legal and sometimes convenient to include both query string and post body
parameters in a single request. 
Other frameworks merge the query string and post body parameters in a single collection.
 Examples include:
- Java Servlets
- Google App Engine webapp framework for Python
- Tornado Web

@rsc
Copy link
Contributor Author

rsc commented Aug 7, 2010

Comment 3:

I think we all agree that it's a bug.

Status changed to HelpWanted.

@gopherbot
Copy link
Contributor

Comment 4 by joerg.sonnenberger:

From the perspective of writing correct web frameworks, merging the worst possible
approach. GET and POST have different semantics and forgetting that can easily create
security issues or undesired side effects due to smart read ahead etc.
I would suggest to strictly distinguish "form" parameters from the query itself and from
the body of a POST in two separate functions. A third interface could be provided for
files later. Precedence for this would be Django.

@rsc
Copy link
Contributor Author

rsc commented Aug 17, 2010

Comment 5:

If you care about get vs post consult req.Method.
Russ

@gopherbot
Copy link
Contributor

Comment 6 by joerg.sonnenberger:

Developers should care and if parameters are merged, you have to check explicitly. 
Let's say you have a simple destructive operation with a simple "confirm" button to
check. What happens if someone else places
<img width="1" height="1"
src="https://missile-launcher.gov/send_nukes?confirm=yes">
on some Facebook page? The validation code has to check that only only are all required
fields present, but also that the method is correct. If experience shows anything in
this regard, it is that the second tends to be forgotten. In short, being explicit which
parameters you mean helps to avoid a common pitfall.

@garyburd
Copy link
Contributor

Comment 7:

A correct application will use Request.Method to determine the HTTP method, not where
the parameters are encoded in the request (query string or request body). 
Using separate maps for query string parameters and request body parameters might lead
to confusion about the distinction between GET and POST.

@rsc
Copy link
Contributor Author

rsc commented Aug 17, 2010

Comment 8:

RFC 3875 seems to imply that they should be merged, as does common sense.
If you can point at an RFC that explains why they should *NOT* be merged,
then please do.

@gopherbot
Copy link
Contributor

Comment 9 by joerg.sonnenberger:

Where you see that in RFC 3875? It only talks about the QUERY_STRING and doesn't mention
the body at all. There is no RFC talking about this as interpretation of the query
string and request body are application defined.
Practical question for merging is what has higher precedence. 
The naming as FormValue would be misleading at best. HTML forms never alter the query
string for method=POST. All I read in RFC 3875 is that both should be provided. For a
more advanced framework, that implies both should be decoded. I don't see a
justification for merging them though.

@rsc
Copy link
Contributor Author

rsc commented Aug 17, 2010

Comment 10:

4.3.2 says
   The POST method is used to request the script perform processing and
   produce a document based on the data in the request message-body, in
   addition to meta-variable values.
The text definitely says both should be available.
Combining the two is a nice, simple interface, which, in the absence
of a compelling reason to the contrary, makes it a better one than
forcing clients to check two different places.  I don't buy the "you'll
forget to check" argument about GET vs POST.  In plenty of cases
you'd be happy to accept parameters via either method and don't care,
and it would be extra effort to implement that.  People launching
missiles should definitely take the extra time to check req.Method,
regardless of the form parameters.
Gary Burd's comment lists other popular web frameworks that
merge the two, suggesting that it's fine to do so in the real world.
This has the potential to devolve into quite a bikeshed discussion.
I'm happy to listen to other (non-GET vs POST) arguments though.
Russ

@gopherbot
Copy link
Contributor

Comment 11 by joerg.sonnenberger:

OK, so we agree that both sets should be available. Good reasons exist for that like
debugging. Some web frameworks provide a merged view, some don't. I don't buy the
simplicity argument, but I can see where the desire comes from.
Is the best approach therefore to provide two maps, one for the query parameters and one
for the (potential) POST parameters? The merged view can be implemented on top and if
code wants to distinguish between the two sets, that possible as well.

@rsc
Copy link
Contributor Author

rsc commented Aug 17, 2010

Comment 12:

I don't see the need for two maps.
I still haven't heard a compelling argument
beyond "you might want that".

@rsc
Copy link
Contributor Author

rsc commented Aug 17, 2010

Comment 13:

Status changed to Started.

@gopherbot
Copy link
Contributor

Comment 14 by joerg.sonnenberger:

Because URI parameters and POST body data differ in visible ways. E.g. POST data is much
less visible as it is not shown in the web browser, it normally doesn't end up in server
log files etc. For URI parameters you normally get a length limit for free, which could
be by-passed if both are parsed and method=GET is not checked.
Distinguishing them can help modularity by separating naturally the parameters that form
the current address and the parameters that describe the current action. Something like
the "starring" of a ticket could be implemented by iterating over the POST keys, match
them against a list of known buttons and executing a handler. The action of the forms
would simply be the current raw URL.

@rsc
Copy link
Contributor Author

rsc commented Aug 17, 2010

Comment 15:

Okay, but why isn't this important enough for the web frameworks Gary Burd listed to
distinguish them?
"method=GET is not checked" is not enough.  method checks are a different issue, one
that could be handled with specific GET vs POST handlers.

@gopherbot
Copy link
Contributor

Comment 16 by joerg.sonnenberger:

I'm not sure. Googling around a bit suggest that many dispute the validity of having
both query parameters and POST body. If that argument was valid, it would hint more at
complaining or ignoring the second set of arguments.
The only reason given for having a single mapping populated from both sets is that it
makes sharing GET/POST with the same handler is easier. I have personally had only one
instance where such a fallback made sense. That was the auto-magic login page for a
Django application. If you are not logged in, you would get a redirect to
/login?next=/you/tried/this... and the form included the URL, so it would have to use
the query parameter for the GET case and the (POST) form data when executing the login.
Looking back, I would now just pass the original URL including all parameters as action
into the login form. That would remove the last useful case I have for automatically
selecting one set or the other.

@rsc
Copy link
Contributor Author

rsc commented Aug 17, 2010

Comment 17:

Personally I just find it easier to put my hard-coded query params
in the URL than to have to write hidden fields in the form.
Russ

@rsc
Copy link
Contributor Author

rsc commented Aug 18, 2010

Comment 18:

This issue was closed by revision 3bf6563.

Status changed to Fixed.

@rsc rsc added the fixed label Aug 18, 2010
@rsc rsc self-assigned this Aug 18, 2010
@golang golang locked and limited conversation to collaborators Jun 24, 2016
@rsc rsc removed their assignment Jun 22, 2022
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