Raise ValueError exception if bucket name is invalid. (#3160)

Author: daspecster

storage/google/cloud/storage/_helpers.py

@@ -21,6 +21,25 @@
 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.
 
@@ -29,11 +48,12 @@ 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):
-        self.name = name
+        self.name = _validate_name(name)
         self._properties = {}
         self._changes = set()
 

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

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)