Add support for argument origin #600

Closed
wants to merge 3 commits into from

3 participants

@xethorn

The request (HTTPRequest) is providing a unique attribute (arguments) which contains all the arguments regardless of their origin. It's thus not possible to determine where each of them has been set and defined.

I think it would be nice to be able to make a distinction somewhere about those two origins. It would also help to avoid situation where some parameters that should only be sent in the body are entered in the url. It could also help to detect fraudulent accesses (e.g. the xsrf should never be part of the url.)

In the update I've made, I added two things to HTTPRequest:
• url_params: the GET params.
• form_params: the POST params.

Initially, I thought of using self.get, self.post, but seems confusing since the request handler is already using them. get_arguments doesn't work either since a function of the same name already exists. :-/ Let me know what your thoughts are!

Michael Ortali added some commits Sep 30, 2012
Michael Ortali The request (HTTPRequest) is providing a unique attribute (arguments)…
… which contains all the arguments regardless of their origin. It's thus not possible to determine where each of them has been set and defined.

I think it would be nice to be able to make a distinction somewhere about those two origins. It would also help to avoid situation where some parameters that should only be sent in the body are entered in the url. It could also help to detect fraudulent accesses (e.g. the xsrf should never be part of the url.)

In the update I've made, I added two things to HTTPRequest:
• url_params: the GET params.
• form_params: the POST params.

Initially, I thought of using self.get, self.post, but seems confusing since the request handler is already using them. get_arguments doesn't work either since a function of the same name already exists.
c733b77
Michael Ortali request_time should be time 61b8d25
Michael Ortali Update the tests using parse_multipart_form_data. 513d405
@xethorn

Any update for this pull?

@bdarnell bdarnell commented on the diff Oct 15, 2012
tornado/wsgi.py
self.query = environ.get("QUERY_STRING", "")
if self.query:
self.uri += "?" + self.query
self.arguments = parse_qs_bytes(native_str(self.query),
keep_blank_values=True)
+ self.url_arguments = copy.copy(self.arguments)
@bdarnell
tornadoweb member

I think this needs to be a deepcopy because the values are mutable lists (otherwise if the query and body both had values with the same key things would be weird)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@bdarnell
tornadoweb member

The copy.copy issue occurs twice.

I'd suggest changing the names to query_arguments and body_arguments. url_arguments is kind of ambiguous with the path-based arguments extracted from the URLSpec regex.

Could you add a test for this? (both for the general functionality and the specific issue with deepcopy)

@MrTravisB

@xethorn any update on this?

I created a new pull request for this at #907.

@MrTravisB MrTravisB referenced this pull request Sep 28, 2013
Merged

Support argument origin #907

@bdarnell
tornadoweb member

#907 has been merged.

@bdarnell bdarnell closed this Oct 6, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment