From fb92c22686921fe32fbd029330d60869b405aa90 Mon Sep 17 00:00:00 2001 From: Jason Campbell Date: Wed, 29 Oct 2014 09:01:21 +1100 Subject: [PATCH] Added support to handle empty query parameters. Added support to handle empty query parameters through a global option set ``RequestOptions`` applied to ``API``. This option can be enabled by using api.req_options.keep_blank_qs_values = True where api is an instance of ``API``. This change allows queries such as '?a=1&bool' and '?a=1&empty=' to preserve the 'bool' and 'empty' values respectively as '' instead of the default value of None. Another option was added to ``Request.get_param_as_bool`` to allow empty query parameters to return a True value. Because of the non-obvious nature of this change ('' == True), it requires a blank_as_true=True argument be passed to ``Request.get_param_as_bool``. --- falcon/api.py | 9 ++-- falcon/request.py | 39 +++++++++++++++-- falcon/util/uri.py | 18 +++++--- tests/test_options.py | 21 +++++++++ tests/test_query_params.py | 88 +++++++++++++++++++++++++++++++++++--- 5 files changed, 154 insertions(+), 21 deletions(-) create mode 100644 tests/test_options.py diff --git a/falcon/api.py b/falcon/api.py index 4a0b05f..122ddfc 100644 --- a/falcon/api.py +++ b/falcon/api.py @@ -15,7 +15,7 @@ import re from falcon import api_helpers as helpers -from falcon.request import Request +from falcon.request import Request, RequestOptions from falcon.response import Response import falcon.responders from falcon.status_codes import HTTP_416 @@ -52,8 +52,8 @@ class API(object): """ __slots__ = ('_after', '_before', '_request_type', '_response_type', - '_error_handlers', '_media_type', - '_routes', '_sinks', '_serialize_error') + '_error_handlers', '_media_type', '_routes', '_sinks', + '_serialize_error', 'req_options') def __init__(self, media_type=DEFAULT_MEDIA_TYPE, before=None, after=None, request_type=Request, response_type=Response): @@ -69,6 +69,7 @@ class API(object): self._error_handlers = [] self._serialize_error = helpers.serialize_error + self.req_options = RequestOptions() def __call__(self, env, start_response): """WSGI `app` method. @@ -86,7 +87,7 @@ class API(object): """ - req = self._request_type(env) + req = self._request_type(env, options=self.req_options) resp = self._response_type() resource = None diff --git a/falcon/request.py b/falcon/request.py index 42abf8b..81cf1a0 100644 --- a/falcon/request.py +++ b/falcon/request.py @@ -52,6 +52,7 @@ class Request(object): Args: env (dict): A WSGI environment dict passed in from the server. See also the PEP-3333 spec. + options (dict): Set of global options passed from the API handler. Attributes: protocol (str): Either 'http' or 'https'. @@ -157,6 +158,8 @@ class Request(object): values. Where the parameter appears multiple times in the query string, the value mapped to that parameter key will be a list of all the values in the order seen. + + options (dict): Set of global options passed from the API handler. """ __slots__ = ( @@ -172,13 +175,15 @@ class Request(object): 'stream', 'context', '_wsgierrors', + 'options', ) # Allow child classes to override this context_type = None - def __init__(self, env): + def __init__(self, env, options=None): self.env = env + self.options = options if options else RequestOptions() if self.context_type is None: # Literal syntax is more efficient than using dict() @@ -210,7 +215,10 @@ class Request(object): if query_str: self.query_string = uri.decode(query_str) - self._params = uri.parse_query_string(self.query_string) + self._params = uri.parse_query_string( + self.query_string, + keep_blank_qs_values=self.options.keep_blank_qs_values, + ) else: self.query_string = six.text_type() @@ -664,7 +672,8 @@ class Request(object): raise HTTPMissingParam(name) - def get_param_as_bool(self, name, required=False, store=None): + def get_param_as_bool(self, name, required=False, store=None, + blank_as_true=False): """Return the value of a query string parameter as a boolean The following bool-like strings are supported:: @@ -680,6 +689,9 @@ class Request(object): store (dict, optional): A dict-like object in which to place the value of the param, but only if the param is found (default *None*). + blank_as_true (bool): If True, empty strings will be treated as + True. keep_blank_qs_values must be set on the Request (or API + object and inherited) for empty strings to not be filtered. Returns: bool: The value of the param if it is found and can be converted @@ -705,6 +717,8 @@ class Request(object): val = True elif val in FALSE_STRINGS: val = False + elif blank_as_true and not val: + val = True else: msg = 'The value of the parameter must be "true" or "false".' raise HTTPInvalidParam(msg, name) @@ -857,5 +871,22 @@ class Request(object): 'will be ignored.') if body: - extra_params = uri.parse_query_string(uri.decode(body)) + extra_params = uri.parse_query_string( + uri.decode(body), + keep_blank_qs_values=self.options.keep_blank_qs_values, + ) + self._params.update(extra_params) + + +class RequestOptions(object): + """This class is a container for Request options. + + PERF: To avoid typos and improve storage space and speed over a dict. + """ + __slots__ = ( + 'keep_blank_qs_values', + ) + + def __init__(self): + self.keep_blank_qs_values = False diff --git a/falcon/util/uri.py b/falcon/util/uri.py index 9d24ad1..3c675a6 100644 --- a/falcon/util/uri.py +++ b/falcon/util/uri.py @@ -245,12 +245,13 @@ else: # pragma: no cover return decoded_uri.decode('utf-8', 'replace') -def parse_query_string(query_string): +def parse_query_string(query_string, keep_blank_qs_values=False): """Parse a query string into a dict. Query string parameters are assumed to use standard form-encoding. Only parameters with values are parsed. for example, given "foo=bar&flag", - this function would ignore "flag". + this function would ignore "flag" unless the keep_blank_qs_values option + is set. Note: In addition to the standard HTML form-based method for specifying @@ -263,6 +264,8 @@ def parse_query_string(query_string): Args: query_string (str): The query string to parse + keep_blank_qs_values (bool): If set to True, preserves boolean fields + and fields with no content as blank strings. Returns: dict: A dict containing ``(name, value)`` tuples, one per query @@ -281,7 +284,7 @@ def parse_query_string(query_string): # and on PyPy 2.3. for field in query_string.split('&'): k, _, v = field.partition('=') - if not v: + if not (v or keep_blank_qs_values): continue if k in params: @@ -302,10 +305,11 @@ def parse_query_string(query_string): # point. v = v.split(',') - # NOTE(kgriffs): Normalize the result in the case that - # some elements are empty strings, such that the result - # will be the same for 'foo=1,,3' as 'foo=1&foo=&foo=3'. - v = [element for element in v if element] + if not keep_blank_qs_values: + # NOTE(kgriffs): Normalize the result in the case that + # some elements are empty strings, such that the result + # will be the same for 'foo=1,,3' as 'foo=1&foo=&foo=3'. + v = [element for element in v if element] params[k] = v diff --git a/tests/test_options.py b/tests/test_options.py new file mode 100644 index 0000000..b3d9812 --- /dev/null +++ b/tests/test_options.py @@ -0,0 +1,21 @@ +from falcon.request import RequestOptions +import falcon.testing as testing + + +class TestRequestOptions(testing.TestBase): + + def test_correct_options(self): + options = RequestOptions() + self.assertFalse(options.keep_blank_qs_values) + options.keep_blank_qs_values = True + self.assertTrue(options.keep_blank_qs_values) + options.keep_blank_qs_values = False + self.assertFalse(options.keep_blank_qs_values) + + def test_incorrect_options(self): + options = RequestOptions() + + def _assign_invalid(): + options.invalid_option_and_attribute = True + + self.assertRaises(AttributeError, _assign_invalid) diff --git a/tests/test_query_params.py b/tests/test_query_params.py index bcc2556..9f1944a 100644 --- a/tests/test_query_params.py +++ b/tests/test_query_params.py @@ -188,7 +188,7 @@ class _TestQueryParams(testing.TestBase): def test_boolean(self): query_string = ('echo=true&doit=false&bogus=0&bogus2=1&' - 't1=True&f1=False&t2=yes&f2=no') + 't1=True&f1=False&t2=yes&f2=no&blank') self.simulate_request('/', query_string=query_string) req = self.resource.req @@ -212,11 +212,31 @@ class _TestQueryParams(testing.TestBase): self.assertEqual(req.get_param_as_bool('t2'), True) self.assertEqual(req.get_param_as_bool('f1'), False) self.assertEqual(req.get_param_as_bool('f2'), False) + self.assertEqual(req.get_param('blank'), None) store = {} self.assertEqual(req.get_param_as_bool('echo', store=store), True) self.assertEqual(store['echo'], True) + def test_boolean_blank(self): + self.api.req_options.keep_blank_qs_values = True + self.simulate_request( + '/', + query_string='blank&blank2=', + ) + + req = self.resource.req + self.assertEqual(req.get_param('blank'), '') + self.assertEqual(req.get_param('blank2'), '') + self.assertRaises(falcon.HTTPInvalidParam, req.get_param_as_bool, + 'blank') + self.assertRaises(falcon.HTTPInvalidParam, req.get_param_as_bool, + 'blank2') + self.assertEqual(req.get_param_as_bool('blank', blank_as_true=True), + True) + self.assertEqual(req.get_param_as_bool('blank3', blank_as_true=True), + None) + def test_list_type(self): query_string = ('colors=red,green,blue&limit=1' '&list-ish1=f,,x&list-ish2=,0&list-ish3=a,,,b' @@ -261,6 +281,62 @@ class _TestQueryParams(testing.TestBase): self.assertEqual(req.get_param_as_list('limit', store=store), ['1']) self.assertEqual(store['limit'], ['1']) + def test_list_type_blank(self): + query_string = ('colors=red,green,blue&limit=1' + '&list-ish1=f,,x&list-ish2=,0&list-ish3=a,,,b' + '&empty1=&empty2=,&empty3=,,' + '&thing_one=1,,3' + '&thing_two=1&thing_two=&thing_two=3' + '&empty4=&empty4&empty4=' + '&empty5&empty5&empty5') + self.api.req_options.keep_blank_qs_values = True + self.simulate_request( + '/', + query_string=query_string + ) + + req = self.resource.req + + # NOTE(kgriffs): For lists, get_param will return one of the + # elements, but which one it will choose is undefined. + self.assertIn(req.get_param('colors'), ('red', 'green', 'blue')) + + self.assertEqual(req.get_param_as_list('colors'), + ['red', 'green', 'blue']) + self.assertEqual(req.get_param_as_list('limit'), ['1']) + self.assertIs(req.get_param_as_list('marker'), None) + + self.assertEqual(req.get_param_as_list('empty1'), ['']) + self.assertEqual(req.get_param_as_list('empty2'), ['', '']) + self.assertEqual(req.get_param_as_list('empty3'), ['', '', '']) + + self.assertEqual(req.get_param_as_list('list-ish1'), + ['f', '', 'x']) + + # Ensure that '0' doesn't get translated to None + self.assertEqual(req.get_param_as_list('list-ish2'), + ['', '0']) + + # Ensure that '0' doesn't get translated to None + self.assertEqual(req.get_param_as_list('list-ish3'), + ['a', '', '', 'b']) + + # Ensure consistency between list conventions + self.assertEqual(req.get_param_as_list('thing_one'), + ['1', '', '3']) + self.assertEqual(req.get_param_as_list('thing_one'), + req.get_param_as_list('thing_two')) + + store = {} + self.assertEqual(req.get_param_as_list('limit', store=store), ['1']) + self.assertEqual(store['limit'], ['1']) + + # Test empty elements + self.assertEqual(req.get_param_as_list('empty4'), ['', '', '']) + self.assertEqual(req.get_param_as_list('empty5'), ['', '', '']) + self.assertEqual(req.get_param_as_list('empty4'), + req.get_param_as_list('empty5')) + def test_list_transformer(self): query_string = 'coord=1.4,13,15.1&limit=100&things=4,,1' self.simulate_request('/', query_string=query_string) @@ -340,10 +416,10 @@ class _TestQueryParams(testing.TestBase): class PostQueryParams(_TestQueryParams): - def simulate_request(self, path, query_string): + def simulate_request(self, path, query_string, **kwargs): headers = {"Content-Type": "application/x-www-form-urlencoded"} - super(PostQueryParams, self).simulate_request(path, body=query_string, - headers=headers) + super(PostQueryParams, self).simulate_request( + path, body=query_string, headers=headers, **kwargs) def test_non_ascii(self): value = u'\u8c46\u74e3' @@ -355,6 +431,6 @@ class PostQueryParams(_TestQueryParams): class GetQueryParams(_TestQueryParams): - def simulate_request(self, path, query_string): + def simulate_request(self, path, query_string, **kwargs): super(GetQueryParams, self).simulate_request( - path, query_string=query_string) + path, query_string=query_string, **kwargs)