adding new exception type and catching other invalid paths

swift/account/server.py

@@ -34,7 +34,7 @@ from swift.common.utils import get_logger, get_param, hash_path, \
 from swift.common.constraints import ACCOUNT_LISTING_LIMIT, \
     check_mount, check_float, check_utf8
 from swift.common.db_replicator import ReplicatorRpc
-
+from swift.common.exceptions import InvalidPathError
 
 DATADIR = 'accounts'
 
@@ -60,7 +60,7 @@ class AccountController(object):
         """Handle HTTP DELETE request."""
         try:
             drive, part, account = split_path(unquote(req.path), 3)
-        except ValueError, err:
+        except InvalidPathError, err:
             return HTTPBadRequest(body=str(err), content_type='text/plain',
                                                     request=req)
         if self.mount_check and not check_mount(self.root, drive):
@@ -77,7 +77,12 @@ class AccountController(object):
 
     def PUT(self, req):
         """Handle HTTP PUT request."""
-        drive, part, account, container = split_path(unquote(req.path), 3, 4)
+        try:
+            drive, part, account, container = split_path(unquote(req.path),
+                                                         3, 4)
+        except InvalidPathError, err:
+            return HTTPBadRequest(body=str(err), content_type='text/plain',
+                                  request=req)
         if self.mount_check and not check_mount(self.root, drive):
             return Response(status='507 %s is not mounted' % drive)
         broker = self._get_account_broker(drive, part, account)
@@ -130,7 +135,7 @@ class AccountController(object):
         try:
             drive, part, account, container = split_path(unquote(req.path),
                                                          3, 4)
-        except ValueError, err:
+        except InvalidPathError, err:
             return HTTPBadRequest(body=str(err), content_type='text/plain',
                                                     request=req)
         if self.mount_check and not check_mount(self.root, drive):
@@ -161,7 +166,7 @@ class AccountController(object):
         """Handle HTTP GET request."""
         try:
             drive, part, account = split_path(unquote(req.path), 3)
-        except ValueError, err:
+        except InvalidPathError, err:
             return HTTPBadRequest(body=str(err), content_type='text/plain',
                                                     request=req)
         if self.mount_check and not check_mount(self.root, drive):
@@ -252,7 +257,7 @@ class AccountController(object):
         """
         try:
             post_args = split_path(unquote(req.path), 3)
-        except ValueError, err:
+        except InvalidPathError, err:
             return HTTPBadRequest(body=str(err), content_type='text/plain',
                                                     request=req)
         drive, partition, hash = post_args
@@ -270,7 +275,7 @@ class AccountController(object):
         """Handle HTTP POST request."""
         try:
             drive, part, account = split_path(unquote(req.path), 3)
-        except ValueError, err:
+        except InvalidPathError, err:
             return HTTPBadRequest(body=str(err), content_type='text/plain',
                                   request=req)
         if 'x-timestamp' not in req.headers or \

swift/auth/server.py

@@ -30,6 +30,7 @@ from webob.exc import HTTPBadRequest, HTTPConflict, HTTPForbidden, \
 from swift.common.bufferedhttp import http_connect_raw as http_connect
 from swift.common.db import get_db_connection
 from swift.common.utils import get_logger, split_path
+from swift.common.exceptions import InvalidPathError
 
 
 class AuthController(object):
@@ -415,7 +416,7 @@ YOU HAVE A FEW OPTIONS:
         """
         try:
             _, token = split_path(request.path, minsegs=2)
-        except ValueError:
+        except InvalidPathError:
             return HTTPBadRequest()
         # Retrieves (TTL, account, user, cfaccount) if valid, False otherwise
         validation = self.validate_token(token)
