Skip to content

Commit

Permalink
simplify bucket name check (#950)
Browse files Browse the repository at this point in the history
  • Loading branch information
balamurugana authored Aug 5, 2020
1 parent dd0643e commit c6c43ab
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 60 deletions.
3 changes: 1 addition & 2 deletions examples/progress.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@

import sys
import time
from queue import Empty, Queue
from threading import Thread
from queue import Queue, Empty


_BAR_SIZE = 20
_KILOBYTE = 1024
Expand Down
76 changes: 38 additions & 38 deletions minio/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,14 @@
from .fold_case_dict import FoldCaseDict
from .helpers import (DEFAULT_PART_SIZE, MAX_MULTIPART_COUNT, MAX_PART_SIZE,
MAX_POOL_SIZE, MIN_PART_SIZE, amzprefix_user_metadata,
dump_http, get_md5_base64digest,
check_bucket_name, dump_http, get_md5_base64digest,
get_s3_region_from_endpoint, get_scheme_host,
get_sha256_hexdigest, get_target_url, is_amz_header,
is_non_empty_string, is_supported_header,
is_valid_bucket_name, is_valid_endpoint,
is_valid_notification_config, is_valid_policy_type,
is_valid_sse_c_object, is_valid_sse_object, mkdir_p,
optimal_part_info, read_full)
is_valid_endpoint, is_valid_notification_config,
is_valid_policy_type, is_valid_sse_c_object,
is_valid_sse_object, mkdir_p, optimal_part_info,
read_full)
from .parsers import (parse_assume_role, parse_copy_object,
parse_get_bucket_notification, parse_list_buckets,
parse_list_multipart_uploads, parse_list_object_versions,
Expand Down Expand Up @@ -240,7 +240,7 @@ def select_object_content(self, bucket_name, object_name, opts):
Example::
data = client.select_object_content('foo', 'test.csv', options)
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)
is_non_empty_string(object_name)

content = xml_marshal_select(opts)
Expand Down Expand Up @@ -279,7 +279,7 @@ def make_bucket(self, bucket_name, location='us-east-1',
minio.make_bucket('foo', 'us-west-1')
minio.make_bucket('foo', 'us-west-1', object_lock=True)
"""
is_valid_bucket_name(bucket_name, True)
check_bucket_name(bucket_name, True)

# Default region for all requests.
region = self._region or 'us-east-1'
Expand Down Expand Up @@ -398,7 +398,7 @@ def bucket_exists(self, bucket_name):
else:
print("my-bucketname does not exist")
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)

try:
self._url_open('HEAD', bucket_name=bucket_name)
Expand All @@ -417,7 +417,7 @@ def remove_bucket(self, bucket_name):
Example::
minio.remove_bucket("my-bucketname")
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)
self._url_open('DELETE', bucket_name=bucket_name)

# Make sure to purge bucket_name from region cache.
Expand All @@ -433,7 +433,7 @@ def get_bucket_policy(self, bucket_name):
Example::
config = minio.get_bucket_policy("my-bucketname")
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)

response = self._url_open("GET",
bucket_name=bucket_name,
Expand Down Expand Up @@ -465,7 +465,7 @@ def set_bucket_policy(self, bucket_name, policy):
"""
is_valid_policy_type(policy)

is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)

headers = {
'Content-Length': str(len(policy)),
Expand All @@ -489,7 +489,7 @@ def get_bucket_notification(self, bucket_name):
Example::
config = minio.get_bucket_notification("my-bucketname")
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)

response = self._url_open(
"GET",
Expand All @@ -509,7 +509,7 @@ def set_bucket_notification(self, bucket_name, notifications):
Example::
minio.set_bucket_notification("my-bucketname", config)
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)
is_valid_notification_config(notifications)

content = marshal_bucket_notifications(notifications)
Expand Down Expand Up @@ -537,7 +537,7 @@ def remove_all_bucket_notification(self, bucket_name):
Example::
minio.remove_all_bucket_notification("my-bucketname")
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)

