x/swiftonhpss
Code Issues Proposed changes

Allow marker dir objects ending with slash

Browse Source 
Allow a slash at the end of object file name but ONLY if the PUT
request also has a "Content-type: application/directory" header.

Change-Id: Ic775de052ee3635e95f5e32ca6e2038909fe1005
Signed-off-by: Prashanth Pai <ppai@redhat.com>
This commit is contained in:
Prashanth Pai 2015-02-10 00:01:07 +05:30
parent aa1bfb3e67
commit c6aae2b0cc
3 changed files with 62 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
swiftonfile/swift/common/constraints.py
View File

@ -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:
if len(obj) > SOF_MAX_DIR_NAME_LENGTH:
return 'too long (%d)' % len(obj)
# 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 '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

5
test/unit/common/middleware/test_constraints.py
View File

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

38
test/unit/common/test_constraints.py
View File

@ -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/"))