@@ -451,7 +452,7 @@ YOU HAVE A FEW OPTIONS:
         """
         try:
             _, account_name, user_name = split_path(request.path, minsegs=3)
-        except ValueError:
+        except InvalidPathError:
             return HTTPBadRequest()
         create_reseller_admin = \
             request.headers.get('x-auth-user-reseller-admin') == 'true'
@@ -607,6 +608,8 @@ YOU HAVE A FEW OPTIONS:
             else:
                 return HTTPBadRequest(request=env)(env, start_response)
             response = handler(req)
+        except InvalidPathError:
+            return HTTPNotFound()(env, start_response)
         except:
             self.logger.exception('ERROR Unhandled exception in ReST request')
             return HTTPServiceUnavailable(request=req)(env, start_response)

swift/common/exceptions.py

@@ -52,3 +52,6 @@ class DriveNotMounted(Exception):
 
 class LockTimeout(MessageTimeout):
     pass
+
+class InvalidPathError(Exception):
+    pass

swift/common/middleware/auth.py

@@ -16,12 +16,12 @@
 from time import time
 
 from eventlet.timeout import Timeout
-from webob.exc import HTTPForbidden, HTTPUnauthorized
+from webob.exc import HTTPForbidden, HTTPUnauthorized, HTTPNotFound
 
 from swift.common.bufferedhttp import http_connect_raw as http_connect
 from swift.common.middleware.acl import clean_acl, parse_acl, referrer_allowed
 from swift.common.utils import cache_from_env, split_path, TRUE_VALUES
-
+from swift.common.exceptions import InvalidPathError
 
 class DevAuth(object):
     """Auth Middleware that uses the dev auth server."""
@@ -82,8 +82,11 @@ class DevAuth(object):
                 # With a non-empty reseller_prefix, I would like to be called
                 # back for anonymous access to accounts I know I'm the
                 # definitive auth for.
+                try:
                     version, rest = split_path(env.get('PATH_INFO', ''),
                                                1, 2, True)
+                except InvalidPathError, err:
+                    return HTTPNotFound()(env, start_response)
                 if rest and rest.startswith(self.reseller_prefix):
                     # Handle anonymous access to accounts I'm the definitive
                     # auth for.
@@ -145,6 +148,8 @@ class DevAuth(object):
         """
         Returns None if the request is authorized to continue or a standard
         WSGI response callable if not.
+
+        :raises: InvalidPathError (thrown by split_path) if given invalid path
         """
         version, account, container, obj = split_path(req.path, 1, 4, True)
         if not account or not account.startswith(self.reseller_prefix):

swift/common/middleware/ratelimit.py

@@ -18,7 +18,7 @@ from webob.exc import HTTPNotFound
 
 from swift.common.utils import split_path, cache_from_env, get_logger
 from swift.proxy.server import get_container_memcache_key
-
+from swift.common.exceptions import InvalidPathError
 
 class MaxSleepTimeHit(Exception):
     pass
@@ -207,7 +207,7 @@ class RateLimitMiddleware(object):
             self.memcache_client = cache_from_env(env)
         try:
             version, account, container, obj = split_path(req.path, 1, 4, True)
-        except ValueError:
+        except InvalidPathError:
             return HTTPNotFound()(env, start_response)
         ratelimit_resp = self.handle_ratelimit(req, account, container, obj)
         if ratelimit_resp is None:

swift/common/utils.py

@@ -40,7 +40,8 @@ import eventlet
 from eventlet import greenio, GreenPool, sleep, Timeout, listen
 from eventlet.green import socket, subprocess, ssl, thread, threading
 
-from swift.common.exceptions import LockTimeout, MessageTimeout
+from swift.common.exceptions import LockTimeout, MessageTimeout, \
+    InvalidPathError
 
 # logging doesn't import patched as cleanly as one would like
 from logging.handlers import SysLogHandler
@@ -208,12 +209,13 @@ def split_path(path, minsegs=1, maxsegs=None, rest_with_last=False):
                            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
+    :raises: InvalidPathError if given an invalid path
     """
     if not maxsegs:
         maxsegs = minsegs
     if minsegs > maxsegs:
-        raise ValueError('minsegs > maxsegs: %d > %d' % (minsegs, maxsegs))
+        raise InvalidPathError('minsegs > maxsegs: %d > %d' % (minsegs,
+                                                               maxsegs))
     if rest_with_last:
         segs = path.split('/', maxsegs)
         minsegs += 1
@@ -221,7 +223,7 @@ def split_path(path, minsegs=1, maxsegs=None, rest_with_last=False):
         count = len(segs)
         if segs[0] or count < minsegs or count > maxsegs or \
            '' in segs[1:minsegs]:
-            raise ValueError('Invalid path: %s' % quote(path))
+            raise InvalidPathError('Invalid path: %s' % quote(path))
     else:
         minsegs += 1
         maxsegs += 1
@@ -229,7 +231,7 @@ def split_path(path, minsegs=1, maxsegs=None, rest_with_last=False):
         count = len(segs)
         if segs[0] or count < minsegs or count > maxsegs + 1 or \
            '' in segs[1:minsegs] or (count == maxsegs + 1 and segs[maxsegs]):
