Repository: falconry/falcon

Issue:
resp.location double-encodes urlencoded strings
I've got a URL which contains a url-encoded URL as a parameter, and when setting resp.location, that results in it being double-encoded:

resp.location = "http://something?redirect_uri=http%3A%2F%2Fsite"

This should result in:
Location: http://something?redirect_uri=http%3A%2F%2Fsite...

But what actually happens is:
Location: http://something?redirect_uri=http%253A%252F%252Fsite

I worked around by raising a HTTPStatus() object, but this doesn't seem like the ideal situation.

Discussion:
codecov-io: ## [Current coverage](https://codecov.io/gh/falconry/falcon/pull/869?src=pr) is 100% (diff: 100%)
> Merging [#869](https://codecov.io/gh/falconry/falcon/pull/869?src=pr) into [master](https://codecov.io/gh/falconry/falcon/branch/master?src=pr) will not change coverage

```diff
@@           master   #869   diff @@
====================================
  Files          30     30          
  Lines        1864   1874    +10   
  Methods         0      0          
  Messages        0      0          
  Branches      305    309     +4   
====================================
+ Hits         1864   1874    +10   
  Misses          0      0          
  Partials        0      0          
```

> Powered by [Codecov](https://codecov.io?src=pr). Last update [9499207...901718d](https://codecov.io/gh/falconry/falcon/compare/94992071b1b9c6f828bb827b8be092146a514200...901718d0d5f4618a704f341a9bb9b923187c4b3a?src=pr)
kgriffs: [rebased]
kgriffs: [rebased]
fxfitz: LGTM
jmvrbanac: :+1: 

Patch:
diff --git a/falcon/util/uri.py b/falcon/util/uri.py
index fc1acd9..47726da 100644
--- a/falcon/util/uri.py
+++ b/falcon/util/uri.py
@@ -60,6 +60,7 @@ def _create_char_encoder(allowed_chars):
 def _create_str_encoder(is_value):
 
     allowed_chars = _UNRESERVED if is_value else _ALL_ALLOWED
+    allowed_chars_plus_percent = allowed_chars + '%'
     encode_char = _create_char_encoder(allowed_chars)
 
     def encoder(uri):
@@ -67,10 +68,32 @@ def _create_str_encoder(is_value):
         if not uri.rstrip(allowed_chars):
             return uri
 
+        if not uri.rstrip(allowed_chars_plus_percent):
+            # NOTE(kgriffs): There's a good chance the string has already
+            # been escaped. Do one more check to increase our certainty.
+            tokens = uri.split('%')
+            for token in tokens[1:]:
+                hex_octet = token[:2]
+
+                if not len(hex_octet) == 2:
+                    break
+
+                if not (hex_octet[0] in _HEX_DIGITS and
+                        hex_octet[1] in _HEX_DIGITS):
+                    break
+            else:
+                # NOTE(kgriffs): All percent-encoded sequences were
+                # valid, so assume that the string has already been
+                # encoded.
+                return uri
+
+            # NOTE(kgriffs): At this point we know there is at least
+            # one unallowed percent character. We are going to assume
+            # that everything should be encoded. If the string is
+            # partially encoded, the caller will need to normalize it
+            # before passing it in here.
+
         # Convert to a byte array if it is not one already
-        #
-        # NOTE(kgriffs): Code coverage disabled since in Py3K the uri
-        # is always a text type, so we get a failure for that tox env.
         if isinstance(uri, six.text_type):
             uri = uri.encode('utf-8')
 


Test patch:
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 6b5f75d..de32f0a 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -177,6 +177,35 @@ class TestFalconUtils(testtools.TestCase):
                     '?limit=3&e%C3%A7ho=true')
         self.assertEqual(uri.encode(url), expected)
 
+    def test_uri_encode_double(self):
+        url = 'http://example.com/v1/fiz bit/messages'
+        expected = 'http://example.com/v1/fiz%20bit/messages'
+        self.assertEqual(uri.encode(uri.encode(url)), expected)
+
+        url = u'http://example.com/v1/fizbit/messages?limit=3&e\u00e7ho=true'
+        expected = ('http://example.com/v1/fizbit/messages'
+                    '?limit=3&e%C3%A7ho=true')
+        self.assertEqual(uri.encode(uri.encode(url)), expected)
+
+        url = 'http://example.com/v1/fiz%bit/mess%ages/%'
+        expected = 'http://example.com/v1/fiz%25bit/mess%25ages/%25'
+        self.assertEqual(uri.encode(uri.encode(url)), expected)
+
+        url = 'http://example.com/%%'
+        expected = 'http://example.com/%25%25'
+        self.assertEqual(uri.encode(uri.encode(url)), expected)
+
+        # NOTE(kgriffs): Specific example cited in GH issue
+        url = 'http://something?redirect_uri=http%3A%2F%2Fsite'
+        self.assertEqual(uri.encode(url), url)
+
+        hex_digits = 'abcdefABCDEF0123456789'
+        for c1 in hex_digits:
+            for c2 in hex_digits:
+                url = 'http://example.com/%' + c1 + c2
+                encoded = uri.encode(uri.encode(url))
+                self.assertEqual(encoded, url)
+
     def test_uri_encode_value(self):
         self.assertEqual(uri.encode_value('abcd'), 'abcd')
         self.assertEqual(uri.encode_value(u'abcd'), u'abcd')