diff --git a/.functests b/.functests index 0a7c6a8..009aee3 100755 --- a/.functests +++ b/.functests @@ -59,6 +59,15 @@ Before proceeding forward, please make sure you already have: name = swiftonfile default = yes + Added sof_constraints middleware in proxy pipeline + + [pipeline:main] + pipeline = catch_errors sof_constraints cache proxy-server + + [filter:sof_constraints] + use = egg:swiftonfile#sof_constraints + policies=swiftonfile + 4. Copied etc/object-server.conf-swiftonfile to /etc/swift/object-server/5.conf 5. Generated ring files for swiftonfile policy. diff --git a/etc/swift.conf-swiftonfile b/etc/swift.conf-swiftonfile index 4bbec24..7b801c1 100644 --- a/etc/swift.conf-swiftonfile +++ b/etc/swift.conf-swiftonfile @@ -107,7 +107,12 @@ name = swiftonfile # max_object_name_length is the max number of bytes in the utf8 encoding # of an object name -max_object_name_length = 221 +# max_object_name_length = 221 +# 221 is in fact the max possible length of the last segment of object name. +# Each segment is separated by a '/'. +# For example: If object name is "abc/def/ghi/jkl", then abc,def,ghi are all +# directories and "jkl" would be the file. +# # Why 221 ? # The longest filename supported by XFS in 255. # http://lxr.free-electrons.com/source/fs/xfs/xfs_types.h#L125 @@ -115,17 +120,18 @@ max_object_name_length = 221 # .OBJECT_NAME. # The random string is 32 character long and and file name has two dots. # Hence 255 - 32 - 2 = 221 -# NOTE: This limitation can be sefely raised by having slashes in really long -# object name. Each segment between slashes ('/') should not exceed 221. +# +# NOTE: Each segment between slashes ('/') should not exceed 255 and the last +# segment should not exceed 221. # max_account_name_length is the max number of bytes in the utf8 encoding # of an account name -max_account_name_length = 255 +# max_account_name_length = 255 # max_container_name_length is the max number of bytes in the utf8 encoding # of a container name -max_container_name_length = 255 +# max_container_name_length = 255 # Why 255 ? # The longest filename supported by XFS in 255. diff --git a/swiftonfile/swift/common/constraints.py b/swiftonfile/swift/common/constraints.py index 7d98feb..adcd2a3 100644 --- a/swiftonfile/swift/common/constraints.py +++ b/swiftonfile/swift/common/constraints.py @@ -16,8 +16,16 @@ import os from swift.common.swob import HTTPBadRequest -SOF_MAX_CONTAINER_NAME_LENGTH = 255 -SOF_MAX_OBJECT_NAME_LENGTH = 221 +SOF_MAX_DIR_NAME_LENGTH = 255 +# A container is also a directory on the fileystem with the same name. Hence: +SOF_MAX_CONTAINER_NAME_LENGTH = SOF_MAX_DIR_NAME_LENGTH + +SOF_MAX_OBJECT_FILENAME_LENGTH = 221 +# SOF_MAX_OBJECT_FILENAME_LENGTH is the length of the last segment of object +# name. Each 'segment/component' is separated by a '/'. +# For example: If object name is "abc/def/ghi/jkl", then abc,def,ghi are all +# directories and "jkl" would be the file. This file name cannot exceed +# SOF_MAX_OBJECT_FILENAME_LENGTH. # Why 221 ? # The longest filename supported by XFS in 255. # http://lxr.free-electrons.com/source/fs/xfs/xfs_types.h#L125 @@ -25,15 +33,19 @@ SOF_MAX_OBJECT_NAME_LENGTH = 221 # .OBJECT_NAME. # The random string is 32 character long and and file name has two dots. # Hence 255 - 32 - 2 = 221 -# NOTE: This limitation can be sefely raised by having slashes in really long -# object name. Each segment between slashes ('/') should not exceed 221. +# NOTE: Each segment between slashes ('/') should not exceed 255 and the last +# segment should not exceed 221. -def validate_obj_name_component(obj): +def validate_obj_name_component(obj, last_component=False): if not obj: return 'cannot begin, end, or have contiguous %s\'s' % os.path.sep - if len(obj) > SOF_MAX_OBJECT_NAME_LENGTH: - return 'too long (%d)' % len(obj) + if not last_component: + if len(obj) > SOF_MAX_DIR_NAME_LENGTH: + return 'too long (%d)' % len(obj) + else: + if len(obj) > SOF_MAX_OBJECT_FILENAME_LENGTH: + return 'too long (%d)' % len(obj) if obj == '.' or obj == '..': return 'cannot be . or ..' return '' @@ -55,8 +67,12 @@ def check_object_creation(req, object_name): """ # SoF's additional checks ret = None - for obj in object_name.split(os.path.sep): - reason = validate_obj_name_component(obj) + object_name_components = object_name.split(os.path.sep) + last_component = False + 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) if reason: bdy = 'Invalid object name "%s", component "%s" %s' \ % (object_name, obj, reason) diff --git a/swiftonfile/swift/common/middleware/check_constraints.py b/swiftonfile/swift/common/middleware/check_constraints.py index ee26ee5..3a6557d 100644 --- a/swiftonfile/swift/common/middleware/check_constraints.py +++ b/swiftonfile/swift/common/middleware/check_constraints.py @@ -28,7 +28,7 @@ For example:: pipeline = catch_errors sof_constraints cache proxy-server [filter:sof_constraints] - use = egg:swift#sof_constraints + use = egg:swiftonfile#sof_constraints policies=swiftonfile,gold """ diff --git a/test/functional/swift_on_file_tests.py b/test/functional/swift_on_file_tests.py index 3537c2f..90d058e 100644 --- a/test/functional/swift_on_file_tests.py +++ b/test/functional/swift_on_file_tests.py @@ -127,7 +127,8 @@ class TestSwiftOnFile(Base): file_item.write_random() self.assert_status(201) file_info = file_item.info() - fhOnMountPoint = open(os.path.join(self.env.root_dir, + fhOnMountPoint = open(os.path.join( + self.env.root_dir, 'AUTH_' + self.env.account.name, self.env.container.name, file_name), 'r') @@ -158,3 +159,23 @@ class TestSwiftOnFile(Base): # Confirm that Etag is present in response headers self.assert_(data_hash == object_item.info()['etag']) self.assert_status(200) + + def testObjectNameConstraints(self): + valid_object_names = ["a/b/c/d", + '/'.join(("1@3%&*0-", "};+=]|")), + '/'.join(('a' * 20, 'b' * 20, 'c' * 20))] + for object_name in valid_object_names: + file_item = self.env.container.file(object_name) + file_item.write_random() + self.assert_status(201) + + invalid_object_names = ["a/./b", + "a/b/../d", + "a//b", + "a/c//", + '/'.join(('a' * 256, 'b' * 255, 'c' * 221)), + '/'.join(('a' * 255, 'b' * 255, 'c' * 222))] + + for object_name in invalid_object_names: + file_item = self.env.container.file(object_name) + self.assertRaises(ResponseError, file_item.write) # 503 or 400 diff --git a/test/functional/test_account.py b/test/functional/test_account.py index b6b279d..0b0ccff 100755 --- a/test/functional/test_account.py +++ b/test/functional/test_account.py @@ -741,6 +741,9 @@ class TestAccount(unittest.TestCase): self.assertEqual(resp.getheader('x-account-meta-two'), '2') def test_bad_metadata(self): + + raise SkipTest('SOF constraints middleware enforces constraints.') + if tf.skip: raise SkipTest diff --git a/test/functional/test_container.py b/test/functional/test_container.py index 3a6e1b9..969b9bf 100755 --- a/test/functional/test_container.py +++ b/test/functional/test_container.py @@ -368,6 +368,9 @@ class TestContainer(unittest.TestCase): self.assertEqual(resp.status, 404) def test_POST_bad_metadata(self): + + raise SkipTest('SOF constraints middleware enforces constraints.') + if tf.skip: raise SkipTest diff --git a/test/functional/tests.py b/test/functional/tests.py index bbe370f..924f193 100644 --- a/test/functional/tests.py +++ b/test/functional/tests.py @@ -351,6 +351,9 @@ class TestContainer(Base): set_up = False def testContainerNameLimit(self): + + raise SkipTest('SOF constraints middleware enforces constraints.') + limit = load_constraint('max_container_name_length') for l in (limit - 100, limit - 10, limit - 1, limit, @@ -971,6 +974,9 @@ class TestFile(Base): self.assert_status(404) def testNameLimit(self): + + raise SkipTest('SOF constraints middleware enforces constraints.') + limit = load_constraint('max_object_name_length') for l in (1, 10, limit / 2, limit - 1, limit, limit + 1, limit * 2): diff --git a/test/unit/common/test_constraints.py b/test/unit/common/test_constraints.py index a9ec7d9..a7a8f96 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, patch +from mock import Mock from swiftonfile.swift.common import constraints as cnt @@ -26,16 +26,32 @@ class TestConstraints(unittest.TestCase): """ Tests for common.constraints """ def test_validate_obj_name_component(self): - max_obj_len = cnt.SOF_MAX_OBJECT_NAME_LENGTH - self.assertFalse( - cnt.validate_obj_name_component('tests' * (max_obj_len / 5))) - self.assertEqual(cnt.validate_obj_name_component( - 'tests' * 60), 'too long (300)') + + # 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)) + + # 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)) def test_validate_obj_name_component_err(self): - max_obj_len = cnt.SOF_MAX_OBJECT_NAME_LENGTH - self.assertTrue(cnt.validate_obj_name_component( - 'tests' * (max_obj_len / 5 + 1))) + + # 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) + + # 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) + self.assertTrue(cnt.validate_obj_name_component('.')) self.assertTrue(cnt.validate_obj_name_component('..')) self.assertTrue(cnt.validate_obj_name_component('')) @@ -43,4 +59,18 @@ class TestConstraints(unittest.TestCase): def test_check_object_creation(self): req = Mock() req.headers = [] - self.assertFalse(cnt.check_object_creation(req, 'dir/z')) + + valid_object_names = ["a/b/c/d", + '/'.join(("1@3%&*0-", "};+=]|")), + '/'.join(('a' * 255, 'b' * 255, 'c' * 221))] + for o in valid_object_names: + self.assertFalse(cnt.check_object_creation(req, o)) + + invalid_object_names = ["a/./b", + "a/b/../d", + "a//b", + "a/c//", + '/'.join(('a' * 256, 'b' * 255, 'c' * 221)), + '/'.join(('a' * 255, 'b' * 255, 'c' * 222))] + for o in invalid_object_names: + self.assertTrue(cnt.check_object_creation(req, o))