diff --git a/setup.py b/setup.py index 9cdfb3c..9b6657a 100644 --- a/setup.py +++ b/setup.py @@ -47,5 +47,9 @@ setup( 'paste.app_factory': [ 'object=swiftonfile.swift.obj.server:app_factory', ], + 'paste.filter_factory': [ + 'sof_constraints=swiftonfile.swift.common.middleware.' + 'check_constraints:filter_factory', + ], }, ) diff --git a/swiftonfile/swift/common/constraints.py b/swiftonfile/swift/common/constraints.py index 7834fc7..7d98feb 100644 --- a/swiftonfile/swift/common/constraints.py +++ b/swiftonfile/swift/common/constraints.py @@ -15,8 +15,8 @@ import os from swift.common.swob import HTTPBadRequest -import swift.common.constraints +SOF_MAX_CONTAINER_NAME_LENGTH = 255 SOF_MAX_OBJECT_NAME_LENGTH = 221 # Why 221 ? # The longest filename supported by XFS in 255. @@ -38,17 +38,12 @@ def validate_obj_name_component(obj): return 'cannot be . or ..' return '' -# Store Swift's check_object_creation method to be invoked later -swift_check_object_creation = swift.common.constraints.check_object_creation - -# Define our new one which invokes the original -def sof_check_object_creation(req, object_name): +def check_object_creation(req, object_name): """ Check to ensure that everything is alright about an object to be created. - Monkey patches swift.common.constraints.check_object_creation, invoking - the original, and then adding an additional check for individual object - name components. + Swift-on-File has extra constraints on object names regarding the + length of directories and the actual file name created on the Filesystem. :param req: HTTP request object :param object_name: name of object to be created @@ -58,17 +53,14 @@ def sof_check_object_creation(req, object_name): :raises HTTPBadRequest: missing or bad content-type header, or bad metadata """ - # Invoke Swift's method - ret = swift_check_object_creation(req, object_name) - # SoF's additional checks - if ret is None: - for obj in object_name.split(os.path.sep): - reason = validate_obj_name_component(obj) - if reason: - bdy = 'Invalid object name "%s", component "%s" %s' \ - % (object_name, obj, reason) - ret = HTTPBadRequest(body=bdy, - request=req, - content_type='text/plain') + ret = None + for obj in object_name.split(os.path.sep): + reason = validate_obj_name_component(obj) + if reason: + bdy = 'Invalid object name "%s", component "%s" %s' \ + % (object_name, obj, reason) + ret = HTTPBadRequest(body=bdy, + request=req, + content_type='text/plain') return ret diff --git a/swiftonfile/swift/common/middleware/__init__.py b/swiftonfile/swift/common/middleware/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/swiftonfile/swift/common/middleware/check_constraints.py b/swiftonfile/swift/common/middleware/check_constraints.py new file mode 100644 index 0000000..ee26ee5 --- /dev/null +++ b/swiftonfile/swift/common/middleware/check_constraints.py @@ -0,0 +1,104 @@ +# Copyright (c) 2012-2014 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +The ``sof_constraints`` middleware should be added to the pipeline in your +``/etc/swift/proxy-server.conf`` file, and a mapping of storage policies +using the swiftonfile object server should be listed in the 'policies' +variable in the filter section. + +The swiftonfile constraints contains additional checks to make sure object +names conform with POSIX filesystems file and directory naming limitations + +For example:: + + [pipeline:main] + pipeline = catch_errors sof_constraints cache proxy-server + + [filter:sof_constraints] + use = egg:swift#sof_constraints + policies=swiftonfile,gold +""" + +from urllib import unquote +from swift.common.utils import get_logger +from swift.common.swob import Request, HTTPBadRequest +from swift.proxy.controllers.base import get_container_info +from swift.common.storage_policy import POLICIES +from swiftonfile.swift.common import constraints +from swiftonfile.swift.common.constraints import check_object_creation \ + as sof_check_object_creation + + +class CheckConstraintsMiddleware(object): + + def __init__(self, app, conf): + self.app = app + self.logger = get_logger(conf, log_route='constraints') + self.swift_dir = conf.get('swift_dir', '/etc/swift') + self.policies = conf.get('policies', '') + + def __call__(self, env, start_response): + request = Request(env) + + if request.method == 'PUT': + try: + version, account, container, obj = \ + request.split_path(1, 4, True) + except ValueError: + return self.app(env, start_response) + + # check container creation request + if account and container and not obj: + policy_name = request.headers.get('X-Storage-Policy', '') + default_policy = POLICIES.default.name + if (policy_name in self.policies) or \ + (policy_name == '' and default_policy in self.policies): + + container = unquote(container) + if len(container) > constraints. \ + SOF_MAX_CONTAINER_NAME_LENGTH: + resp = HTTPBadRequest(request=request) + resp.body = \ + 'Container name length of %d longer than %d' % \ + (len(container), + constraints.SOF_MAX_CONTAINER_NAME_LENGTH) + return resp(env, start_response) + elif account and container and obj: + # check object creation request + obj = unquote(obj) + + container_info = get_container_info( + env, self.app) + policy = POLICIES.get_by_index( + container_info['storage_policy']) + + if policy.name in self.policies: + error_response = sof_check_object_creation(request, obj) + if error_response: + self.logger.warn("returning error: %s", error_response) + return error_response(env, start_response) + + return self.app(env, start_response) + + +def filter_factory(global_conf, **local_conf): + conf = global_conf.copy() + conf.update(local_conf) + + def check_constraints_filter(app): + return CheckConstraintsMiddleware(app, conf) + + return check_constraints_filter diff --git a/swiftonfile/swift/obj/diskfile.py b/swiftonfile/swift/obj/diskfile.py index 025f932..a296686 100644 --- a/swiftonfile/swift/obj/diskfile.py +++ b/swiftonfile/swift/obj/diskfile.py @@ -559,9 +559,9 @@ class DiskFile(object): Manage object files on disk. Object names ending or beginning with a '/' as in /a, a/, /a/b/, - etc, or object names with multiple consecutive slahes, like a//b, - are not supported. The proxy server's contraints filter - swiftonfile.common.constrains.sof_check_object_creation() should + etc, or object names with multiple consecutive slashes, like a//b, + are not supported. The proxy server's constraints filter + swiftonfile.common.constrains.check_object_creation() should reject such requests. :param mgr: associated on-disk manager instance diff --git a/swiftonfile/swift/obj/server.py b/swiftonfile/swift/obj/server.py index cec3809..0dac3e0 100644 --- a/swiftonfile/swift/obj/server.py +++ b/swiftonfile/swift/obj/server.py @@ -15,9 +15,9 @@ """ Object Server for Gluster for Swift """ -import swiftonfile.swift.common.constraints # noqa from swift.common.swob import HTTPConflict from swift.common.utils import public, timing_stats +from swift.common.request_helpers import get_name_and_placement from swiftonfile.swift.common.exceptions import AlreadyExistsAsFile, \ AlreadyExistsAsDir from swift.common.request_helpers import split_and_validate_path @@ -25,6 +25,7 @@ from swift.common.request_helpers import split_and_validate_path from swift.obj import server from swiftonfile.swift.obj.diskfile import DiskFileManager +from swiftonfile.swift.common.constraints import check_object_creation class ObjectController(server.ObjectController): @@ -63,8 +64,15 @@ class ObjectController(server.ObjectController): @timing_stats() def PUT(self, request): try: - server.check_object_creation = \ - swiftonfile.swift.common.constraints.sof_check_object_creation + device, partition, account, container, obj, policy_idx = \ + get_name_and_placement(request, 5, 5, True) + + # check swiftonfile constraints first + error_response = check_object_creation(request, obj) + if error_response: + return error_response + + # now call swift's PUT method return server.ObjectController.PUT(self, request) except (AlreadyExistsAsFile, AlreadyExistsAsDir): device = \ diff --git a/test/unit/common/middleware/__init__.py b/test/unit/common/middleware/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/test/unit/common/middleware/test_constraints.py b/test/unit/common/middleware/test_constraints.py new file mode 100644 index 0000000..85f07c8 --- /dev/null +++ b/test/unit/common/middleware/test_constraints.py @@ -0,0 +1,186 @@ +# Copyright (c) 2014 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import unittest +from swift.common.swob import Request, Response +from swiftonfile.swift.common.middleware import check_constraints +from mock import Mock, patch +from contextlib import nested + + +class FakeApp(object): + + def __call__(self, env, start_response): + return Response(body="OK")(env, start_response) + + +def check_object_creation(req, object_name): + return + + +class TestConstraintsMiddleware(unittest.TestCase): + + """ Tests for common.middleware.constraints.check_constraints """ + + def setUp(self): + self.conf = { + 'policies': 'swiftonfile,cephfs-policy'} + + self.container1_info_mock = Mock() + self.container1_info_mock.return_value = {'status': 0, + 'sync_key': None, 'storage_policy': '0', 'meta': {}, + 'cors': {'allow_origin': None, 'expose_headers': None, + 'max_age': None}, 'sysmeta': {}, 'read_acl': None, + 'object_count': None, 'write_acl': None, 'versions': None, + 'bytes': None} + + self.container2_info_mock = Mock() + self.container2_info_mock.return_value = {'status': 0, + 'sync_key': None, 'storage_policy': '2', 'meta': {}, + 'cors': {'allow_origin': None, 'expose_headers': None, + 'max_age': None}, 'sysmeta': {}, 'read_acl': None, + 'object_count': None, 'write_acl': None, 'versions': None, + 'bytes': None} + + self.policies_mock = Mock() + self.sof_policy_mock = Mock() + self.sof_policy_mock.name = 'swiftonfile' + attrs = {'get_by_index.return_value': self.sof_policy_mock } + self.policies_mock.configure_mock(**attrs) + + self.test_check = check_constraints.filter_factory( + self.conf)(FakeApp()) + + def test_GET(self): + path = '/V1.0/a/c/o' + resp = Request.blank(path, environ={'REQUEST_METHOD': 'GET'} + ).get_response(self.test_check) + self.assertEquals(resp.status_int, 200) + + def test_PUT_container(self): + path = '/V1.0/a/c' + resp = Request.blank(path, environ={'REQUEST_METHOD': 'PUT'} + ).get_response(self.test_check) + self.assertEquals(resp.status_int, 200) + + def test_PUT_object_with_double_slashes(self): + path = '/V1.0/a/c2//o' + + with nested( + patch("swiftonfile.swift.common.middleware.check_constraints." + "get_container_info", self.container2_info_mock), + patch("swiftonfile.swift.common.middleware.check_constraints." + "POLICIES", self.policies_mock)): + resp = Request.blank(path, environ={'REQUEST_METHOD': 'PUT'} + ).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) + + def test_PUT_object_end_with_slashes(self): + path = '/V1.0/a/c2/o/' + + with nested( + patch("swiftonfile.swift.common.middleware.check_constraints." + "get_container_info", self.container2_info_mock), + patch("swiftonfile.swift.common.middleware.check_constraints." + "POLICIES", self.policies_mock)): + resp = Request.blank(path, environ={'REQUEST_METHOD': 'PUT'} + ).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) + + def test_PUT_object_named_dot(self): + path = '/V1.0/a/c2/.' + + with nested( + patch("swiftonfile.swift.common.middleware.check_constraints." + "get_container_info", self.container2_info_mock), + patch("swiftonfile.swift.common.middleware.check_constraints." + "POLICIES", self.policies_mock)): + resp = Request.blank(path, environ={'REQUEST_METHOD': 'PUT'} + ).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) + + def test_PUT_container_with_long_names(self): + longname = 'c' * 256 + path = '/V1.0/a/' + longname + resp = Request.blank(path, method='PUT', + headers={'X-Storage-Policy': 'swiftonfile'} + ).get_response(self.test_check) + self.assertEquals(resp.status_int, 400) + + # test case where storage policy is not defined in header and + # container would be created in default policy, which happens to be + # a swiftonfile policy + default_policies_mock = Mock() + sof_policy_mock = Mock() + sof_policy_mock.name = 'swiftonfile' + attrs = {'default.return_value': self.sof_policy_mock } + default_policies_mock.configure_mock(**attrs) + with patch("swiftonfile.swift.common.middleware.check_constraints." + "POLICIES", default_policies_mock): + resp = Request.blank(path, method='PUT').get_response(self.test_check) + self.assertEquals(resp.status_int, 400) + + def test_PUT_object_with_long_names(self): + for i in (220,221): + longname = 'o' * i + path = '/V1.0/a/c2/' + longname + + with nested( + patch("swiftonfile.swift.common.middleware." + "check_constraints.get_container_info", + self.container2_info_mock), + patch("swiftonfile.swift.common.middleware." + "check_constraints.POLICIES", self.policies_mock)): + resp = Request.blank(path, environ={'REQUEST_METHOD': 'PUT'} + ).get_response(self.test_check) + self.assertEquals(resp.status_int, 200) + + longname = 'o' * 222 + path = '/V1.0/a/c2/' + longname + + with nested( + patch("swiftonfile.swift.common.middleware.check_constraints." + "get_container_info", self.container2_info_mock), + patch("swiftonfile.swift.common.middleware.check_constraints." + "POLICIES", self.policies_mock)): + resp = Request.blank(path, environ={'REQUEST_METHOD': 'PUT'} + ).get_response(self.test_check) + self.assertEquals(resp.status_int, 400) + self.assertTrue('too long' in resp.body) + + def test_PUT_object_with_policy0(self): + path = '/V1.0/a/c1//o' + + with patch("swiftonfile.swift.common.middleware." + "check_constraints.get_container_info", + self.container1_info_mock): + resp = Request.blank(path, environ={'REQUEST_METHOD': 'PUT'} + ).get_response(self.test_check) + self.assertEquals(resp.status_int, 200) + + longname = 'o' * 222 + path = '/V1.0/a/c2/' + longname + + with patch("swiftonfile.swift.common.middleware.check_constraints." + "get_container_info", self.container1_info_mock): + resp = Request.blank(path, environ={'REQUEST_METHOD': 'PUT'} + ).get_response(self.test_check) + self.assertEquals(resp.status_int, 200) diff --git a/test/unit/common/test_constraints.py b/test/unit/common/test_constraints.py index 8a1616f..a9ec7d9 100644 --- a/test/unit/common/test_constraints.py +++ b/test/unit/common/test_constraints.py @@ -40,9 +40,7 @@ class TestConstraints(unittest.TestCase): self.assertTrue(cnt.validate_obj_name_component('..')) self.assertTrue(cnt.validate_obj_name_component('')) - def test_sof_check_object_creation(self): - with patch('swiftonfile.swift.common.constraints.swift_check_object_creation', - mock_check_object_creation): - req = Mock() - req.headers = [] - self.assertFalse(cnt.sof_check_object_creation(req, 'dir/z')) + def test_check_object_creation(self): + req = Mock() + req.headers = [] + self.assertFalse(cnt.check_object_creation(req, 'dir/z'))