Repository: boto/botocore
Pull request #640 (2015-09-02 01:42:11)

Add bucket name validation to S3 calls
Fixes boto/boto3#181

Based on:

http://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html#non-dns-compliant-bucketname-challenges


cc @kyleknap @mtdowling @rayluo 


Patch:
diff --git a/botocore/handlers.py b/botocore/handlers.py
index eae563c09..b648758a7 100644
--- a/botocore/handlers.py
+++ b/botocore/handlers.py
@@ -21,6 +21,8 @@ import hashlib
 import logging
 import xml.etree.cElementTree
 import copy
+import re
+import string
 
 from botocore.compat import urlsplit, urlunsplit, unquote, json, quote, six
 from botocore.docs.utils import AutoPopulatedParam
@@ -28,6 +30,7 @@ from botocore.docs.utils import HideParamFromOperations
 from botocore.docs.utils import AppendParamDocumentation
 from botocore.signers import add_generate_presigned_url
 from botocore.signers import add_generate_presigned_post
+from botocore.exceptions import ParamValidationError
 
 from botocore import retryhandler
 from botocore import utils
@@ -40,6 +43,12 @@ logger = logging.getLogger(__name__)
 
 REGISTER_FIRST = object()
 REGISTER_LAST = object()
+# From the S3 docs:
+# The rules for bucket names in the US Standard region allow bucket names
+# to be as long as 255 characters, and bucket names can contain any
+# combination of uppercase letters, lowercase letters, numbers, periods
+# (.), hyphens (-), and underscores (_).
+VALID_BUCKET = re.compile('^[a-zA-Z0-9.\-_]{1,255}$')
 
 
 def check_for_200_error(response, **kwargs):
@@ -140,6 +149,17 @@ def conditionally_calculate_md5(params, **kwargs):
         calculate_md5(params, **kwargs)
 
 
+def validate_bucket_name(params, **kwargs):
+    if 'Bucket' not in params:
+        return
+    bucket = params['Bucket']
+    if VALID_BUCKET.search(bucket) is None:
+        error_msg = (
+            'Invalid bucket name "%s": Bucket name must match '
+            'the regex "%s"' % (bucket, VALID_BUCKET.pattern))
+        raise ParamValidationError(report=error_msg)
+
+
 def sse_md5(params, **kwargs):
     """
     S3 server-side encryption requires the encryption key to be sent to the
@@ -444,6 +464,8 @@ BUILTIN_HANDLERS = [
     ('after-call.cloudformation.GetTemplate', json_decode_template_body),
     ('after-call.s3.GetBucketLocation', parse_get_bucket_location),
 
+    ('before-parameter-build.s3', validate_bucket_name),
+
     ('before-call.s3.PutBucketTagging', calculate_md5),
     ('before-call.s3.PutBucketLifecycle', calculate_md5),
     ('before-call.s3.PutBucketCors', calculate_md5),


Test patch:
diff --git a/tests/functional/test_s3.py b/tests/functional/test_s3.py
new file mode 100644
index 000000000..4a78aa58f
--- /dev/null
+++ b/tests/functional/test_s3.py
@@ -0,0 +1,25 @@
+# Copyright 2015 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You
+# may not use this file except in compliance with the License. A copy of
+# the License is located at
+#
+# http://aws.amazon.com/apache2.0/
+#
+# or in the "license" file accompanying this file. This file is
+# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
+# ANY KIND, either express or implied. See the License for the specific
+# language governing permissions and limitations under the License.
+from tests import unittest
+
+import botocore.session
+from botocore.exceptions import ParamValidationError
+
+
+class TestS3BucketValidation(unittest.TestCase):
+    def test_invalid_bucket_name_raises_error(self):
+        session = botocore.session.get_session()
+        s3 = session.create_client('s3')
+        with self.assertRaises(ParamValidationError):
+            s3.put_object(Bucket='adfgasdfadfs/bucket/name',
+                          Key='foo', Body=b'asdf')
diff --git a/tests/unit/test_handlers.py b/tests/unit/test_handlers.py
index 4d4cae932..b93827fa1 100644
--- a/tests/unit/test_handlers.py
+++ b/tests/unit/test_handlers.py
@@ -19,6 +19,7 @@ import copy
 
 import botocore
 import botocore.session
+from botocore.exceptions import ParamValidationError
 from botocore.awsrequest import AWSRequest
 from botocore.compat import quote, six
 from botocore.model import OperationModel, ServiceModel
@@ -486,6 +487,47 @@ class TestHandlers(BaseSessionTest):
             request_dict['headers']['Content-MD5'],
             'OFj2IjCsPJFfMAxmQxLGPw==')
 
+    def test_invalid_char_in_bucket_raises_exception(self):
+        params = {
+            'Bucket': 'bad/bucket/name',
+            'Key': 'foo',
+            'Body': b'asdf',
+        }
+        with self.assertRaises(ParamValidationError):
+            handlers.validate_bucket_name(params)
+
+    def test_bucket_too_long_raises_exception(self):
+        params = {
+            'Bucket': 'a' * 300,
+            'Key': 'foo',
+            'Body': b'asdf',
+        }
+        with self.assertRaises(ParamValidationError):
+            handlers.validate_bucket_name(params)
+
+    def test_not_dns_compat_but_still_valid_bucket_name(self):
+        params = {
+            'Bucket': 'foasdf......bar--baz-a_b_CD10',
+            'Key': 'foo',
+            'Body': b'asdf',
+        }
+        self.assertIsNone(handlers.validate_bucket_name(params))
+
+    def test_valid_bucket_name_hyphen(self):
+        self.assertIsNone(
+            handlers.validate_bucket_name({'Bucket': 'my-bucket-name'}))
+
+    def test_valid_bucket_name_underscore(self):
+        self.assertIsNone(
+            handlers.validate_bucket_name({'Bucket': 'my_bucket_name'}))
+
+    def test_valid_bucket_name_period(self):
+        self.assertIsNone(
+            handlers.validate_bucket_name({'Bucket': 'my.bucket.name'}))
+
+    def test_validation_is_noop_if_no_bucket_param_exists(self):
+        self.assertIsNone(handlers.validate_bucket_name(params={}))
+
 
 class TestRetryHandlerOrder(BaseSessionTest):
     def get_handler_names(self, responses):