From 77676869a6db0285aa6bcf79fb9b02212f02b32a Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Fri, 7 Feb 2020 17:30:07 +0300 Subject: [PATCH 1/8] feat(storage): improve v4 signature query parameters encoding --- google/cloud/storage/_signing.py | 37 +++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/google/cloud/storage/_signing.py b/google/cloud/storage/_signing.py index e7c8e3328..29556bebe 100644 --- a/google/cloud/storage/_signing.py +++ b/google/cloud/storage/_signing.py @@ -510,7 +510,7 @@ def generate_signed_url_v4( :type query_parameters: dict :param query_parameters: - (Optional) Additional query paramtersto be included as part of the + (Optional) Additional query parameters to be included as part of the signed URLs. See: https://cloud.google.com/storage/docs/xml-api/reference-headers#query @@ -586,8 +586,7 @@ def generate_signed_url_v4( if generation is not None: query_parameters["generation"] = generation - ordered_query_parameters = sorted(query_parameters.items()) - canonical_query_string = six.moves.urllib.parse.urlencode(ordered_query_parameters) + canonical_query_string = _url_encode(query_parameters) canonical_elements = [ method, @@ -666,3 +665,35 @@ def _sign_message(message, access_token, service_account_email): data = json.loads(response.data.decode("utf-8")) return data["signature"] + + +def _url_encode(query_params): + """Encode query params into URL. + + :type query_params: dict + :param query_params: Query params to be encoded. + + :rtype: str + :returns: URL encoded query params. + """ + query_params = sorted(query_params.items()) + + params = [] + for name, value in query_params: + params.append(_quote_param(name) + "=" + _quote_param(value)) + + return "&".join(params) + + +def _quote_param(param): + """Quote query param. + + :type param: dict + :param param: Query param to be encoded. + + :rtype: str + :returns: URL encoded query param. + """ + if not isinstance(param, bytes): + param = str(param) + return six.moves.urllib.parse.quote(param, safe="~") From 8cf8de4b2036f9d4d590877e5b56a0f50c02a61e Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Mon, 10 Feb 2020 12:35:36 +0300 Subject: [PATCH 2/8] add URL encoding test --- google/cloud/storage/_signing.py | 6 ++---- tests/unit/test__signing.py | 13 +++++++++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/google/cloud/storage/_signing.py b/google/cloud/storage/_signing.py index 29556bebe..77f5aa9e4 100644 --- a/google/cloud/storage/_signing.py +++ b/google/cloud/storage/_signing.py @@ -676,13 +676,11 @@ def _url_encode(query_params): :rtype: str :returns: URL encoded query params. """ - query_params = sorted(query_params.items()) - params = [] - for name, value in query_params: + for name, value in query_params.items(): params.append(_quote_param(name) + "=" + _quote_param(value)) - return "&".join(params) + return "&".join(sorted(params)) def _quote_param(param): diff --git a/tests/unit/test__signing.py b/tests/unit/test__signing.py index ebd7f9c17..20a5de055 100644 --- a/tests/unit/test__signing.py +++ b/tests/unit/test__signing.py @@ -702,6 +702,19 @@ def test_sign_bytes_failure(self): ) +class TestCustomURLEncoding(unittest.TestCase): + def test_url_encode(self): + from google.cloud.storage._signing import _url_encode + + # param1 includes safe symbol ~ + # param# includes symbols, which must be encoded + query_params = {"param1": "value~1-2", "param#": "*value+value/"} + + self.assertEqual( + _url_encode(query_params), "param%23=%2Avalue%2Bvalue%2F¶m1=value~1-2" + ) + + _DUMMY_SERVICE_ACCOUNT = None From cbd04127326a2a6231a761f399cc85afc1070eda Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 12 Feb 2020 17:25:37 +0300 Subject: [PATCH 3/8] add query_parameters arg into conformance tests --- tests/unit/test__signing.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/test__signing.py b/tests/unit/test__signing.py index 20a5de055..266b76e50 100644 --- a/tests/unit/test__signing.py +++ b/tests/unit/test__signing.py @@ -741,6 +741,7 @@ def _run_conformance_test(resource, test_data): method=test_data["method"], _request_timestamp=test_data["timestamp"], headers=test_data.get("headers"), + query_parameters=test_data.get("queryParameters"), ) assert url == test_data["expectedUrl"] From 9cca610aeb66b1eacbee9a2830f922726e65d7e0 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Thu, 13 Feb 2020 15:08:35 +0300 Subject: [PATCH 4/8] add tests for _quote_param() function --- google/cloud/storage/_signing.py | 7 ++++--- tests/unit/test__signing.py | 35 ++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/google/cloud/storage/_signing.py b/google/cloud/storage/_signing.py index 94be7acee..72db2f9a4 100644 --- a/google/cloud/storage/_signing.py +++ b/google/cloud/storage/_signing.py @@ -682,9 +682,10 @@ def _url_encode(query_params): :rtype: str :returns: URL encoded query params. """ - params = [] - for name, value in query_params.items(): - params.append(_quote_param(name) + "=" + _quote_param(value)) + params = [ + "{}={}".format(_quote_param(name), _quote_param(value)) + for name, value in query_params.items() + ] return "&".join(sorted(params)) diff --git a/tests/unit/test__signing.py b/tests/unit/test__signing.py index c1bd91b82..67d91514e 100644 --- a/tests/unit/test__signing.py +++ b/tests/unit/test__signing.py @@ -718,6 +718,41 @@ def test_url_encode(self): ) +class TestQuoteParam(unittest.TestCase): + def test_ascii_symbols(self): + from google.cloud.storage._signing import _quote_param + + encoded_param = _quote_param("param") + self.assertIsInstance(encoded_param, str) + self.assertEqual(encoded_param, "param") + + def test_quoted_symbols(self): + from google.cloud.storage._signing import _quote_param + + encoded_param = _quote_param("!#$%&'()*+,/:;=?@[]") + self.assertIsInstance(encoded_param, str) + self.assertEqual( + encoded_param, "%21%23%24%25%26%27%28%29%2A%2B%2C%2F%3A%3B%3D%3F%40%5B%5D" + ) + + def test_unquoted_symbols(self): + from google.cloud.storage._signing import _quote_param + import string + + UNQUOTED = string.ascii_letters + string.digits + ".~_-" + + encoded_param = _quote_param(UNQUOTED) + self.assertIsInstance(encoded_param, str) + self.assertEqual(encoded_param, UNQUOTED) + + def test_unicode_symbols(self): + from google.cloud.storage._signing import _quote_param + + encoded_param = _quote_param("ЁЙЦЯЩЯЩ") + self.assertIsInstance(encoded_param, str) + self.assertEqual(encoded_param, "%D0%81%D0%99%D0%A6%D0%AF%D0%A9%D0%AF%D0%A9") + + _DUMMY_SERVICE_ACCOUNT = None From ae8ba7bb4959ed79d954d93a20058101ba039344 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Thu, 13 Feb 2020 15:25:16 +0300 Subject: [PATCH 5/8] declare test file encoding --- tests/unit/test__signing.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/test__signing.py b/tests/unit/test__signing.py index 67d91514e..fbd8f4e66 100644 --- a/tests/unit/test__signing.py +++ b/tests/unit/test__signing.py @@ -1,3 +1,5 @@ +# -*- coding: utf-8 -*- +# # Copyright 2017 Google LLC # # Licensed under the Apache License, Version 2.0 (the "License"); From bfeb0507f1c71f85645115f07436358e24eac2c1 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Fri, 14 Feb 2020 11:58:58 +0300 Subject: [PATCH 6/8] fix the param type --- google/cloud/storage/_signing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/storage/_signing.py b/google/cloud/storage/_signing.py index 72db2f9a4..39a4d830a 100644 --- a/google/cloud/storage/_signing.py +++ b/google/cloud/storage/_signing.py @@ -693,7 +693,7 @@ def _url_encode(query_params): def _quote_param(param): """Quote query param. - :type param: dict + :type param: Union[str, bytes] :param param: Query param to be encoded. :rtype: str From 9fcf70f65b7f8a66e88afd53700acec0d3deb39f Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Fri, 14 Feb 2020 14:21:34 +0300 Subject: [PATCH 7/8] add test with bytes --- tests/unit/test__signing.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/unit/test__signing.py b/tests/unit/test__signing.py index fbd8f4e66..47ed2bdf0 100644 --- a/tests/unit/test__signing.py +++ b/tests/unit/test__signing.py @@ -754,6 +754,13 @@ def test_unicode_symbols(self): self.assertIsInstance(encoded_param, str) self.assertEqual(encoded_param, "%D0%81%D0%99%D0%A6%D0%AF%D0%A9%D0%AF%D0%A9") + def test_bytes(self): + from google.cloud.storage._signing import _quote_param + + encoded_param = _quote_param(b"bytes") + self.assertIsInstance(encoded_param, str) + self.assertEqual(encoded_param, "bytes") + _DUMMY_SERVICE_ACCOUNT = None From 8e94d8f9b5c3bf88b3e7182235fbb48737ad1592 Mon Sep 17 00:00:00 2001 From: Gurov Ilya Date: Wed, 26 Feb 2020 21:13:50 +0300 Subject: [PATCH 8/8] Update _signing.py --- google/cloud/storage/_signing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/storage/_signing.py b/google/cloud/storage/_signing.py index 39a4d830a..8151786e9 100644 --- a/google/cloud/storage/_signing.py +++ b/google/cloud/storage/_signing.py @@ -693,7 +693,7 @@ def _url_encode(query_params): def _quote_param(param): """Quote query param. - :type param: Union[str, bytes] + :type param: Any :param param: Query param to be encoded. :rtype: str