diff --git a/swift/account/server.py b/swift/account/server.py index cd6484eb41..5a6ca3fa52 100644 --- a/swift/account/server.py +++ b/swift/account/server.py @@ -26,7 +26,7 @@ from eventlet import Timeout import swift.common.db from swift.common.db import AccountBroker from swift.common.utils import get_logger, get_param, hash_path, public, \ - normalize_timestamp, split_path, storage_directory, config_true_value, \ + normalize_timestamp, storage_directory, config_true_value, \ validate_device_partition, json, timing_stats from swift.common.constraints import ACCOUNT_LISTING_LIMIT, \ check_mount, check_float, check_utf8, FORMAT2CONTENT_TYPE @@ -67,7 +67,7 @@ class AccountController(object): def DELETE(self, req): """Handle HTTP DELETE request.""" try: - drive, part, account = split_path(unquote(req.path), 3) + drive, part, account = req.split_path(3) validate_device_partition(drive, part) except ValueError, err: return HTTPBadRequest(body=str(err), content_type='text/plain', @@ -89,8 +89,7 @@ class AccountController(object): def PUT(self, req): """Handle HTTP PUT request.""" try: - drive, part, account, container = split_path(unquote(req.path), - 3, 4) + drive, part, account, container = req.split_path(3, 4) validate_device_partition(drive, part) except ValueError, err: return HTTPBadRequest(body=str(err), content_type='text/plain', @@ -151,8 +150,7 @@ class AccountController(object): # refactor out the container existence check here and retest # everything. try: - drive, part, account, container = split_path(unquote(req.path), - 3, 4) + drive, part, account, container = req.split_path(3, 4) validate_device_partition(drive, part) except ValueError, err: return HTTPBadRequest(body=str(err), content_type='text/plain', @@ -193,7 +191,7 @@ class AccountController(object): def GET(self, req): """Handle HTTP GET request.""" try: - drive, part, account = split_path(unquote(req.path), 3) + drive, part, account = req.split_path(3) validate_device_partition(drive, part) except ValueError, err: return HTTPBadRequest(body=str(err), content_type='text/plain', @@ -284,7 +282,7 @@ class AccountController(object): Handler for RPC calls for account replication. """ try: - post_args = split_path(unquote(req.path), 3) + post_args = req.split_path(3) drive, partition, hash = post_args validate_device_partition(drive, partition) except ValueError, err: @@ -305,7 +303,7 @@ class AccountController(object): def POST(self, req): """Handle HTTP POST request.""" try: - drive, part, account = split_path(unquote(req.path), 3) + drive, part, account = req.split_path(3) validate_device_partition(drive, part) except ValueError, err: return HTTPBadRequest(body=str(err), content_type='text/plain', diff --git a/swift/common/middleware/bulk.py b/swift/common/middleware/bulk.py index 0decabd33a..875a4dd432 100644 --- a/swift/common/middleware/bulk.py +++ b/swift/common/middleware/bulk.py @@ -19,8 +19,8 @@ from xml.sax import saxutils from swift.common.swob import Request, HTTPBadGateway, \ HTTPCreated, HTTPBadRequest, HTTPNotFound, HTTPUnauthorized, HTTPOk, \ HTTPPreconditionFailed, HTTPRequestEntityTooLarge, HTTPNotAcceptable, \ - catch_http_exception -from swift.common.utils import split_path, json, TRUE_VALUES + wsgify +from swift.common.utils import json, TRUE_VALUES from swift.common.constraints import check_utf8, MAX_FILE_SIZE from swift.common.http import HTTP_BAD_REQUEST, HTTP_UNAUTHORIZED, \ HTTP_NOT_FOUND @@ -199,7 +199,7 @@ class Bulk(object): :returns: a swob Response """ try: - vrs, account, _junk = split_path(unquote(req.path), 2, 3, True) + vrs, account, _junk = req.split_path(2, 3, True) except ValueError: return HTTPNotFound(request=req) @@ -274,8 +274,7 @@ class Bulk(object): req.headers.get('transfer-encoding', '').lower() != 'chunked': return HTTPBadRequest('Invalid request: no content sent.') try: - vrs, account, extract_base = split_path( - unquote(req.path), 2, 3, True) + vrs, account, extract_base = req.split_path(2, 3, True) except ValueError: return HTTPNotFound(request=req) extract_base = extract_base or '' @@ -363,24 +362,22 @@ class Bulk(object): except tarfile.TarError, tar_error: return HTTPBadRequest('Invalid Tar File: %s' % tar_error) - @catch_http_exception - def __call__(self, env, start_response): - req = Request(env) + @wsgify + def __call__(self, req): extract_type = \ req.headers.get('X-Extract-Archive', '').lower().strip('.') if extract_type and req.method == 'PUT': archive_type = {'tar': '', 'tar.gz': 'gz', 'tar.bz2': 'bz2'}.get(extract_type) if archive_type is not None: - resp = self.handle_extract(req, archive_type) + return self.handle_extract(req, archive_type) else: - resp = HTTPBadRequest("Unsupported archive format") - return resp(env, start_response) + return HTTPBadRequest("Unsupported archive format") if (req.headers.get('X-Bulk-Delete', '').lower() in TRUE_VALUES and req.method == 'DELETE'): - return self.handle_delete(req)(env, start_response) + return self.handle_delete(req) - return self.app(env, start_response) + return self.app def filter_factory(global_conf, **local_conf): diff --git a/swift/common/middleware/keystoneauth.py b/swift/common/middleware/keystoneauth.py index b7cdd347cb..856614ff04 100644 --- a/swift/common/middleware/keystoneauth.py +++ b/swift/common/middleware/keystoneauth.py @@ -170,7 +170,7 @@ class KeystoneAuth(object): referrers, roles = swift_acl.parse_acl(getattr(req, 'acl', None)) try: - part = swift_utils.split_path(req.path, 1, 4, True) + part = req.split_path(1, 4, True) version, account, container, obj = part except ValueError: return HTTPNotFound(request=req) @@ -239,7 +239,7 @@ class KeystoneAuth(object): :returns: None if authorization is granted, an error page otherwise. """ try: - part = swift_utils.split_path(req.path, 1, 4, True) + part = req.split_path(1, 4, True) version, account, container, obj = part except ValueError: return HTTPNotFound(request=req) diff --git a/swift/common/middleware/ratelimit.py b/swift/common/middleware/ratelimit.py index ff51ccbfdd..11a6f1dbb1 100644 --- a/swift/common/middleware/ratelimit.py +++ b/swift/common/middleware/ratelimit.py @@ -14,7 +14,7 @@ import time import eventlet -from swift.common.utils import split_path, cache_from_env, get_logger +from swift.common.utils import cache_from_env, get_logger from swift.proxy.controllers.base import get_container_memcache_key from swift.common.memcached import MemcacheConnectionError from swift.common.swob import Request, Response @@ -227,7 +227,7 @@ class RateLimitMiddleware(object): _('Warning: Cannot ratelimit without a memcached client')) return self.app(env, start_response) try: - version, account, container, obj = split_path(req.path, 1, 4, True) + version, account, container, obj = req.split_path(1, 4, True) except ValueError: return self.app(env, start_response) ratelimit_resp = self.handle_ratelimit(req, account, container, obj) diff --git a/swift/common/middleware/recon.py b/swift/common/middleware/recon.py index aa3bd963bb..17c95dd189 100644 --- a/swift/common/middleware/recon.py +++ b/swift/common/middleware/recon.py @@ -17,7 +17,7 @@ import errno import os from swift.common.swob import Request, Response -from swift.common.utils import split_path, get_logger, config_true_value +from swift.common.utils import get_logger, config_true_value from swift.common.constraints import check_mount from resource import getpagesize from hashlib import md5 @@ -276,7 +276,7 @@ class ReconMiddleware(object): return sockstat def GET(self, req): - root, rcheck, rtype = split_path(req.path, 1, 3, True) + root, rcheck, rtype = req.split_path(1, 3, True) all_rtypes = ['account', 'container', 'object'] if rcheck == "mem": content = self.get_mem() diff --git a/swift/common/middleware/tempauth.py b/swift/common/middleware/tempauth.py index c1d1f3cf74..7a2f2462e9 100644 --- a/swift/common/middleware/tempauth.py +++ b/swift/common/middleware/tempauth.py @@ -241,7 +241,7 @@ class TempAuth(object): """ try: - version, account, container, obj = split_path(req.path, 1, 4, True) + version, account, container, obj = req.split_path(1, 4, True) except ValueError: self.logger.increment('errors') return HTTPNotFound(request=req) @@ -338,11 +338,7 @@ class TempAuth(object): req.start_time = time() handler = None try: - version, account, user, _junk = split_path( - req.path_info, - minsegs=1, - maxsegs=4, - rest_with_last=True) + version, account, user, _junk = req.split_path(1, 4, True) except ValueError: self.logger.increment('errors') return HTTPNotFound(request=req) @@ -382,8 +378,7 @@ class TempAuth(object): """ # Validate the request info try: - pathsegs = split_path(req.path_info, minsegs=1, maxsegs=3, - rest_with_last=True) + pathsegs = split_path(req.path_info, 1, 3, True) except ValueError: self.logger.increment('errors') return HTTPNotFound(request=req) diff --git a/swift/common/swob.py b/swift/common/swob.py index 741141beae..f745ce79dd 100755 --- a/swift/common/swob.py +++ b/swift/common/swob.py @@ -32,8 +32,10 @@ import urlparse import urllib2 import re import random +import functools +import inspect -from swift.common.utils import reiterate +from swift.common.utils import reiterate, split_path RESPONSE_REASONS = { @@ -856,6 +858,31 @@ class Request(object): return Response(status=status, headers=dict(headers), app_iter=app_iter, request=self) + def split_path(self, minsegs=1, maxsegs=None, rest_with_last=False): + """ + Validate and split the Request's path. + + **Examples**:: + + ['a'] = split_path('/a') + ['a', None] = split_path('/a', 1, 2) + ['a', 'c'] = split_path('/a/c', 1, 2) + ['a', 'c', 'o/r'] = split_path('/a/c/o/r', 1, 3, True) + + :param path: HTTP Request path to be split + :param minsegs: Minimum number of segments to be extracted + :param maxsegs: Maximum number of segments to be extracted + :param rest_with_last: If True, trailing data will be returned as part + of last segment. If False, and there is + trailing data, raises ValueError. + :returns: list of segments with a length of maxsegs (non-existant + segments will return as None) + :raises: ValueError if given an invalid path + """ + return split_path( + self.environ.get('SCRIPT_NAME', '') + self.environ['PATH_INFO'], + minsegs, maxsegs, rest_with_last) + def content_range_header_value(start, stop, size): return 'bytes %s-%s/%s' % (start, (stop - 1), size) @@ -1031,18 +1058,28 @@ class HTTPException(Response, Exception): Exception.__init__(self, self.status) -def catch_http_exception(func): +def wsgify(func): """ - A decorator function to wrap a __call__ function. If an HTTPException - is raised it will be appropriately returned as the response. + A decorator for translating functions which take a swob Request object and + return a Response object into WSGI callables. Also catches any raised + HTTPExceptions and treats them as a returned Response. """ - - def catch_exception_func(self, env, start_response): - try: - return func(self, env, start_response) - except HTTPException, err_resp: - return err_resp(env, start_response) - return catch_exception_func + argspec = inspect.getargspec(func) + if argspec.args and argspec.args[0] == 'self': + @functools.wraps(func) + def _wsgify(self, env, start_response): + try: + return func(self, Request(env))(env, start_response) + except HTTPException, err_resp: + return err_resp(env, start_response) + else: + @functools.wraps(func) + def _wsgify(env, start_response): + try: + return func(Request(env))(env, start_response) + except HTTPException, err_resp: + return err_resp(env, start_response) + return _wsgify class StatusMap(object): diff --git a/swift/container/server.py b/swift/container/server.py index 5959bb5431..12df62b2bd 100644 --- a/swift/container/server.py +++ b/swift/container/server.py @@ -28,7 +28,7 @@ from eventlet import Timeout import swift.common.db from swift.common.db import ContainerBroker from swift.common.utils import get_logger, get_param, hash_path, public, \ - normalize_timestamp, storage_directory, split_path, validate_sync_to, \ + normalize_timestamp, storage_directory, validate_sync_to, \ config_true_value, validate_device_partition, json, timing_stats from swift.common.constraints import CONTAINER_LISTING_LIMIT, \ check_mount, check_float, check_utf8, FORMAT2CONTENT_TYPE @@ -169,8 +169,7 @@ class ContainerController(object): def DELETE(self, req): """Handle HTTP DELETE request.""" try: - drive, part, account, container, obj = split_path( - unquote(req.path), 4, 5, True) + drive, part, account, container, obj = req.split_path(4, 5, True) validate_device_partition(drive, part) except ValueError, err: return HTTPBadRequest(body=str(err), content_type='text/plain', @@ -212,8 +211,7 @@ class ContainerController(object): def PUT(self, req): """Handle HTTP PUT request.""" try: - drive, part, account, container, obj = split_path( - unquote(req.path), 4, 5, True) + drive, part, account, container, obj = req.split_path(4, 5, True) validate_device_partition(drive, part) except ValueError, err: return HTTPBadRequest(body=str(err), content_type='text/plain', @@ -276,8 +274,7 @@ class ContainerController(object): def HEAD(self, req): """Handle HTTP HEAD request.""" try: - drive, part, account, container, obj = split_path( - unquote(req.path), 4, 5, True) + drive, part, account, container, obj = req.split_path(4, 5, True) validate_device_partition(drive, part) except ValueError, err: return HTTPBadRequest(body=str(err), content_type='text/plain', @@ -315,8 +312,7 @@ class ContainerController(object): def GET(self, req): """Handle HTTP GET request.""" try: - drive, part, account, container, obj = split_path( - unquote(req.path), 4, 5, True) + drive, part, account, container, obj = req.split_path(4, 5, True) validate_device_partition(drive, part) except ValueError, err: return HTTPBadRequest(body=str(err), content_type='text/plain', @@ -425,7 +421,7 @@ class ContainerController(object): Handle HTTP REPLICATE request (json-encoded RPC calls for replication.) """ try: - post_args = split_path(unquote(req.path), 3) + post_args = req.split_path(3) drive, partition, hash = post_args validate_device_partition(drive, partition) except ValueError, err: @@ -446,7 +442,7 @@ class ContainerController(object): def POST(self, req): """Handle HTTP POST request.""" try: - drive, part, account, container = split_path(unquote(req.path), 4) + drive, part, account, container = req.split_path(4) validate_device_partition(drive, part) except ValueError, err: return HTTPBadRequest(body=str(err), content_type='text/plain', diff --git a/test/unit/common/test_swob.py b/test/unit/common/test_swob.py index 34754768a4..8e26b7a462 100755 --- a/test/unit/common/test_swob.py +++ b/test/unit/common/test_swob.py @@ -427,6 +427,72 @@ class TestRequest(unittest.TestCase): 'QUERY_STRING': 'hello=equal&acl'}) self.assertEqual(req.path_qs, '/hi/there?hello=equal&acl') + def test_wsgify(self): + used_req = [] + + @swift.common.swob.wsgify + def _wsgi_func(req): + used_req.append(req) + return swift.common.swob.Response('200 OK') + + req = swift.common.swob.Request.blank('/hi/there') + resp = req.get_response(_wsgi_func) + self.assertEqual(used_req[0].path, '/hi/there') + self.assertEqual(resp.status_int, 200) + + def test_wsgify_raise(self): + used_req = [] + + @swift.common.swob.wsgify + def _wsgi_func(req): + used_req.append(req) + raise swift.common.swob.HTTPServerError() + + req = swift.common.swob.Request.blank('/hi/there') + resp = req.get_response(_wsgi_func) + self.assertEqual(used_req[0].path, '/hi/there') + self.assertEqual(resp.status_int, 500) + + def test_split_path(self): + """ + Copied from swift.common.utils.split_path + """ + def _test_split_path(path, minsegs=1, maxsegs=None, rwl=False): + req = swift.common.swob.Request.blank(path) + return req.split_path(minsegs, maxsegs, rwl) + self.assertRaises(ValueError, _test_split_path, '') + self.assertRaises(ValueError, _test_split_path, '/') + self.assertRaises(ValueError, _test_split_path, '//') + self.assertEquals(_test_split_path('/a'), ['a']) + self.assertRaises(ValueError, _test_split_path, '//a') + self.assertEquals(_test_split_path('/a/'), ['a']) + self.assertRaises(ValueError, _test_split_path, '/a/c') + self.assertRaises(ValueError, _test_split_path, '//c') + self.assertRaises(ValueError, _test_split_path, '/a/c/') + self.assertRaises(ValueError, _test_split_path, '/a//') + self.assertRaises(ValueError, _test_split_path, '/a', 2) + self.assertRaises(ValueError, _test_split_path, '/a', 2, 3) + self.assertRaises(ValueError, _test_split_path, '/a', 2, 3, True) + self.assertEquals(_test_split_path('/a/c', 2), ['a', 'c']) + self.assertEquals(_test_split_path('/a/c/o', 3), ['a', 'c', 'o']) + self.assertRaises(ValueError, _test_split_path, '/a/c/o/r', 3, 3) + self.assertEquals(_test_split_path('/a/c/o/r', 3, 3, True), + ['a', 'c', 'o/r']) + self.assertEquals(_test_split_path('/a/c', 2, 3, True), + ['a', 'c', None]) + self.assertRaises(ValueError, _test_split_path, '/a', 5, 4) + self.assertEquals(_test_split_path('/a/c/', 2), ['a', 'c']) + self.assertEquals(_test_split_path('/a/c/', 2, 3), ['a', 'c', '']) + try: + _test_split_path('o\nn e', 2) + except ValueError, err: + self.assertEquals(str(err), 'Invalid path: o%0An%20e') + try: + _test_split_path('o\nn e', 2, 3, True) + except ValueError, err: + self.assertEquals(str(err), 'Invalid path: o%0An%20e') + + class TestStatusMap(unittest.TestCase): def test_status_map(self):