From b982094df67206018df66c30b067e30e6f363419 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Thu, 7 May 2020 12:11:22 +0300 Subject: [PATCH 1/7] feat: add helper for bucket bound hostname URLs --- google/cloud/storage/_helpers.py | 22 ++++++++++++++++++++++ google/cloud/storage/blob.py | 10 ++++------ google/cloud/storage/bucket.py | 10 ++++------ google/cloud/storage/client.py | 12 ++++-------- tests/unit/test__helpers.py | 26 ++++++++++++++++++++++++++ 5 files changed, 60 insertions(+), 20 deletions(-) diff --git a/google/cloud/storage/_helpers.py b/google/cloud/storage/_helpers.py index b649384f7..b2e70c637 100644 --- a/google/cloud/storage/_helpers.py +++ b/google/cloud/storage/_helpers.py @@ -312,3 +312,25 @@ def _convert_to_timestamp(value): utc_naive = value.replace(tzinfo=None) - value.utcoffset() mtime = (utc_naive - datetime(1970, 1, 1)).total_seconds() return mtime + + +def _bucket_bound_hostname_url(host, scheme=None, end_slash=False): + """Helper to build bucket bound hostname URL. + + :type host: str + :param host: Host name. + + :type scheme: str + :param scheme: (Optional) Web scheme. If passed, use it + as a scheme in the result URL. + + :type end_slash: bool + :param end_slash: (Optional) If True, add "/" slash to + the end of the result URL. + """ + if ":" in host: + return host + + return "{scheme}://{host}{slash}".format( + scheme=scheme, host=host, slash="/" if end_slash else "" + ) \ No newline at end of file diff --git a/google/cloud/storage/blob.py b/google/cloud/storage/blob.py index d3416616b..18f56d271 100644 --- a/google/cloud/storage/blob.py +++ b/google/cloud/storage/blob.py @@ -56,6 +56,7 @@ from google.cloud.exceptions import NotFound from google.cloud.storage._helpers import _PropertyMixin from google.cloud.storage._helpers import _scalar_property +from google.cloud.storage._helpers import _bucket_bound_hostname_url from google.cloud.storage._helpers import _convert_to_timestamp from google.cloud.storage._signing import generate_signed_url_v2 from google.cloud.storage._signing import generate_signed_url_v4 @@ -514,12 +515,9 @@ def generate_signed_url( bucket_name=self.bucket.name ) elif bucket_bound_hostname: - if ":" in bucket_bound_hostname: - api_access_endpoint = bucket_bound_hostname - else: - api_access_endpoint = "{scheme}://{bucket_bound_hostname}".format( - scheme=scheme, bucket_bound_hostname=bucket_bound_hostname - ) + api_access_endpoint = _bucket_bound_hostname_url( + bucket_bound_hostname, scheme + ) else: resource = "/{bucket_name}/{quoted_name}".format( bucket_name=self.bucket.name, quoted_name=quoted_name diff --git a/google/cloud/storage/bucket.py b/google/cloud/storage/bucket.py index 8540bef6e..edd2f121d 100644 --- a/google/cloud/storage/bucket.py +++ b/google/cloud/storage/bucket.py @@ -37,6 +37,7 @@ from google.cloud.storage._helpers import _validate_name from google.cloud.storage._signing import generate_signed_url_v2 from google.cloud.storage._signing import generate_signed_url_v4 +from google.cloud.storage._helpers import _bucket_bound_hostname_url from google.cloud.storage.acl import BucketACL from google.cloud.storage.acl import DefaultObjectACL from google.cloud.storage.blob import Blob @@ -2560,12 +2561,9 @@ def generate_signed_url( bucket_name=self.name ) elif bucket_bound_hostname: - if ":" in bucket_bound_hostname: - api_access_endpoint = bucket_bound_hostname - else: - api_access_endpoint = "{scheme}://{bucket_bound_hostname}".format( - scheme=scheme, bucket_bound_hostname=bucket_bound_hostname - ) + api_access_endpoint = _bucket_bound_hostname_url( + bucket_bound_hostname, scheme + ) else: resource = "/{bucket_name}".format(bucket_name=self.name) diff --git a/google/cloud/storage/client.py b/google/cloud/storage/client.py index 4a4bbe733..5600edc0e 100644 --- a/google/cloud/storage/client.py +++ b/google/cloud/storage/client.py @@ -30,6 +30,7 @@ from google.cloud.client import ClientWithProject from google.cloud.exceptions import NotFound from google.cloud.storage._helpers import _get_storage_host +from google.cloud.storage._helpers import _bucket_bound_hostname_url from google.cloud.storage._http import Connection from google.cloud.storage._signing import ( get_expiration_seconds_v4, @@ -1033,15 +1034,10 @@ def generate_signed_post_policy_v4( # designate URL if virtual_hosted_style: url = "https://{}.storage.googleapis.com/".format(bucket_name) - elif bucket_bound_hostname: - if ":" in bucket_bound_hostname: # URL includes scheme - url = bucket_bound_hostname - - else: # scheme is given separately - url = "{scheme}://{host}/".format( - scheme=scheme, host=bucket_bound_hostname - ) + url = _bucket_bound_hostname_url( + bucket_bound_hostname, scheme, end_slash=True + ) else: url = "https://storage.googleapis.com/{}/".format(bucket_name) diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index 10b71b7bc..db8d723a0 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -342,6 +342,32 @@ def read(self, block_size): self.assertEqual(MD5.hash_obj.num_digest_calls, 1) self.assertEqual(MD5.hash_obj._blocks, [BYTES_TO_SIGN]) +class Test__bucket_bound_hostname_url(unittest.TestCase): + def _call_fut(self, **args): + from google.cloud.storage._helpers import _bucket_bound_hostname_url + + return _bucket_bound_hostname_url(**args) + + def test_full_hostname(self): + HOST = "scheme://domain.tcl" + self.assertEqual(self._call_fut(host=HOST), HOST) + + def test_hostname_and_scheme(self): + HOST = "domain.tcl" + SCHEME = "scheme" + EXPECTED_URL = SCHEME + "://" + HOST + + self.assertEqual(self._call_fut(host=HOST, scheme=SCHEME), EXPECTED_URL) + + def test_with_end_slash(self): + HOST = "domain.tcl" + SCHEME = "scheme" + EXPECTED_URL = "{scheme}://{host}/".format(scheme=SCHEME, host=HOST) + + self.assertEqual( + self._call_fut(host=HOST, scheme=SCHEME, end_slash=True), EXPECTED_URL + ) + class _Connection(object): def __init__(self, *responses): From 596660f8c9198a5bf044e7e781a5304d3420ff5f Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Thu, 7 May 2020 12:23:36 +0300 Subject: [PATCH 2/7] add newline to the end of file --- google/cloud/storage/_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/storage/_helpers.py b/google/cloud/storage/_helpers.py index b2e70c637..1d7379d4e 100644 --- a/google/cloud/storage/_helpers.py +++ b/google/cloud/storage/_helpers.py @@ -333,4 +333,4 @@ def _bucket_bound_hostname_url(host, scheme=None, end_slash=False): return "{scheme}://{host}{slash}".format( scheme=scheme, host=host, slash="/" if end_slash else "" - ) \ No newline at end of file + ) From a468720c8e3cad93b588a96138ce1ea26b10e6ed Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Thu, 7 May 2020 14:39:00 +0300 Subject: [PATCH 3/7] lint fix --- tests/unit/test__helpers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index db8d723a0..0ce489aed 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -342,6 +342,7 @@ def read(self, block_size): self.assertEqual(MD5.hash_obj.num_digest_calls, 1) self.assertEqual(MD5.hash_obj._blocks, [BYTES_TO_SIGN]) + class Test__bucket_bound_hostname_url(unittest.TestCase): def _call_fut(self, **args): from google.cloud.storage._helpers import _bucket_bound_hostname_url From 5178aa2a9ff3822f788d4823ea12dfc2140aaa91 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Thu, 7 May 2020 14:59:02 +0300 Subject: [PATCH 4/7] add return type docs --- google/cloud/storage/_helpers.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/google/cloud/storage/_helpers.py b/google/cloud/storage/_helpers.py index 1d7379d4e..bffd709e3 100644 --- a/google/cloud/storage/_helpers.py +++ b/google/cloud/storage/_helpers.py @@ -327,6 +327,9 @@ def _bucket_bound_hostname_url(host, scheme=None, end_slash=False): :type end_slash: bool :param end_slash: (Optional) If True, add "/" slash to the end of the result URL. + + :rtype: str + :returns: A bucket bound hostname URL. """ if ":" in host: return host From 4eb684e546eafa0db49a5ef90da6cf41b6a60066 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Fri, 15 May 2020 11:44:47 +0300 Subject: [PATCH 5/7] del the excess empty line --- google/cloud/storage/_helpers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/google/cloud/storage/_helpers.py b/google/cloud/storage/_helpers.py index a5ff125ee..7758bd7d9 100644 --- a/google/cloud/storage/_helpers.py +++ b/google/cloud/storage/_helpers.py @@ -508,4 +508,3 @@ def _bucket_bound_hostname_url(host, scheme=None, end_slash=False): return "{scheme}://{host}{slash}".format( scheme=scheme, host=host, slash="/" if end_slash else "" ) - From c2fce54bd8556dd5f8bdc6472ccf1c25504bfb13 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Fri, 22 May 2020 10:32:41 +0300 Subject: [PATCH 6/7] remove end slash from the helper --- google/cloud/storage/_helpers.py | 10 ++-------- google/cloud/storage/client.py | 4 +--- tests/unit/test__helpers.py | 13 ++----------- tests/unit/test_blob.py | 10 ++++------ tests/unit/test_bucket.py | 10 ++++------ 5 files changed, 13 insertions(+), 34 deletions(-) diff --git a/google/cloud/storage/_helpers.py b/google/cloud/storage/_helpers.py index e36529861..5dfb70693 100644 --- a/google/cloud/storage/_helpers.py +++ b/google/cloud/storage/_helpers.py @@ -493,7 +493,7 @@ def _raise_if_more_than_one_set(**kwargs): raise ValueError(msg) -def _bucket_bound_hostname_url(host, scheme=None, end_slash=False): +def _bucket_bound_hostname_url(host, scheme=None): """Helper to build bucket bound hostname URL. :type host: str @@ -503,16 +503,10 @@ def _bucket_bound_hostname_url(host, scheme=None, end_slash=False): :param scheme: (Optional) Web scheme. If passed, use it as a scheme in the result URL. - :type end_slash: bool - :param end_slash: (Optional) If True, add "/" slash to - the end of the result URL. - :rtype: str :returns: A bucket bound hostname URL. """ if ":" in host: return host - return "{scheme}://{host}{slash}".format( - scheme=scheme, host=host, slash="/" if end_slash else "" - ) + return "{scheme}://{host}/".format(scheme=scheme, host=host) diff --git a/google/cloud/storage/client.py b/google/cloud/storage/client.py index db04b9c17..4927aecf9 100644 --- a/google/cloud/storage/client.py +++ b/google/cloud/storage/client.py @@ -1081,9 +1081,7 @@ def generate_signed_post_policy_v4( if virtual_hosted_style: url = "https://{}.storage.googleapis.com/".format(bucket_name) elif bucket_bound_hostname: - url = _bucket_bound_hostname_url( - bucket_bound_hostname, scheme, end_slash=True - ) + url = _bucket_bound_hostname_url(bucket_bound_hostname, scheme) else: url = "https://storage.googleapis.com/{}/".format(bucket_name) diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index a14121cff..e295cbefc 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -532,25 +532,16 @@ def _call_fut(self, **args): return _bucket_bound_hostname_url(**args) def test_full_hostname(self): - HOST = "scheme://domain.tcl" + HOST = "scheme://domain.tcl/" self.assertEqual(self._call_fut(host=HOST), HOST) def test_hostname_and_scheme(self): HOST = "domain.tcl" SCHEME = "scheme" - EXPECTED_URL = SCHEME + "://" + HOST + EXPECTED_URL = SCHEME + "://" + HOST + "/" self.assertEqual(self._call_fut(host=HOST, scheme=SCHEME), EXPECTED_URL) - def test_with_end_slash(self): - HOST = "domain.tcl" - SCHEME = "scheme" - EXPECTED_URL = "{scheme}://{host}/".format(scheme=SCHEME, host=HOST) - - self.assertEqual( - self._call_fut(host=HOST, scheme=SCHEME, end_slash=True), EXPECTED_URL - ) - class _Connection(object): def __init__(self, *responses): diff --git a/tests/unit/test_blob.py b/tests/unit/test_blob.py index 94822b93a..d5174a612 100644 --- a/tests/unit/test_blob.py +++ b/tests/unit/test_blob.py @@ -405,6 +405,7 @@ def _generate_signed_url_helper( ): from six.moves.urllib import parse from google.cloud._helpers import UTC + from google.cloud.storage._helpers import _bucket_bound_hostname_url from google.cloud.storage.blob import _API_ACCESS_ENDPOINT from google.cloud.storage.blob import _get_encryption_headers @@ -464,12 +465,9 @@ def _generate_signed_url_helper( bucket.name ) elif bucket_bound_hostname: - if ":" in bucket_bound_hostname: - expected_api_access_endpoint = bucket_bound_hostname - else: - expected_api_access_endpoint = "{scheme}://{bucket_bound_hostname}".format( - scheme=scheme, bucket_bound_hostname=bucket_bound_hostname - ) + expected_api_access_endpoint = _bucket_bound_hostname_url( + bucket_bound_hostname, scheme + ) else: expected_api_access_endpoint = api_access_endpoint expected_resource = "/{}/{}".format(bucket.name, quoted_name) diff --git a/tests/unit/test_bucket.py b/tests/unit/test_bucket.py index 5069d876d..29320367a 100644 --- a/tests/unit/test_bucket.py +++ b/tests/unit/test_bucket.py @@ -2990,6 +2990,7 @@ def _generate_signed_url_helper( ): from six.moves.urllib import parse from google.cloud._helpers import UTC + from google.cloud.storage._helpers import _bucket_bound_hostname_url from google.cloud.storage.blob import _API_ACCESS_ENDPOINT api_access_endpoint = api_access_endpoint or _API_ACCESS_ENDPOINT @@ -3037,12 +3038,9 @@ def _generate_signed_url_helper( bucket_name ) elif bucket_bound_hostname: - if ":" in bucket_bound_hostname: - expected_api_access_endpoint = bucket_bound_hostname - else: - expected_api_access_endpoint = "{scheme}://{bucket_bound_hostname}".format( - scheme=scheme, bucket_bound_hostname=bucket_bound_hostname - ) + expected_api_access_endpoint = _bucket_bound_hostname_url( + bucket_bound_hostname, scheme + ) else: expected_api_access_endpoint = api_access_endpoint expected_resource = "/{}".format(parse.quote(bucket_name)) From f0740b4ce2e7d0931518aae59449bc601b1e0187 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Fri, 22 May 2020 12:31:49 +0300 Subject: [PATCH 7/7] use urllib.parse() in _bucket_bound_hostname_url() --- google/cloud/storage/_helpers.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/google/cloud/storage/_helpers.py b/google/cloud/storage/_helpers.py index 5dfb70693..a1075eac7 100644 --- a/google/cloud/storage/_helpers.py +++ b/google/cloud/storage/_helpers.py @@ -22,6 +22,7 @@ from datetime import datetime import os +from six.moves.urllib.parse import urlsplit from google.cloud.storage.constants import _DEFAULT_TIMEOUT @@ -506,7 +507,8 @@ def _bucket_bound_hostname_url(host, scheme=None): :rtype: str :returns: A bucket bound hostname URL. """ - if ":" in host: + url_parts = urlsplit(host) + if url_parts.scheme and url_parts.netloc: return host return "{scheme}://{host}/".format(scheme=scheme, host=host)