Skip to content
This repository

Do not remove empty GET/POST parameters #585

Closed
wants to merge 2 commits into from

5 participants

Yusuf Simonson Don't Add Me To Your Organization a.k.a The Travis Bot Victor Miroshnikov Ben Darnell Jonathan Camp
Yusuf Simonson

Is there a reason why tornado currently removes empty GET/POST parameters? It is causing issues with validating forms generated by formalchemy, because formalchemy expects GET/POST parameters to be there, even if they are empty.

Don't Add Me To Your Organization a.k.a The Travis Bot

This pull request passes (merged ebf17aa into 2b07385).

Victor Miroshnikov

Especially useful for OAuth basestring calculation, when you need list of all parameters even if they have empty values.

Ben Darnell
Collaborator

This has come up several times before and I think it's a good change (with the exception that I'm not sure whether the singular RequestHandler.get_argument should treat empty arguments as unspecified). It is potentially backwards-compatibile, though, so I'd like to hold off until tornado 3.0 (which is not that far off - I'm going to do a 2.4 release soon and then next will probably be 3.0).

Ben Darnell bdarnell closed this pull request from a commit September 09, 2012
Ben Darnell Merge commit 'ebf17aa'
Closes #585.
f1ae398
Ben Darnell bdarnell closed this in f1ae398 September 09, 2012
Jonathan Camp

FYI, this pull does not enable empty POST parameters. I've submitted a pull here with the update: #614

Victor Miroshnikov

@kung-foo nice catch, but still incomplete :P your pull does not enable empty POST parameters for requests with Content-Type multipart/form-data

Jonathan Camp

ah yes. now I remember starting to look into the multipart processing code and thinking, wtf. nope. :neutral_face:

Victor Miroshnikov

@kung-foo false alarm :blush: your patch seems to be fine. I've checked again parse_multipart_form_data implementation and figured out that there is no filter for empty values on multipart data forms. maybe need to add tests for this case.

Sriram Velamur techiev2 referenced this pull request from a commit March 12, 2013
Commit has since been removed from the repository and is no longer available.
Rudd-O Rudd-O referenced this pull request from a commit in Rudd-O/tornado September 09, 2012
Ben Darnell Merge commit 'ebf17aa'
Closes #585.
9cbe1c3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
39  tornado/auth.py
@@ -230,20 +230,14 @@ def authorize_redirect(self, callback_uri=None, extra_params=None,
230 230
             raise Exception("This service does not support oauth_callback")
231 231
         if http_client is None:
232 232
             http_client = httpclient.AsyncHTTPClient()
233  
-        if getattr(self, "_OAUTH_VERSION", "1.0a") == "1.0a":
234  
-            http_client.fetch(
235  
-                self._oauth_request_token_url(callback_uri=callback_uri,
236  
-                                              extra_params=extra_params),
237  
-                self.async_callback(
238  
-                    self._on_request_token,
239  
-                    self._OAUTH_AUTHORIZE_URL,
240  
-                callback_uri))
241  
-        else:
242  
-            http_client.fetch(
243  
-                self._oauth_request_token_url(),
244  
-                self.async_callback(
245  
-                    self._on_request_token, self._OAUTH_AUTHORIZE_URL,
246  
-                    callback_uri))
  233
+
  234
+        request_token_url = self._oauth_request_token_url(
  235
+            callback_uri=callback_uri, extra_params=extra_params)
  236
+
  237
+        callback = self.async_callback(
  238
+            self._on_request_token, self._OAUTH_AUTHORIZE_URL, callback_uri)
  239
+
  240
+        http_client.fetch(request_token_url, callback)
247 241
 
248 242
     def get_authenticated_user(self, callback, http_client=None):
249 243
         """Gets the OAuth authorized user and access token on callback.
@@ -288,14 +282,17 @@ def _oauth_request_token_url(self, callback_uri=None, extra_params=None):
288 282
             oauth_nonce=escape.to_basestring(binascii.b2a_hex(uuid.uuid4().bytes)),
289 283
             oauth_version=getattr(self, "_OAUTH_VERSION", "1.0a"),
290 284
         )
  285
+
  286
+        if callback_uri == "oob":
  287
+            args["oauth_callback"] = "oob"
  288
+        elif callback_uri:
  289
+            args["oauth_callback"] = urlparse.urljoin(
  290
+                self.request.full_url(), callback_uri)
  291
+        
  292
+        if extra_params:
  293
+            args.update(extra_params)
  294
+        
291 295
         if getattr(self, "_OAUTH_VERSION", "1.0a") == "1.0a":
292  
-            if callback_uri == "oob":
293  
-                args["oauth_callback"] = "oob"
294  
-            elif callback_uri:
295  
-                args["oauth_callback"] = urlparse.urljoin(
296  
-                    self.request.full_url(), callback_uri)
297  
-            if extra_params:
298  
-                args.update(extra_params)
299 296
             signature = _oauth10a_signature(consumer_token, "GET", url, args)
300 297
         else:
301 298
             signature = _oauth_signature(consumer_token, "GET", url, args)
7  tornado/httpserver.py
@@ -382,12 +382,7 @@ def __init__(self, method, uri, version="HTTP/1.0", headers=None,
382 382
         self._finish_time = None
383 383
 
384 384
         self.path, sep, self.query = uri.partition('?')
385  
-        arguments = parse_qs_bytes(self.query)
386  
-        self.arguments = {}
387  
-        for name, values in arguments.iteritems():
388  
-            values = [v for v in values if v]
389  
-            if values:
390  
-                self.arguments[name] = values
  385
+        self.arguments = parse_qs_bytes(self.query, keep_blank_values=True)
391 386
 
392 387
     def supports_http_1_1(self):
393 388
         """Returns True if this request supports HTTP/1.1 semantics"""
5  tornado/test/httpserver_test.py
@@ -300,6 +300,11 @@ def test_query_string_encoding(self):
300 300
         data = json_decode(response.body)
301 301
         self.assertEqual(data, {u"foo": [u"\u00e9"]})
302 302
 
  303
+    def test_empty_query_string(self):
  304
+        response = self.fetch("/echo?foo=&foo=")
  305
+        data = json_decode(response.body)
  306
+        self.assertEqual(data, {u"foo": [u"", u""]})
  307
+
303 308
     def test_types(self):
304 309
         headers = {"Cookie": "foo=bar"}
305 310
         response = self.fetch("/typecheck?foo=bar", headers=headers)
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.