-            raise ValueError('Invalid path: %s' % quote(path))
+            raise InvalidPathError('Invalid path: %s' % quote(path))
     segs = segs[1:maxsegs]
     segs.extend([None] * (maxsegs - 1 - len(segs)))
     return segs

swift/container/server.py

@@ -35,7 +35,7 @@ from swift.common.utils import get_logger, get_param, hash_path, \
 from swift.common.constraints import CONTAINER_LISTING_LIMIT, \
     check_mount, check_float, check_utf8
 from swift.common.bufferedhttp import http_connect
-from swift.common.exceptions import ConnectionTimeout
+from swift.common.exceptions import ConnectionTimeout, InvalidPathError
 from swift.common.db_replicator import ReplicatorRpc
 
 DATADIR = 'containers'
@@ -130,7 +130,7 @@ class ContainerController(object):
         try:
             drive, part, account, container, obj = split_path(
                 unquote(req.path), 4, 5, True)
-        except ValueError, err:
+        except InvalidPathError, err:
             return HTTPBadRequest(body=str(err), content_type='text/plain',
                                 request=req)
         if 'x-timestamp' not in req.headers or \
@@ -166,7 +166,7 @@ class ContainerController(object):
         try:
             drive, part, account, container, obj = split_path(
                 unquote(req.path), 4, 5, True)
-        except ValueError, err:
+        except InvalidPathError, err:
             return HTTPBadRequest(body=str(err), content_type='text/plain',
                                 request=req)
         if 'x-timestamp' not in req.headers or \
@@ -212,7 +212,7 @@ class ContainerController(object):
         try:
             drive, part, account, container, obj = split_path(
                 unquote(req.path), 4, 5, True)
-        except ValueError, err:
+        except InvalidPathError, err:
             return HTTPBadRequest(body=str(err), content_type='text/plain',
                                 request=req)
         if self.mount_check and not check_mount(self.root, drive):
@@ -239,7 +239,7 @@ class ContainerController(object):
         try:
             drive, part, account, container, obj = split_path(
                 unquote(req.path), 4, 5, True)
-        except ValueError, err:
+        except InvalidPathError, err:
             return HTTPBadRequest(body=str(err), content_type='text/plain',
                                 request=req)
         if self.mount_check and not check_mount(self.root, drive):
@@ -343,7 +343,7 @@ class ContainerController(object):
         """
         try:
             post_args = split_path(unquote(req.path), 3)
-        except ValueError, err:
+        except InvalidPathError, err:
             return HTTPBadRequest(body=str(err), content_type='text/plain',
                                 request=req)
         drive, partition, hash = post_args
@@ -361,7 +361,7 @@ class ContainerController(object):
         """Handle HTTP POST request."""
         try:
             drive, part, account, container = split_path(unquote(req.path), 4)
-        except ValueError, err:
+        except InvalidPathError, err:
             return HTTPBadRequest(body=str(err), content_type='text/plain',
                                   request=req)
         if 'x-timestamp' not in req.headers or \

swift/obj/server.py

@@ -41,7 +41,7 @@ from swift.common.utils import mkdirs, normalize_timestamp, \
 from swift.common.bufferedhttp import http_connect
 from swift.common.constraints import check_object_creation, check_mount, \
     check_float, check_utf8
-from swift.common.exceptions import ConnectionTimeout
+from swift.common.exceptions import ConnectionTimeout, InvalidPathError
 from swift.obj.replicator import get_hashes, invalidate_hash, \
     recalculate_hashes
 
@@ -313,7 +313,7 @@ class ObjectController(object):
         try:
             device, partition, account, container, obj = \
                 split_path(unquote(request.path), 5, 5, True)
-        except ValueError, err:
+        except InvalidPathError, err:
             return HTTPBadRequest(body=str(err), request=request,
                         content_type='text/plain')
         if 'x-timestamp' not in request.headers or \
@@ -342,7 +342,7 @@ class ObjectController(object):
         try:
             device, partition, account, container, obj = \
                 split_path(unquote(request.path), 5, 5, True)
-        except ValueError, err:
+        except InvalidPathError, err:
             return HTTPBadRequest(body=str(err), request=request,
                         content_type='text/plain')
         if self.mount_check and not check_mount(self.devices, device):
@@ -414,7 +414,7 @@ class ObjectController(object):
         try:
             device, partition, account, container, obj = \
                 split_path(unquote(request.path), 5, 5, True)
-        except ValueError, err:
+        except InvalidPathError, err:
             return HTTPBadRequest(body=str(err), request=request,
                         content_type='text/plain')
         if self.mount_check and not check_mount(self.devices, device):
@@ -474,7 +474,7 @@ class ObjectController(object):
         try:
             device, partition, account, container, obj = \
                 split_path(unquote(request.path), 5, 5, True)
-        except ValueError, err:
+        except InvalidPathError, err:
             resp = HTTPBadRequest(request=request)
             resp.content_type = 'text/plain'
             resp.body = str(err)
@@ -502,7 +502,7 @@ class ObjectController(object):
         try:
             device, partition, account, container, obj = \
                 split_path(unquote(request.path), 5, 5, True)
-        except ValueError, e:
+        except InvalidPathError, e:
             return HTTPBadRequest(body=str(e), request=request,
                         content_type='text/plain')
         if 'x-timestamp' not in request.headers or \
@@ -534,8 +534,12 @@ class ObjectController(object):
         Handle REPLICATE requests for the Swift Object Server.  This is used
         by the object replicator to get hashes for directories.
         """
