From 4e73251d18f84bea35fa2b16dad5b84252fda0fa Mon Sep 17 00:00:00 2001 From: daspecster Date: Thu, 16 Mar 2017 16:03:15 -0400 Subject: [PATCH 1/3] Raise exception if bucket name is invalid. --- storage/google/cloud/storage/_helpers.py | 8 +++++++- storage/unit_tests/test__helpers.py | 13 +++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/storage/google/cloud/storage/_helpers.py b/storage/google/cloud/storage/_helpers.py index 955eba01d7aa..f64e382b139a 100644 --- a/storage/google/cloud/storage/_helpers.py +++ b/storage/google/cloud/storage/_helpers.py @@ -18,6 +18,7 @@ """ import base64 +import re from hashlib import md5 @@ -33,7 +34,12 @@ class _PropertyMixin(object): """ def __init__(self, name=None): - self.name = name + if name is None or (re.match(r'\w', name[0]) and + re.match(r'\w', name[-1])): + self.name = name + else: + raise ValueError( + 'Bucket names must start and end with a number or letter.') self._properties = {} self._changes = set() diff --git a/storage/unit_tests/test__helpers.py b/storage/unit_tests/test__helpers.py index 89967f3a0db0..9e6835e57eb9 100644 --- a/storage/unit_tests/test__helpers.py +++ b/storage/unit_tests/test__helpers.py @@ -46,6 +46,19 @@ def test_client_is_abstract(self): mixin = self._make_one() self.assertRaises(NotImplementedError, lambda: mixin.client) + def test_bucket_name_value(self): + bucket_name = 'testing123' + mixin = self._make_one(name=bucket_name) + self.assertEqual(mixin.name, bucket_name) + + bad_start_bucket_name = '/testing123' + with self.assertRaises(ValueError): + self._make_one(name=bad_start_bucket_name) + + bad_end_bucket_name = 'testing123/' + with self.assertRaises(ValueError): + self._make_one(name=bad_end_bucket_name) + def test_reload(self): connection = _Connection({'foo': 'Foo'}) client = _Client(connection) From 8c47c9302b395b16d2a9adf9a9ad3c893f5b82b7 Mon Sep 17 00:00:00 2001 From: daspecster Date: Fri, 17 Mar 2017 11:06:01 -0400 Subject: [PATCH 2/3] Update Bucket name docstring to define valid values. --- storage/google/cloud/storage/_helpers.py | 3 ++- storage/google/cloud/storage/bucket.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/storage/google/cloud/storage/_helpers.py b/storage/google/cloud/storage/_helpers.py index f64e382b139a..9efe7292fe5d 100644 --- a/storage/google/cloud/storage/_helpers.py +++ b/storage/google/cloud/storage/_helpers.py @@ -30,7 +30,8 @@ class _PropertyMixin(object): - path :type name: str - :param name: The name of the object. + :param name: The name of the object. Bucket names must start and end with a + number or letter. """ def __init__(self, name=None): diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index f54791785d6b..daf3d9fda8a2 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -81,7 +81,8 @@ class Bucket(_PropertyMixin): for the bucket (which requires a project). :type name: str - :param name: The name of the bucket. + :param name: The name of the bucket. Bucket names must start and end with a + number or letter. """ _MAX_OBJECTS_FOR_ITERATION = 256 From 79f107d6d7e1267f790db778b54916298d2dd178 Mon Sep 17 00:00:00 2001 From: daspecster Date: Fri, 17 Mar 2017 15:32:18 -0400 Subject: [PATCH 3/3] Add _validate_name helper method. --- storage/google/cloud/storage/_helpers.py | 27 ++++++++++++++++++------ 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/storage/google/cloud/storage/_helpers.py b/storage/google/cloud/storage/_helpers.py index 9efe7292fe5d..d1534e3a005e 100644 --- a/storage/google/cloud/storage/_helpers.py +++ b/storage/google/cloud/storage/_helpers.py @@ -18,10 +18,28 @@ """ import base64 -import re from hashlib import md5 +def _validate_name(name): + """Pre-flight ``Bucket`` name validation. + + :type name: str or :data:`NoneType` + :param name: Proposed bucket name. + + :rtype: str or :data:`NoneType` + :returns: ``name`` if valid. + """ + if name is None: + return + + # The first and las characters must be alphanumeric. + if not all([name[0].isalnum(), name[-1].isalnum()]): + raise ValueError( + 'Bucket names must start and end with a number or letter.') + return name + + class _PropertyMixin(object): """Abstract mixin for cloud storage classes with associated propertties. @@ -35,12 +53,7 @@ class _PropertyMixin(object): """ def __init__(self, name=None): - if name is None or (re.match(r'\w', name[0]) and - re.match(r'\w', name[-1])): - self.name = name - else: - raise ValueError( - 'Bucket names must start and end with a number or letter.') + self.name = _validate_name(name) self._properties = {} self._changes = set()