content_bytes = marshal_bucket_notifications({})
headers = {
Expand All @@ -564,7 +564,7 @@ def put_bucket_encryption(self, bucket_name, enc_config):
Example::
minio.put_bucket_encryption("my-bucketname", config)
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)

# 'Rule' is a list, so we need to go through each one of
# its key/value pair and collect the encryption values.
Expand Down Expand Up @@ -594,7 +594,7 @@ def get_bucket_encryption(self, bucket_name):
Example::
config = minio.get_bucket_encryption("my-bucketname")
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)

response = self._url_open(
"GET",
Expand All @@ -612,7 +612,7 @@ def delete_bucket_encryption(self, bucket_name):
Example::
minio.delete_bucket_encryption("my-bucketname")
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)

self._url_open(
'DELETE',
Expand Down Expand Up @@ -642,7 +642,7 @@ def listen_bucket_notification(self, bucket_name, prefix='', suffix='',
for events in iter:
print(events)
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)

# If someone explicitly set prefix to None convert it to empty string.
prefix = prefix or ''
Expand Down Expand Up @@ -679,7 +679,7 @@ def listen_bucket_notification(self, bucket_name, prefix='', suffix='',

def _do_bucket_versioning(self, bucket_name, status):
"""Do versioning support in a bucket."""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)
config = (
'<VersioningConfiguration '
'xmlns="http://s3.amazonaws.com/doc/2006-03-01/">'
Expand Down Expand Up @@ -773,7 +773,7 @@ def fget_object(self, bucket_name, object_name, file_path,
'foo', 'bar', 'localfile', version_id='VERSION-ID',
)
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)
is_non_empty_string(object_name)

if os.path.isdir(file_path):
Expand Down Expand Up @@ -867,7 +867,7 @@ def get_object(self, bucket_name, object_name, request_headers=None,
response.close()
response.release_conn()
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)
is_non_empty_string(object_name)

return self._get_partial_object(
Expand Down Expand Up @@ -906,7 +906,7 @@ def get_partial_object(self, bucket_name, object_name, offset=0, length=0,
response.close()
response.release_conn()
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)
is_non_empty_string(object_name)

return self._get_partial_object(
Expand Down Expand Up @@ -951,7 +951,7 @@ def copy_object(self, bucket_name, object_name, object_source,
"?versionId=b6602757-7c9c-449b-937f-fed504d04c94",
)
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)
is_non_empty_string(object_name)
is_non_empty_string(object_source)

Expand Down Expand Up @@ -1010,7 +1010,7 @@ def put_object(self, bucket_name, object_name, data, length,
)
"""
is_valid_sse_object(sse)
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)
is_non_empty_string(object_name)

if progress:
Expand Down Expand Up @@ -1178,7 +1178,7 @@ def stat_object(self, bucket_name, object_name, sse=None, version_id=None,
stat = minio.stat_object("my-bucketname", "my-objectname")
"""

is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)
is_non_empty_string(object_name)

headers = {}
Expand Down Expand Up @@ -1236,7 +1236,7 @@ def remove_object(self, bucket_name, object_name, version_id=None):
version_id="13f88b18-8dcd-4c83-88f2-8631fdb6250c",
)
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)
is_non_empty_string(object_name)

# No reason to store successful response, for errors
Expand Down Expand Up @@ -1293,7 +1293,7 @@ def remove_objects(self, bucket_name, objects_iter):
],
)
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)
if isinstance(objects_iter, basestring):
raise TypeError(
'objects_iter cannot be `str` or `bytes` instance. It must be '
Expand Down Expand Up @@ -1362,7 +1362,7 @@ def list_incomplete_uploads(self, bucket_name, prefix='', recursive=False):
for object in objects:
print(object)
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)

return self._list_incomplete_uploads(bucket_name, prefix, recursive)

Expand All @@ -1378,7 +1378,7 @@ def _list_incomplete_uploads(self, bucket_name, prefix='',
specified prefix.
:return: An generator of incomplete uploads in alphabetical order.
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)