+        try:
             device, partition, suffix = split_path(
                 unquote(request.path), 2, 3, True)
+        except InvalidPathError, e:
+            return HTTPBadRequest(body=str(e), request=request,
+                                  content_type='text/plain')
         if self.mount_check and not check_mount(self.devices, device):
             return Response(status='507 %s is not mounted' % device)
         path = os.path.join(self.devices, device, DATADIR, partition)

swift/proxy/server.py

@@ -39,7 +39,7 @@ from swift.common.constraints import check_metadata, check_object_creation, \
     check_utf8, MAX_ACCOUNT_NAME_LENGTH, MAX_CONTAINER_NAME_LENGTH, \
     MAX_FILE_SIZE
 from swift.common.exceptions import ChunkReadTimeout, \
-    ChunkWriteTimeout, ConnectionTimeout
+    ChunkWriteTimeout, ConnectionTimeout, InvalidPathError
 
 
 def update_headers(response, headers):
@@ -1259,6 +1259,8 @@ class BaseApplication(object):
 
         :param path: path from request
         :returns: tuple of (controller class, path dictionary)
+
+        :raises: InvalidPathError (thrown by split_path) if given invalid path
         """
         version, account, container, obj = split_path(path, 1, 4, True)
         d = dict(version=version,
@@ -1297,6 +1299,8 @@ class BaseApplication(object):
                 response = self.handle_request(req)(env, start_response)
                 self.posthooklogger(env, req)
                 return response
+        except InvalidPathError:
+            HTTPNotFound()(env, start_response)
         except:
             print "EXCEPTION IN __call__: %s: %s" % \
                   (traceback.format_exc(), env)
@@ -1326,7 +1330,7 @@ class BaseApplication(object):
         try:
             try:
                 controller, path_parts = self.get_controller(req.path)
-            except ValueError:
+            except InvalidPathError:
                 return HTTPNotFound(request=req)
             if not check_utf8(req.path_info):
                 return HTTPPreconditionFailed(request=req, body='Invalid UTF8')

swift/stats/access_processor.py

@@ -18,6 +18,7 @@ from urllib import unquote
 import copy
 
 from swift.common.utils import split_path, get_logger
+from swift.common.exceptions import InvalidPathError
 
 month_map = '_ Jan Feb Mar Apr May Jun Jul Aug Sep Oct Nov Dec'.split()
 
@@ -66,10 +67,13 @@ class AccessLogProcessor(object):
             self.logger.debug('Bad server name: found "%s" expected "%s"' \
                                % (server, self.server_name))
             return {}
-        (version,
+        try:
-        account,
+            (version, account, container_name, object_name) = \
-        container_name,
+                split_path(request, 2, 4, True)
-        object_name) = split_path(request, 2, 4, True)
+        except InvalidPathError, e:
+            self.logger.debug(
+                'Invalid path: %s from data: %s' % (e, repr(raw_log)))
+            return {}
         if container_name is not None:
             container_name = container_name.split('?', 1)[0]
         if object_name is not None:

test/unit/auth/test_server.py

@@ -27,6 +27,7 @@ from webob import Request
 from swift.auth import server as auth_server
 from swift.common.db import DatabaseConnectionError, get_db_connection
 from swift.common.utils import get_logger
+from swift.common.exceptions import InvalidPathError
 
 
 class TestException(Exception):
@@ -241,7 +242,7 @@ class TestAuthServer(unittest.TestCase):
             len(set(repr(a) for a in cfaccounts) - set(failed)), 2)
 
     def test_auth_bad_path(self):
-        self.assertRaises(ValueError, self.controller.handle_auth,
+        self.assertRaises(InvalidPathError, self.controller.handle_auth,
             Request.blank('', environ={'REQUEST_METHOD': 'GET'}))
         res = self.controller.handle_auth(Request.blank('/bad',
                 environ={'REQUEST_METHOD': 'GET'}))

test/unit/common/test_utils.py

@@ -29,7 +29,7 @@ from StringIO import StringIO
 from eventlet import sleep
 
 from swift.common import utils
-
+from swift.common.exceptions import InvalidPathError
 
 class TestUtils(unittest.TestCase):
     """ Tests for swift.common.utils """
@@ -87,36 +87,36 @@ class TestUtils(unittest.TestCase):
 
     def test_split_path(self):
         """ Test swift.common.utils.split_account_path """
-        self.assertRaises(ValueError, utils.split_path, '')
+        self.assertRaises(InvalidPathError, utils.split_path, '')
-        self.assertRaises(ValueError, utils.split_path, '/')
+        self.assertRaises(InvalidPathError, utils.split_path, '/')
-        self.assertRaises(ValueError, utils.split_path, '//')
+        self.assertRaises(InvalidPathError, utils.split_path, '//')
         self.assertEquals(utils.split_path('/a'), ['a'])
-        self.assertRaises(ValueError, utils.split_path, '//a')
+        self.assertRaises(InvalidPathError, utils.split_path, '//a')
         self.assertEquals(utils.split_path('/a/'), ['a'])
-        self.assertRaises(ValueError, utils.split_path, '/a/c')
+        self.assertRaises(InvalidPathError, utils.split_path, '/a/c')
-        self.assertRaises(ValueError, utils.split_path, '//c')
+        self.assertRaises(InvalidPathError, utils.split_path, '//c')
-        self.assertRaises(ValueError, utils.split_path, '/a/c/')
+        self.assertRaises(InvalidPathError, utils.split_path, '/a/c/')
-        self.assertRaises(ValueError, utils.split_path, '/a//')
+        self.assertRaises(InvalidPathError, utils.split_path, '/a//')
-        self.assertRaises(ValueError, utils.split_path, '/a', 2)
+        self.assertRaises(InvalidPathError, utils.split_path, '/a', 2)
-        self.assertRaises(ValueError, utils.split_path, '/a', 2, 3)
+        self.assertRaises(InvalidPathError, utils.split_path, '/a', 2, 3)
-        self.assertRaises(ValueError, utils.split_path, '/a', 2, 3, True)
+        self.assertRaises(InvalidPathError, utils.split_path, '/a', 2, 3, True)
         self.assertEquals(utils.split_path('/a/c', 2), ['a', 'c'])
         self.assertEquals(utils.split_path('/a/c/o', 3), ['a', 'c', 'o'])
-        self.assertRaises(ValueError, utils.split_path, '/a/c/o/r', 3, 3)
+        self.assertRaises(InvalidPathError, utils.split_path, '/a/c/o/r', 3, 3)
         self.assertEquals(utils.split_path('/a/c/o/r', 3, 3, True),
                           ['a', 'c', 'o/r'])
         self.assertEquals(utils.split_path('/a/c', 2, 3, True),
                           ['a', 'c', None])
-        self.assertRaises(ValueError, utils.split_path, '/a', 5, 4)
+        self.assertRaises(InvalidPathError, utils.split_path, '/a', 5, 4)
         self.assertEquals(utils.split_path('/a/c/', 2), ['a', 'c'])
         self.assertEquals(utils.split_path('/a/c/', 2, 3), ['a', 'c', ''])
         try:
             utils.split_path('o\nn e', 2)
-        except ValueError, err:
+        except InvalidPathError, err:
             self.assertEquals(str(err), 'Invalid path: o%0An%20e')
         try:
             utils.split_path('o\nn e', 2, 3, True)
-        except ValueError, err:
+        except InvalidPathError, err:
             self.assertEquals(str(err), 'Invalid path: o%0An%20e')
 
     def test_NullLogger(self):