Encode special characters in presigned urls (#74)

uc-cdis · Aug 9, 2019 · 45ad662 · 45ad662
1 parent 3b765df
commit 45ad662
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 44 deletions.
diff --git a/cirrus/google_cloud/utils.py b/cirrus/google_cloud/utils.py
@@ -1,6 +1,7 @@
 import re
 import base64
 import json
+from urllib.parse import quote
 
 from oauth2client.service_account import ServiceAccountCredentials
 
@@ -162,6 +163,10 @@ def get_signed_url(
         str: Completed signed URL
     """
     path_to_resource = path_to_resource.strip("/")
+
+    # escape special characters
+    path_to_resource = quote(path_to_resource)
+
     string_to_sign = _get_string_to_sign(
         path_to_resource, http_verb, expires, extension_headers, content_type, md5_value
     )

diff --git a/test/conftest.py b/test/conftest.py
@@ -1,12 +1,6 @@
 import time
 
-# Python 2 and 3 compatible
-try:
-    from unittest.mock import MagicMock
-    from unittest.mock import patch
-except ImportError:
-    from mock import MagicMock
-    from mock import patch
+from unittest.mock import MagicMock, patch
 
 import pytest
 

diff --git a/test/test_google_cloud_manage.py b/test/test_google_cloud_manage.py
@@ -2,15 +2,7 @@
 import datetime
 import json
 
-# Python 2 and 3 compatible
-try:
-    from unittest import mock
-    from unittest.mock import MagicMock
-    from unittest.mock import patch
-except ImportError:
-    import mock
-    from mock import MagicMock
-    from mock import patch
+from unittest.mock import MagicMock, patch
 
 from googleapiclient.errors import HttpError
 import pytest
@@ -1380,9 +1372,7 @@ def test_add_member_backoff_giveup(test_cloud_manager):
     test_cloud_manager._admin_service.configure_mock(**mock_config)
     warn = cirrus.google_cloud.manager.logger.warn
     error = cirrus.google_cloud.manager.logger.error
-    with mock.patch(
-        "cirrus.google_cloud.manager.logger.warn"
-    ) as logger_warn, mock.patch(
+    with patch("cirrus.google_cloud.manager.logger.warn") as logger_warn, patch(
         "cirrus.google_cloud.manager.logger.error"
     ) as logger_error:
         # keep the side effect to actually put logs, so you can see the format with `-s`
@@ -1408,9 +1398,7 @@ def test_authorized_session_retry(test_cloud_manager):
     test_cloud_manager._authed_session.configure_mock(**mock_config)
     warn = cirrus.google_cloud.manager.logger.warn
     error = cirrus.google_cloud.manager.logger.error
-    with mock.patch(
-        "cirrus.google_cloud.manager.logger.warn"
-    ) as logger_warn, mock.patch(
+    with patch("cirrus.google_cloud.manager.logger.warn") as logger_warn, patch(
         "cirrus.google_cloud.manager.logger.error"
     ) as logger_error:
         # keep the side effect to actually put logs, so you can see the format with `-s`
@@ -1435,9 +1423,7 @@ def test_handled_exception_no_retry(test_cloud_manager):
     test_cloud_manager._admin_service.configure_mock(**mock_config)
     warn = cirrus.google_cloud.manager.logger.warn
     error = cirrus.google_cloud.manager.logger.error
-    with mock.patch(
-        "cirrus.google_cloud.manager.logger.warn"
-    ) as logger_warn, mock.patch(
+    with patch("cirrus.google_cloud.manager.logger.warn") as logger_warn, patch(
         "cirrus.google_cloud.manager.logger.error"
     ) as logger_error:
         # keep the side effect to actually put logs, so you can see the format with `-s`
@@ -1469,9 +1455,7 @@ def test_handled_exception_403_no_retry(test_cloud_manager):
     test_cloud_manager._authed_session.configure_mock(**mock_config)
     warn = cirrus.google_cloud.manager.logger.warn
     error = cirrus.google_cloud.manager.logger.error
-    with mock.patch(
-        "cirrus.google_cloud.manager.logger.warn"
-    ) as logger_warn, mock.patch(
+    with patch("cirrus.google_cloud.manager.logger.warn") as logger_warn, patch(
         "cirrus.google_cloud.manager.logger.error"
     ) as logger_error:
         # keep the side effect to actually put logs, so you can see the format with `-s`
@@ -1502,9 +1486,7 @@ def test_unhandled_exception_403_ratelimit_retry(test_cloud_manager):
     test_cloud_manager._authed_session.configure_mock(**mock_config)
     warn = cirrus.google_cloud.manager.logger.warn
     error = cirrus.google_cloud.manager.logger.error
-    with mock.patch(
-        "cirrus.google_cloud.manager.logger.warn"
-    ) as logger_warn, mock.patch(
+    with patch("cirrus.google_cloud.manager.logger.warn") as logger_warn, patch(
         "cirrus.google_cloud.manager.logger.error"
     ) as logger_error:
         # keep the side effect to actually put logs, so you can see the format with `-s`
@@ -1529,9 +1511,7 @@ def test_unhandled_exception_retry(test_cloud_manager):
     test_cloud_manager._admin_service.configure_mock(**mock_config)
     warn = cirrus.google_cloud.manager.logger.warn
     error = cirrus.google_cloud.manager.logger.error
-    with mock.patch(
-        "cirrus.google_cloud.manager.logger.warn"
-    ) as logger_warn, mock.patch(
+    with patch("cirrus.google_cloud.manager.logger.warn") as logger_warn, patch(
         "cirrus.google_cloud.manager.logger.error"
     ) as logger_error:
         # keep the side effect to actually put logs, so you can see the format with `-s`
@@ -1557,9 +1537,7 @@ def test_authorized_session_unhandled_exception_retry(test_cloud_manager):
     test_cloud_manager._authed_session.configure_mock(**mock_config)
     warn = cirrus.google_cloud.manager.logger.warn
     error = cirrus.google_cloud.manager.logger.error
-    with mock.patch(
-        "cirrus.google_cloud.manager.logger.warn"
-    ) as logger_warn, mock.patch(
+    with patch("cirrus.google_cloud.manager.logger.warn") as logger_warn, patch(
         "cirrus.google_cloud.manager.logger.error"
     ) as logger_error:
         # keep the side effect to actually put logs, so you can see the format with `-s`

diff --git a/test/test_utils.py b/test/test_utils.py
@@ -1,12 +1,7 @@
 import pytest
 
-# Python 2 and 3 compatible
-try:
-    from unittest.mock import MagicMock
-    from unittest.mock import patch
-except ImportError:
-    from mock import MagicMock
-    from mock import patch
+from unittest.mock import MagicMock, patch
+from urllib.parse import quote
 
 from cirrus.google_cloud.utils import (
     _get_string_to_sign,
@@ -89,6 +84,34 @@ def test_get_string_to_sign():
     )
 
 
+def test_get_string_to_sign_escaped():
+    http_verb = "GET"
+    md5_hash = "rmYdCNHKFXam78uCt7xQLw=="
+    content_type = "text/plain"
+    expires = "1388534400"
+    ext_headers = ["x-goog-encryption-algorithm:AES256", "x-goog-meta-foo:bar,baz"]
+    # get_signed_url() quotes the path before calling _get_string_to_sign()
+    resource_path = quote("/bucket/P0001_T1/[test] ;.tar.gz")
+
+    result = _get_string_to_sign(
+        path_to_resource=resource_path,
+        http_verb=http_verb,
+        expires=expires,
+        extension_headers=ext_headers,
+        content_type=content_type,
+        md5_value=md5_hash,
+    )
+
+    assert result == (
+        "GET\n"
+        "rmYdCNHKFXam78uCt7xQLw==\n"
+        "text/plain\n"
+        "1388534400\n"
+        "x-goog-encryption-algorithm:AES256\n"
+        "x-goog-meta-foo:bar,baz\n" + quote("/bucket/P0001_T1/[test] ;.tar.gz")
+    )
+
+
 def test_get_string_to_sign_no_optional_params():
     http_verb = "GET"
     expires = "1388534400"