diff --git a/cirrus/google_cloud/utils.py b/cirrus/google_cloud/utils.py index 8f701346..36629745 100644 --- 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 index 8c158918..bae6e4c0 100644 --- 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 index fdf11256..697a5fa6 100644 --- 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 index 70fcc7fc..e492d9fa 100644 --- 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"