diff --git a/swiftonfile/swift/common/constraints.py b/swiftonfile/swift/common/constraints.py index adcd2a3..c1ffe6a 100644 --- a/swiftonfile/swift/common/constraints.py +++ b/swiftonfile/swift/common/constraints.py @@ -37,17 +37,37 @@ SOF_MAX_OBJECT_FILENAME_LENGTH = 221 # segment should not exceed 221. -def validate_obj_name_component(obj, last_component=False): +def validate_obj_name_component(obj, req, last_component=False): + + marker_dir = False + if req.headers.get('content-type', None): + marker_dir = req.headers.get('content-type', '').lower()\ + == 'application/directory' + if not obj: - return 'cannot begin, end, or have contiguous %s\'s' % os.path.sep - if not last_component: + # Encountered extra slash somewhere, so obj component is empty + if last_component: + if marker_dir: + # Allow directory marker objects if it ends with slash + pass # Check further for length, don't return yet + else: + return 'can end with a slash only if it is a directory marker'\ + ' object with "Content-Type: application/directory" header' + else: + return 'cannot begin, end, or have contiguous %s\'s' % os.path.sep + + if not last_component or (last_component and marker_dir): + # Will result in directory creation if len(obj) > SOF_MAX_DIR_NAME_LENGTH: - return 'too long (%d)' % len(obj) + return 'has component %s too long (%d)' % (obj, len(obj)) else: + # Last component: will result in file creation if len(obj) > SOF_MAX_OBJECT_FILENAME_LENGTH: - return 'too long (%d)' % len(obj) + return 'has component %s too long (%d)' % (obj, len(obj)) + if obj == '.' or obj == '..': - return 'cannot be . or ..' + return 'cannot have . or ..' + return '' @@ -72,11 +92,14 @@ def check_object_creation(req, object_name): for i, obj in enumerate(object_name_components): if i == (len(object_name_components) - 1): last_component = True - reason = validate_obj_name_component(obj, last_component) + reason = validate_obj_name_component(obj, req, last_component) if reason: - bdy = 'Invalid object name "%s", component "%s" %s' \ - % (object_name, obj, reason) + bdy = 'Invalid object name "%s", object path %s' \ + % (object_name, reason) ret = HTTPBadRequest(body=bdy, request=req, content_type='text/plain') + # Return on first invalid component, does not check rest of + # the object path + break return ret diff --git a/test/unit/common/middleware/test_constraints.py b/test/unit/common/middleware/test_constraints.py index 85f07c8..14b0c30 100644 --- a/test/unit/common/middleware/test_constraints.py +++ b/test/unit/common/middleware/test_constraints.py @@ -101,7 +101,8 @@ class TestConstraintsMiddleware(unittest.TestCase): ).get_response(self.test_check) self.assertEquals(resp.status_int, 400) self.assertTrue('Invalid object name' in resp.body) - self.assertTrue('cannot begin, end, or have' in resp.body) + self.assertTrue('can end with a slash only if it is a directory' + in resp.body) def test_PUT_object_named_dot(self): path = '/V1.0/a/c2/.' @@ -115,7 +116,7 @@ class TestConstraintsMiddleware(unittest.TestCase): ).get_response(self.test_check) self.assertEquals(resp.status_int, 400) self.assertTrue('Invalid object name' in resp.body) - self.assertTrue('cannot be . or ..' in resp.body) + self.assertTrue('cannot have . or ..' in resp.body) def test_PUT_container_with_long_names(self): longname = 'c' * 256 diff --git a/test/unit/common/test_constraints.py b/test/unit/common/test_constraints.py index a7a8f96..d4d86df 100644 --- a/test/unit/common/test_constraints.py +++ b/test/unit/common/test_constraints.py @@ -14,7 +14,7 @@ # limitations under the License. import unittest -from mock import Mock +from mock import patch, Mock from swiftonfile.swift.common import constraints as cnt @@ -26,39 +26,42 @@ class TestConstraints(unittest.TestCase): """ Tests for common.constraints """ def test_validate_obj_name_component(self): + req = Mock() # Non-last object name component - success for i in (220, 221, 222, 254, 255): obj_comp_name = 'a' * i - self.assertFalse(cnt.validate_obj_name_component(obj_comp_name)) + self.assertFalse(cnt.validate_obj_name_component(obj_comp_name, + req)) # Last object name component - success for i in (220, 221): obj_comp_name = 'a' * i self.assertFalse( - cnt.validate_obj_name_component(obj_comp_name, True)) + cnt.validate_obj_name_component(obj_comp_name, req, True)) def test_validate_obj_name_component_err(self): + req = Mock() # Non-last object name component - err for i in (256, 257): obj_comp_name = 'a' * i - result = cnt.validate_obj_name_component(obj_comp_name) - self.assertEqual(result, "too long (%d)" % i) + result = cnt.validate_obj_name_component(obj_comp_name, req) + self.assertTrue(("too long (%d)" % i) in result) # Last object name component - err for i in (222, 223): obj_comp_name = 'a' * i - result = cnt.validate_obj_name_component(obj_comp_name, True) - self.assertEqual(result, "too long (%d)" % i) + result = cnt.validate_obj_name_component(obj_comp_name, req, True) + self.assertTrue(("too long (%d)" % i) in result) - self.assertTrue(cnt.validate_obj_name_component('.')) - self.assertTrue(cnt.validate_obj_name_component('..')) - self.assertTrue(cnt.validate_obj_name_component('')) + self.assertTrue(cnt.validate_obj_name_component('.', req)) + self.assertTrue(cnt.validate_obj_name_component('..', req)) + self.assertTrue(cnt.validate_obj_name_component('', req)) def test_check_object_creation(self): req = Mock() - req.headers = [] + req.headers = dict() valid_object_names = ["a/b/c/d", '/'.join(("1@3%&*0-", "};+=]|")), @@ -74,3 +77,16 @@ class TestConstraints(unittest.TestCase): '/'.join(('a' * 255, 'b' * 255, 'c' * 222))] for o in invalid_object_names: self.assertTrue(cnt.check_object_creation(req, o)) + + # Check for creation of directory marker objects that ends with slash + with patch.dict(req.headers, {'content-type': + 'application/directory'}): + self.assertFalse(cnt.check_object_creation(req, "a/b/c/d/")) + + # Check creation of objects ending with slash having any other content + # type than application/directory is not allowed + for content_type in ('text/plain', 'text/html', 'image/jpg', + 'application/octet-stream', 'blah/blah'): + with patch.dict(req.headers, {'content-type': + content_type}): + self.assertTrue(cnt.check_object_creation(req, "a/b/c/d/"))