fix: relax bucket_name validation for existing buckets (#840)

minio/api.py

@@ -272,7 +272,7 @@ def select_object_content(self, bucket_name, object_name, opts):
         :param object_name: Name of object to read
         :param options: Options for select object
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
         is_non_empty_string(object_name)
 
         content = xml_marshal_select(opts)
@@ -315,7 +315,7 @@ def make_bucket(self, bucket_name, location='us-east-1'):
         :param bucket_name: Bucket to create on server
         :param location: Location to create bucket on
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, True)
 
         # Default region for all requests.
         region = 'us-east-1'
@@ -417,7 +417,7 @@ def bucket_exists(self, bucket_name):
         :param bucket_name: To test the existence and user access.
         :return: True on success.
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
 
         try:
             self._url_open('HEAD', bucket_name=bucket_name)
@@ -434,7 +434,7 @@ def remove_bucket(self, bucket_name):
 
         :param bucket_name: Bucket to remove
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
         self._url_open('DELETE', bucket_name=bucket_name)
 
         # Make sure to purge bucket_name from region cache.
@@ -446,7 +446,7 @@ def get_bucket_policy(self, bucket_name):
 
         :param bucket_name: Bucket name.
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
 
         response = self._url_open("GET",
                                   bucket_name=bucket_name,
@@ -467,7 +467,7 @@ def set_bucket_policy(self, bucket_name, policy):
         """
         is_valid_policy_type(policy)
 
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
 
         headers = {
             'Content-Length': str(len(policy)),
@@ -487,7 +487,7 @@ def get_bucket_notification(self, bucket_name):
 
         :param bucket_name: Bucket name.
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
 
         response = self._url_open(
             "GET",
@@ -504,7 +504,7 @@ def set_bucket_notification(self, bucket_name, notifications):
         :param bucket_name: Bucket name.
         :param notifications: Notifications structure
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
         is_valid_bucket_notification_config(notifications)
 
         content = xml_marshal_bucket_notifications(notifications)
@@ -532,7 +532,7 @@ def remove_all_bucket_notification(self, bucket_name):
 
         :param bucket_name: Bucket name.
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
 
         content_bytes = xml_marshal_bucket_notifications({})
         headers = {
@@ -566,7 +566,7 @@ def listen_bucket_notification(self, bucket_name, prefix='', suffix='',
         :param events: Enables notifications for specific event types.
              of events.
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
 
         # If someone explicitly set prefix to None convert it to empty string.
         if prefix is None:
@@ -643,7 +643,7 @@ def fget_object(self, bucket_name, object_name, file_path, request_headers=None,
         :param file_path: Local file path to save the object.
         :param request_headers: Any additional headers to be added with GET request.
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
         is_non_empty_string(object_name)
 
         stat = self.stat_object(bucket_name, object_name, sse)
@@ -724,7 +724,7 @@ def get_object(self, bucket_name, object_name, request_headers=None, sse=None):
         :return: :class:`urllib3.response.HTTPResponse` object.
 
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
         is_non_empty_string(object_name)
 
         return self._get_partial_object(bucket_name,
@@ -757,7 +757,7 @@ def get_partial_object(self, bucket_name, object_name, offset=0, length=0, reque
         :return: :class:`urllib3.response.HTTPResponse` object.
 
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
         is_non_empty_string(object_name)
 
         return self._get_partial_object(bucket_name,
@@ -783,7 +783,7 @@ def copy_object(self, bucket_name, object_name, object_source,
         :param metadata: Any user-defined metadata to be copied along with
         destination object.
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
         is_non_empty_string(object_name)
         is_non_empty_string(object_source)
 
@@ -848,7 +848,7 @@ def put_object(self, bucket_name, object_name, data, length,
         """
 
         is_valid_sse_object(sse)
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
         is_non_empty_string(object_name)
 
         if progress:
@@ -935,7 +935,7 @@ def list_objects(self, bucket_name, prefix='', recursive=False):
         :param recursive: If yes, returns all objects for a specified prefix
         :return: An iterator of objects in alphabetical order.
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
 
         # If someone explicitly set prefix to None convert it to empty string.
         if prefix is None:
@@ -1011,7 +1011,7 @@ def list_objects_v2(self, bucket_name, prefix='', recursive=False, start_after='
         :param recursive: If yes, returns all objects for a specified prefix
         :return: An iterator of objects in alphabetical order.
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
 
         # If someone explicitly set prefix to None convert it to empty string.
         if prefix is None:
@@ -1060,7 +1060,7 @@ def stat_object(self, bucket_name, object_name, sse=None):
             is_valid_sse_c_object(sse=sse)
             headers.update(sse.marshal())
 
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
         is_non_empty_string(object_name)
 
         response = self._url_open('HEAD', bucket_name=bucket_name,
@@ -1090,7 +1090,7 @@ def remove_object(self, bucket_name, object_name):
         :param object_name: Name of object to remove
         :return: None
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
         is_non_empty_string(object_name)
 
         # No reason to store successful response, for errors
@@ -1136,7 +1136,7 @@ def remove_objects(self, bucket_name, objects_iter):
         object that had a delete error.
 
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
         if isinstance(objects_iter, basestring):
             raise TypeError(
                 'objects_iter cannot be `str` or `bytes` instance. It must be '
@@ -1215,7 +1215,7 @@ def list_incomplete_uploads(self, bucket_name, prefix='',
            a specified prefix.
         :return: An generator of incomplete uploads in alphabetical order.
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
 
         return self._list_incomplete_uploads(bucket_name, prefix, recursive)
 
@@ -1229,7 +1229,7 @@ def _list_incomplete_uploads(self, bucket_name, prefix='',
         :param recursive: If yes, returns all incomplete objects for a specified prefix.
         :return: An generator of incomplete uploads in alphabetical order.
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
 
         # If someone explicitly set prefix to None convert it to empty string.
         if prefix is None:
@@ -1288,7 +1288,7 @@ def _list_object_parts(self, bucket_name, object_name, upload_id):
         :param object_name: Object name to list parts for.
         :param upload_id: Upload id of the previously uploaded object name.
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
         is_non_empty_string(object_name)
         is_non_empty_string(upload_id)
 
@@ -1324,7 +1324,7 @@ def remove_incomplete_upload(self, bucket_name, object_name):
         :param object_name: Name of object to remove incomplete uploads
         :return: None
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
         is_non_empty_string(object_name)
 
         recursive = True
@@ -1366,7 +1366,7 @@ def presigned_url(self, method,
                               current date.
         :return: Presigned put object url.
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
         is_non_empty_string(object_name)
 
         if expires.total_seconds() < 1 or \
@@ -1531,7 +1531,7 @@ def _get_partial_object(self, bucket_name, object_name,
 
         """
         is_valid_sse_c_object(sse=sse)
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
         is_non_empty_string(object_name)
 
         headers = {}
@@ -1569,7 +1569,7 @@ def _do_put_object(self, bucket_name, object_name, part_data,
            with your object.
         :param progress: A progress object
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
         is_non_empty_string(object_name)
 
         # Accept only bytes - otherwise we need to know how to encode
@@ -1650,7 +1650,7 @@ def _stream_put_object(self, bucket_name, object_name,
         :param progress: A progress object
         :param part_size: Multipart part size
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
         is_non_empty_string(object_name)
         if not callable(getattr(data, 'read')):
             raise ValueError(
@@ -1760,7 +1760,7 @@ def _new_multipart_upload(self, bucket_name, object_name,
         :param metadata: Additional new metadata for the new object.
         :return: Returns an upload id.
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
         is_non_empty_string(object_name)
 
         headers = {}
@@ -1786,7 +1786,7 @@ def _complete_multipart_upload(self, bucket_name, object_name,
         :param upload_id: Upload id of the active multipart request.
         :param uploaded_parts: Key, Value dictionary of uploaded parts.
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
         is_non_empty_string(object_name)
         is_non_empty_string(upload_id)
 

minio/helpers.py

@@ -55,9 +55,15 @@
 MIN_PART_SIZE = 5 * 1024 * 1024  # 5MiB
 DEFAULT_PART_SIZE = MIN_PART_SIZE  # Currently its 5MiB
 
-_VALID_BUCKETNAME_REGEX = re.compile('^[a-z0-9][a-z0-9\\.\\-]+[a-z0-9]$')
+_VALID_BUCKETNAME_REGEX = re.compile(
+    '^[A-Za-z0-9][A-Za-z0-9\\.\\-\\_\\:]{1,61}[A-Za-z0-9]$')
+_VALID_BUCKETNAME_STRICT_REGEX = re.compile(
+    '^[a-z0-9][a-z0-9\\.\\-]{1,61}[a-z0-9]$')
+_VALID_IP_ADDRESS = re.compile(
+    '^(\d+\.){3}\d+$')
 _ALLOWED_HOSTNAME_REGEX = re.compile(
-    '^((?!-)(?!_)[A-Z_\\d-]{1,63}(?<!-)(?<!_)\\.)*((?!_)(?!-)[A-Z_\\d-]{1,63}(?<!-)(?<!_))$',
+    '^((?!-)(?!_)[A-Z_\\d-]{1,63}(?<!-)(?<!_)\\.)*((?!_)(?!-)' +
+    '[A-Z_\\d-]{1,63}(?<!-)(?<!_))$',
     re.IGNORECASE)
 
 _EXTRACT_REGION_REGEX = re.compile('s3[.-]?(.+?).amazonaws.com')
@@ -328,7 +334,7 @@ def is_virtual_host(endpoint_url, bucket_name):
     :param endpoint_url: Endpoint url which will be used for virtual host.
     :param bucket_name: Bucket name to be validated against.
     """
-    is_valid_bucket_name(bucket_name)
+    is_valid_bucket_name(bucket_name, False)
 
     parsed_url = urlsplit(endpoint_url)
     # bucket_name can be valid but '.' in the hostname will fail
@@ -341,7 +347,7 @@ def is_virtual_host(endpoint_url, bucket_name):
             return True
     return False
 
-def is_valid_bucket_name(bucket_name):
+def is_valid_bucket_name(bucket_name, strict):
     """
     Check to see if the ``bucket_name`` complies with the
     restricted DNS naming conventions necessary to allow
@@ -351,16 +357,33 @@ def is_valid_bucket_name(bucket_name):
     :return: True if the bucket is valid. Raise :exc:`InvalidBucketError`
        otherwise.
     """
+    # Verify bucket name is not empty
+    bucket_name = str(bucket_name).strip()
+    if bucket_name == '':
+        raise InvalidBucketError('Bucket name cannot be empty.')
+
     # Verify bucket name length.
     if len(bucket_name) < 3:
         raise InvalidBucketError('Bucket name cannot be less than'
                                  ' 3 characters.')
     if len(bucket_name) > 63:
-        raise InvalidBucketError('Bucket name cannot be more than'
+        raise InvalidBucketError('Bucket name cannot be greater than'
                                  ' 63 characters.')
-    if '..' in bucket_name:
-        raise InvalidBucketError('Bucket name cannot have successive'
-                                 ' periods.')
+
+    match = _VALID_IP_ADDRESS.match(bucket_name)
+    if match:
+        raise InvalidBucketError('Bucket name cannot be an ip address')
+
+    unallowed_successive_chars = ['..', '.-', '-.']
+    if any(x in bucket_name for x in unallowed_successive_chars):
+        raise InvalidBucketError('Bucket name contains invalid '
+                'successive chars ' + str(unallowed_successive_chars) + '.')
+
+    if strict:
+        match = _VALID_BUCKETNAME_STRICT_REGEX.match(bucket_name)
+        if match is None or match.end() != len(bucket_name):
+            raise InvalidBucketError('Bucket name contains invalid '
+                                     'characters (strictly enforced).')
 
     match = _VALID_BUCKETNAME_REGEX.match(bucket_name)
     if match is None or match.end() != len(bucket_name):

minio/post_policy.py

@@ -88,7 +88,7 @@ def set_bucket_name(self, bucket_name):
 
         :param bucket_name: set bucket name.
         """
-        is_valid_bucket_name(bucket_name)
+        is_valid_bucket_name(bucket_name, False)
 
         self.policies.append(('eq', '$bucket', bucket_name))
         self.form_data['bucket'] = bucket_name

tests/unit/bucket_exist_test.py

@@ -38,19 +38,19 @@ def test_bucket_is_not_empty_string(self):
     @raises(InvalidBucketError)
     def test_bucket_exists_invalid_name(self):
         client = Minio('localhost:9000')
-        client.bucket_exists('ABCD')
+        client.bucket_exists('AB*CD')
 
     @mock.patch('urllib3.PoolManager')
     @raises(ResponseError)
-    def test_bucket_exists_works(self, mock_connection):
+    def test_bucket_exists_bad_request(self, mock_connection):
         mock_server = MockConnection()
         mock_connection.return_value = mock_server
         mock_server.mock_add_request(MockResponse('HEAD',
                                                   'https://localhost:9000/hello/',
                                                   {'User-Agent': _DEFAULT_USER_AGENT},
                                                   400))
         client = Minio('localhost:9000')
-        result = client.bucket_exists('hello')
+        client.bucket_exists('hello')
 
     @mock.patch('urllib3.PoolManager')
     def test_bucket_exists_works(self, mock_connection):