Fix bug in object name constraint

SOF can support object names of upto 1024 if it conforms to the
following constraints:

Object names can have forward slashes ('/') in them. Each segment in
between these slashes cannot exceed 255 characters with exception of
the last segment which cannot exceed 221 characters.

This constraint arises from the fact that each segment except the last
one in object name is a directory, the last segment being an actual
file on the filesystem.

Also, restored default constraint values in swift.conf since the SOF
constraints middleware (always in proxy pipeline) will take care of it.

Change-Id: Ia7dc44671a87911c092fecd0344eace92f5c225b
Signed-off-by: Prashanth Pai <ppai@redhat.com>

.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.

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.<random-string>
 # 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.

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_DIR_NAME_LENGTH = 255
-SOF_MAX_OBJECT_NAME_LENGTH = 221
+# 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.<random-string>
 # 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
+# NOTE: Each segment between slashes ('/') should not exceed 255 and the last
-# object name. Each segment between slashes ('/') should not exceed 221.
+# 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:
+    if not last_component:
-        return 'too long (%d)' % len(obj)
+        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):
+    object_name_components = object_name.split(os.path.sep)
-        reason = validate_obj_name_component(obj)
+    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)

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
 """
 

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

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
 

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
 

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):

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(
+        # Non-last object name component - success
-            cnt.validate_obj_name_component('tests' * (max_obj_len / 5)))
+        for i in (220, 221, 222, 254, 255):
-        self.assertEqual(cnt.validate_obj_name_component(
+            obj_comp_name = 'a' * i
-            'tests' * 60), 'too long (300)')
+            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(
+        # Non-last object name component - err
-            'tests' * (max_obj_len / 5 + 1)))
+        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))