Merge pull request #14 from j0hnsmith/fix_escaped_mp_uploadId_invalid_signature

fix invalid signature when params dict used with = in mp uploadId 

2 files changed, 50 insertions, 4 deletions

diff --git a/awsauth.py b/awsauth.py
index c3d5c0a..9414420 100644
--- a/awsauth.py
+++ b/awsauth.py
@@ -2,6 +2,8 @@
 import hmac
 
 from hashlib import sha1 as sha
+import urllib
+
 py3k = False
 try:
     from urlparse import urlparse
@@ -115,9 +117,34 @@ class S3Auth(AuthBase):
         for q in query_args:
             k = q.split('=')[0]
             if k in self.special_params:
-                if params_found:
-                    buf += '&%s' % q
-                else:
-                    buf += '?%s' % q
+                buf += '&' if params_found else '?'
                 params_found = True
+
+                try:
+                    k, v = q.split('=', 1)
+
+                except ValueError:
+                    buf += q
+
+                else:
+                    # Riak CS multipart upload ids look like this, `TFDSheOgTxC2Tsh1qVK73A==`, is should be escaped to
+                    # be included as part of a query string.
+                    #
+                    # A requests mp upload part request may look like
+                    # resp = requests.put(
+                    #     'https://url_here',
+                    #     params={
+                    #         'partNumber': 1,
+                    #         'uploadId': 'TFDSheOgTxC2Tsh1qVK73A=='
+                    #     },
+                    #     data='some data',
+                    #     auth=S3Auth('access_key', 'secret_key')
+                    # )
+                    #
+                    # Requests automatically escapes the values in the `params` dict, so now our uploadId is
+                    # `TFDSheOgTxC2Tsh1qVK73A%3D%3D`, if we sign the request with the encoded value the signature will
+                    # not be valid, we'll get 403 Access Denied.
+                    # So we unquote, this is no-op if the value isn't encoded.
+                    buf += '{}={}'.format(k, urllib.unquote(v))
+
         return buf
diff --git a/test.py b/test.py
index 9d7b3fc..ce38e68 100644
--- a/test.py
+++ b/test.py
@@ -126,5 +126,24 @@ class TestAWS(unittest.TestCase):
         # No Delete ?notification API, empty <NotificationConfiguration>
         # tag is default
 
+    def test_canonical_string_not_using_encoded_query_params(self):
+        url = 'https://bucket.ca.tier3.io/object-name?partNumber=1&uploadId=TFDSheOgTxC2Tsh1qVK73A%3D%3D'
+        headers = {
+            'Content-Length': 0,
+            'Accept-Encoding': 'gzip, deflate',
+            'Accept': '*/*',
+            'User-Agent': 'python-requests/2.7.0 CPython/2.7.6 Linux/3.13.0-24-generic',
+            'Connection': 'keep-alive',
+            'date': 'Fri, 21 Aug 2015 16:08:26 GMT',
+        }
+        method = 'PUT'
+        canonical_string = self.auth.get_canonical_string(url, headers, method)
+        self.assertTrue('TFDSheOgTxC2Tsh1qVK73A%3D%3D' not in canonical_string)
+        self.assertTrue('TFDSheOgTxC2Tsh1qVK73A==' in canonical_string)
+
+        url = 'https://bucket.ca.tier3.io/object-name?partNumber=1&uploadId=not%escaped'
+        canonical_string = self.auth.get_canonical_string(url, headers, method)
+        self.assertTrue('not%escaped' in canonical_string)
+
 if __name__ == '__main__':
     unittest.main()