Repository: falconry/falcon
Pull request #1425 (2019-02-04 22:18:28)

Testing framework preserves + in URL paths
When testing using a URL that contains a "+" in the path component (as distinct from the query component), Falcon replaces "+" with a space—this does not appear to be appropriate behavior.

Consider the call

```python
import falcon.testing

client = falcon.testing.TestClient(app)
client.simulate_get("/a+b")
```

for some `app`. This code initiates the sequence of method invocations:

1. `falcon.testing.client.TestClient.simulate_get()`
2. `falcon.testing.client.TestClient.simulate_request()`
3. `falcon.testing.client.simulate_request()`
4. `falcon.testing.helpers.create_environ()`
5. `falcon.util.uri.decode()`

The last method replaces "+" with a space so that the path `/a+b` becomes `/a b`.

As I understand it, plus signs should be unquoted in query strings but not in the path component of a URI. See [urlencode - When to encode space to plus (+) or %20? - Stack Overflow](https://stackoverflow.com/questions/2678551/when-to-encode-space-to-plus-or-20) for a discussion.

This pull request modifies the `decode()` method by adding the parameter `unquote_plus` so that the method signature becomes `decode(encoded_uri, unquote_plus=True)`. This solution is just a suggestion; I welcome alternatives.


Patch:
diff --git a/falcon/util/uri.py b/falcon/util/uri.py
index 38691d3..f5c98ed 100644
--- a/falcon/util/uri.py
+++ b/falcon/util/uri.py
@@ -171,7 +171,7 @@ if compat.PY2:  # NOQA: C901 - Work around a bug in flake8 McCabe scoring
                         for a in _HEX_DIGITS
                         for b in _HEX_DIGITS)
 
-    def decode(encoded_uri):
+    def decode(encoded_uri, unquote_plus=True):
         """Decodes percent-encoded characters in a URI or query string.
 
         This function models the behavior of `urllib.unquote_plus`, but
@@ -192,7 +192,7 @@ if compat.PY2:  # NOQA: C901 - Work around a bug in flake8 McCabe scoring
 
         # PERF(kgriffs): Don't take the time to instantiate a new
         # string unless we have to.
-        if '+' in decoded_uri:
+        if '+' in decoded_uri and unquote_plus:
             decoded_uri = decoded_uri.replace('+', ' ')
 
         # Short-circuit if we can
@@ -235,7 +235,7 @@ else:
                         for a in _HEX_DIGITS
                         for b in _HEX_DIGITS)
 
-    def decode(encoded_uri):
+    def decode(encoded_uri, unquote_plus=True):
         """Decodes percent-encoded characters in a URI or query string.
 
         This function models the behavior of `urllib.parse.unquote_plus`,
@@ -254,7 +254,7 @@ else:
 
         # PERF(kgriffs): Don't take the time to instantiate a new
         # string unless we have to.
-        if '+' in decoded_uri:
+        if '+' in decoded_uri and unquote_plus:
             decoded_uri = decoded_uri.replace('+', ' ')
 
         # Short-circuit if we can


Test patch:
diff --git a/falcon/testing/helpers.py b/falcon/testing/helpers.py
index 39a75d8..df140a1 100644
--- a/falcon/testing/helpers.py
+++ b/falcon/testing/helpers.py
@@ -128,7 +128,7 @@ def create_environ(path='/', query_string='', protocol='HTTP/1.1',
 
     # NOTE(kgriffs): wsgiref, gunicorn, and uWSGI all unescape
     # the paths before setting PATH_INFO
-    path = uri.decode(path)
+    path = uri.decode(path, unquote_plus=False)
 
     if compat.PY3:
         # NOTE(kgriffs): The decoded path may contain UTF-8 characters.
diff --git a/tests/test_utils.py b/tests/test_utils.py
index cac3657..d7eb4a9 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -201,6 +201,20 @@ class TestFalconUtils(object):
             'http://example.com?x=ab%2Bcd%3D42%2C9'
         ) == 'http://example.com?x=ab+cd=42,9'
 
+    def test_uri_decode_unquote_plus(self):
+        assert uri.decode('/disk/lost+found/fd0') == '/disk/lost found/fd0'
+        assert uri.decode('/disk/lost+found/fd0', unquote_plus=True) == (
+            '/disk/lost found/fd0')
+        assert uri.decode('/disk/lost+found/fd0', unquote_plus=False) == (
+            '/disk/lost+found/fd0')
+
+        assert uri.decode('http://example.com?x=ab%2Bcd%3D42%2C9') == (
+            'http://example.com?x=ab+cd=42,9')
+        assert uri.decode('http://example.com?x=ab%2Bcd%3D42%2C9', unquote_plus=True) == (
+            'http://example.com?x=ab+cd=42,9')
+        assert uri.decode('http://example.com?x=ab%2Bcd%3D42%2C9', unquote_plus=False) == (
+            'http://example.com?x=ab+cd=42,9')
+
     def test_prop_uri_encode_models_stdlib_quote(self):
         equiv_quote = functools.partial(
             compat.quote, safe=uri._ALL_ALLOWED
@@ -388,6 +402,10 @@ class TestFalconTestingUtils(object):
         env = testing.create_environ(u'/simple')
         assert env['PATH_INFO'] == '/simple'
 
+    def test_plus_in_path_in_create_environ(self):
+        env = testing.create_environ('/mnt/grub2/lost+found/inode001')
+        assert env['PATH_INFO'] == '/mnt/grub2/lost+found/inode001'
+
     def test_none_header_value_in_create_environ(self):
         env = testing.create_environ('/', headers={'X-Foo': None})
         assert env['HTTP_X_FOO'] == ''