Repository: boto/botocore
Pull request #1176 (2017-04-11 05:25:24)

utils.percent_encode fix (with test)
`percent_encode` fails when handed a string that's already been encoded into bytes. Have it just URL-quote that instead.

This fixes an error I'm running into when using S3 via the "django storages" package. I have a few keys with non-ASCII characters. Django storages handles this by encoding them as UTF-8 and handing the bytes off to boto. On Python 2 this fails with a UnicodeDecodeError, and on Python 3 I don't think it will raise an exception, but will return a url-encoded repr of the string passed in (which is probably not what you would want).

codecov-io: # [Codecov](https://codecov.io/gh/boto/botocore/pull/1176?src=pr&el=h1) Report
> Merging [#1176](https://codecov.io/gh/boto/botocore/pull/1176?src=pr&el=desc) into [develop](https://codecov.io/gh/boto/botocore/commit/852e79ca7232b06f899a0b837ff6dbac959dfeab?src=pr&el=desc) will **decrease** coverage by `0.52%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/boto/botocore/pull/1176/graphs/tree.svg?height=150&width=650&token=QOLXw6XSza&src=pr)](https://codecov.io/gh/boto/botocore/pull/1176?src=pr&el=tree)

```diff
@@             Coverage Diff             @@
##           develop    #1176      +/-   ##
===========================================
- Coverage    97.98%   97.45%   -0.53%     
===========================================
  Files           45       45              
  Lines         7186     7188       +2     
===========================================
- Hits          7041     7005      -36     
- Misses         145      183      +38
```


| [Impacted Files](https://codecov.io/gh/boto/botocore/pull/1176?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [botocore/utils.py](https://codecov.io/gh/boto/botocore/pull/1176?src=pr&el=tree#diff-Ym90b2NvcmUvdXRpbHMucHk=) | `95.97% <100%> (+0.01%)` | :arrow_up: |
| [botocore/compat.py](https://codecov.io/gh/boto/botocore/pull/1176?src=pr&el=tree#diff-Ym90b2NvcmUvY29tcGF0LnB5) | `68.14% <0%> (-23.71%)` | :arrow_down: |
| [botocore/configloader.py](https://codecov.io/gh/boto/botocore/pull/1176?src=pr&el=tree#diff-Ym90b2NvcmUvY29uZmlnbG9hZGVyLnB5) | `98.8% <0%> (-1.2%)` | :arrow_down: |
| [botocore/handlers.py](https://codecov.io/gh/boto/botocore/pull/1176?src=pr&el=tree#diff-Ym90b2NvcmUvaGFuZGxlcnMucHk=) | `95.52% <0%> (-0.75%)` | :arrow_down: |
| [botocore/signers.py](https://codecov.io/gh/boto/botocore/pull/1176?src=pr&el=tree#diff-Ym90b2NvcmUvc2lnbmVycy5weQ==) | `97.74% <0%> (-0.57%)` | :arrow_down: |
| [botocore/auth.py](https://codecov.io/gh/boto/botocore/pull/1176?src=pr&el=tree#diff-Ym90b2NvcmUvYXV0aC5weQ==) | `97.85% <0%> (-0.22%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/boto/botocore/pull/1176?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/boto/botocore/pull/1176?src=pr&el=footer). Last update [852e79c...b6fa020](https://codecov.io/gh/boto/botocore/pull/1176?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Patch:
diff --git a/botocore/utils.py b/botocore/utils.py
index b6027975c..ab2a86fad 100644
--- a/botocore/utils.py
+++ b/botocore/utils.py
@@ -318,10 +318,18 @@ def percent_encode(input_str, safe=SAFE_CHARS):
     producing a percent encoded string, this function deals only with
     taking a string (not a dict/sequence) and percent encoding it.
 
+    If given the binary type, will simply URL encode it. If given the
+    text type, will produce the binary type by UTF-8 encoding the
+    text. If given something else, will convert it to the the text type
+    first.
     """
-    if not isinstance(input_str, six.string_types):
+    # If its not a binary or text string, make it a text string.
+    if not isinstance(input_str, (six.binary_type, six.text_type)):
         input_str = six.text_type(input_str)
-    return quote(six.text_type(input_str).encode('utf-8'), safe=safe)
+    # If it's not bytes, make it bytes by UTF-8 encoding it.
+    if not isinstance(input_str, six.binary_type):
+        input_str = input_str.encode('utf-8')
+    return quote(input_str, safe=safe)
 
 
 def parse_timestamp(value):


Test patch:
diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
index f5f3feb02..615225eb5 100644
--- a/tests/unit/test_utils.py
+++ b/tests/unit/test_utils.py
@@ -47,6 +47,7 @@ from botocore.utils import instance_cache
 from botocore.utils import merge_dicts
 from botocore.utils import get_service_module_name
 from botocore.utils import percent_encode_sequence
+from botocore.utils import percent_encode
 from botocore.utils import switch_host_s3_accelerate
 from botocore.utils import deep_merge
 from botocore.utils import S3RegionRedirector
@@ -1057,6 +1058,27 @@ class TestPercentEncodeSequence(unittest.TestCase):
                              ('k2', ['another', 'list'])])),
             'k1=a&k1=list&k2=another&k2=list')
 
+class TestPercentEncode(unittest.TestCase):
+    def test_percent_encode_obj(self):
+        self.assertEqual(percent_encode(1), '1')
+
+    def test_percent_encode_text(self):
+        self.assertEqual(percent_encode(u''), '')
+        self.assertEqual(percent_encode(u'a'), 'a')
+        self.assertEqual(percent_encode(u'\u0000'), '%00')
+        # Codepoint > 0x7f
+        self.assertEqual(percent_encode(u'\u2603'), '%E2%98%83')
+        # Codepoint > 0xffff
+        self.assertEqual(percent_encode(u'\U0001f32e'), '%F0%9F%8C%AE')
+
+    def test_percent_encode_bytes(self):
+        self.assertEqual(percent_encode(b''), '')
+        self.assertEqual(percent_encode(b'a'), u'a')
+        self.assertEqual(percent_encode(b'\x00'), u'%00')
+        # UTF-8 Snowman
+        self.assertEqual(percent_encode(b'\xe2\x98\x83'), '%E2%98%83')
+        # Arbitrary bytes (not valid UTF-8).
+        self.assertEqual(percent_encode(b'\x80\x00'), '%80%00')
 
 class TestSwitchHostS3Accelerate(unittest.TestCase):
     def setUp(self):