# If someone explicitly set prefix to None convert it to empty string.
prefix = prefix or ''
Expand Down Expand Up @@ -1435,7 +1435,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, False)
check_bucket_name(bucket_name)
is_non_empty_string(object_name)
is_non_empty_string(upload_id)

Expand Down Expand Up @@ -1473,7 +1473,7 @@ def remove_incomplete_upload(self, bucket_name, object_name):
Example::
minio.remove_incomplete_upload("my-bucketname", "my-objectname")
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)
is_non_empty_string(object_name)

uploads = self._list_incomplete_uploads(bucket_name, object_name,
Expand Down Expand Up @@ -1543,7 +1543,7 @@ def presigned_url(self, method,
)
print(url)
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)
is_non_empty_string(object_name)

if (expires.total_seconds() < 1 or
Expand Down Expand Up @@ -1739,7 +1739,7 @@ def _get_partial_object(self, bucket_name, object_name,
"""
is_valid_sse_c_object(sse)
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)
is_non_empty_string(object_name)

headers = request_headers or {}
Expand Down Expand Up @@ -1782,7 +1782,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, False)
check_bucket_name(bucket_name)
is_non_empty_string(object_name)

# Accept only bytes - otherwise we need to know how to encode
Expand Down Expand Up @@ -1865,7 +1865,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, False)
check_bucket_name(bucket_name)
is_non_empty_string(object_name)
if not callable(getattr(data, 'read')):
raise ValueError(
Expand Down Expand Up @@ -1976,7 +1976,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, False)
check_bucket_name(bucket_name)
is_non_empty_string(object_name)

headers = {}
Expand All @@ -2002,7 +2002,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, False)
check_bucket_name(bucket_name)
is_non_empty_string(object_name)
is_non_empty_string(upload_id)

Expand Down Expand Up @@ -2252,7 +2252,7 @@ def _list_objects( # pylint: disable=too-many-arguments,too-many-branches
policies.
"""

is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)

if version_id_marker:
include_version = True
Expand Down
14 changes: 3 additions & 11 deletions minio/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,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, False)
check_bucket_name(bucket_name)

parsed_url = urlsplit(endpoint_url)
# bucket_name can be valid but '.' in the hostname will fail
Expand All @@ -362,16 +362,9 @@ def is_virtual_host(endpoint_url, bucket_name):
's3-accelerate.amazonaws.com', 's3.amazonaws.com', 'aliyuncs.com'])


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
access via virtual-hosting style.
def check_bucket_name(bucket_name, strict=False):
"""Check whether bucket name is valid optional with strict check or not."""

:param bucket_name: Bucket name in *str*.
:return: True if the bucket is valid. Raise :exc:`InvalidBucketError`
otherwise.
"""
# Verify bucket name is not empty
bucket_name = str(bucket_name).strip()
if not bucket_name:
Expand Down Expand Up @@ -405,7 +398,6 @@ def is_valid_bucket_name(bucket_name, strict):
if (not match) or match.end() != len(bucket_name):
raise InvalidBucketError('Bucket name does not follow S3 standards.'
' Bucket: {0}'.format(bucket_name))
return True


def is_non_empty_string(input_string):
Expand Down
4 changes: 2 additions & 2 deletions minio/post_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import json

from .error import InvalidArgumentError
from .helpers import is_non_empty_string, is_valid_bucket_name
from .helpers import check_bucket_name, is_non_empty_string


# Policy explanation:
Expand Down Expand Up @@ -89,7 +89,7 @@ def set_bucket_name(self, bucket_name):
:param bucket_name: set bucket name.
"""
is_valid_bucket_name(bucket_name, False)
check_bucket_name(bucket_name)

self.policies.append(('eq', '$bucket', bucket_name))
self.form_data['bucket'] = bucket_name
Expand Down
Loading

0 comments on commit c6c43ab

Please sign in to comment.