utils: paths with empty components are invalid

Note that you can still have a "//" in the path with rest_with_last, though.

Change-Id: I171afcd67b162634189b752ff92a4f43484bc12a

swift/common/utils/base.py

@@ -132,5 +132,7 @@ def split_path(path, minsegs=1, maxsegs=None, rest_with_last=False):
                 (count == maxsegs + 1 and segs[maxsegs])):
             raise ValueError('Invalid path: %s' % quote(path))
     segs = segs[1:maxsegs]
+    if not all(segs[:-1]):
+        raise ValueError('Invalid path: %s' % quote(path))
     segs.extend([None] * (maxsegs - 1 - len(segs)))
     return segs

test/functional/__init__.py

@@ -1092,10 +1092,10 @@ def connection(url):
 
     # Add the policy header if policy_specified is set
     def request_with_policy(method, url, body=None, headers={}):
-        version, account, container, obj = split_path(url, 1, 4, True)
+        if policy_specified and method == 'PUT':
-        if policy_specified and method == 'PUT' and container and not obj \
+            version, account, container, obj = split_path(url, 1, 4, True)
-                and 'X-Storage-Policy' not in headers:
+            if container and not obj and 'X-Storage-Policy' not in headers:
-            headers['X-Storage-Policy'] = policy_specified
+                headers['X-Storage-Policy'] = policy_specified
 
         return orig_request(method, url, body, headers)
 

test/functional/test_protocol.py

@@ -32,6 +32,14 @@ def tearDownModule():
 class TestHttpProtocol(unittest.TestCase):
     existing_metadata = None
 
+    def _check_transaction_id(self, resp):
+        self.assertIsNotNone(resp.getheader('X-Trans-Id'))
+        self.assertIsNotNone(resp.getheader('X-Openstack-Request-Id'))
+        self.assertIn('tx', resp.getheader('X-Trans-Id'))
+        self.assertIn('tx', resp.getheader('X-Openstack-Request-Id'))
+        self.assertEqual(resp.getheader('X-Openstack-Request-Id'),
+                         resp.getheader('X-Trans-Id'))
+
     def test_invalid_path_info(self):
         if tf.skip:
             raise SkipTest
@@ -43,11 +51,28 @@ class TestHttpProtocol(unittest.TestCase):
 
         resp = retry(get)
         resp.read()
-
         self.assertEqual(resp.status, 412)
-        self.assertIsNotNone(resp.getheader('X-Trans-Id'))
+        self._check_transaction_id(resp)
-        self.assertIsNotNone(resp.getheader('X-Openstack-Request-Id'))
+
-        self.assertIn('tx', resp.getheader('X-Trans-Id'))
+    def _do_test_path_missing_element(self, path):
-        self.assertIn('tx', resp.getheader('X-Openstack-Request-Id'))
+        if tf.skip:
-        self.assertEqual(resp.getheader('X-Openstack-Request-Id'),
+            raise SkipTest
-                         resp.getheader('X-Trans-Id'))
+
+        def get(url, token, parsed, conn, **kwargs):
+            conn.request('GET', path, '', {'X-Auth-Token': token})
+            resp = check_response(conn)
+            resp.read()
+            return resp
+
+        resp = retry(get, resource=path)
+        self.assertEqual(resp.status, 404)
+        self._check_transaction_id(resp)
+
+    def test_path_missing_account(self):
+        self._do_test_path_missing_element('/v1//testc/testo')
+
+    def test_path_missing_container(self):
+        self._do_test_path_missing_element('/v1/testa//testo')
+
+    def test_path_missing_account_and_container(self):
+        self._do_test_path_missing_element('/v1///testo')

test/unit/common/middleware/test_proxy_logging.py

@@ -302,28 +302,28 @@ class TestProxyLogging(unittest.TestCase):
     def test_log_request_stat_type_bad(self):
         app = proxy_logging.ProxyLoggingMiddleware(
             FakeApp(), {}, logger=self.logger)
-        for bad_path, prefix in [
+        for bad_path in [
-                ('', 'UNKNOWN'),
+                '',
-                ('/', 'UNKNOWN'),
+                '/',
-                ('/bad', 'UNKNOWN'),
+                '/bad',
-                ('/baddy/mc_badderson', 'UNKNOWN'),
+                '/baddy/mc_badderson',
-                ('/v1', 'UNKNOWN'),
+                '/v1',
-                ('/v1/', 'UNKNOWN'),
+                '/v1/',
-                ('/v1.0', 'UNKNOWN'),
+                '/v1.0',
-                ('/v1.0/', 'UNKNOWN'),
+                '/v1.0/',
-                ('/v1.0//', 'UNKNOWN'),
+                '/v1.0//',
-                ('/v1.0//c', 'UNKNOWN'),
+                '/v1.0//c',
-                ('/v1.0/a//', 'account'),
+                '/v1.0/a//',
-                ('/v1.0/a//o', 'object'),
+                '/v1.0/a//o',
         ]:
             req = Request.blank(bad_path, environ={'REQUEST_METHOD': 'GET'})
             now = 10000.0
             app.log_request(req, 123, 7, 13, now, now + 2.71828182846)
             self.assertEqual(
-                [((prefix + '.GET.123.timing', 2718.2818284600216), {})],
+                [(('UNKNOWN.GET.123.timing', 2718.2818284600216), {})],
                 app.access_logger.statsd_client.calls['timing'])
             self.assertEqual(
-                [((prefix + '.GET.123.xfer', 20), {})],
+                [(('UNKNOWN.GET.123.xfer', 20), {})],
                 app.access_logger.statsd_client.calls['update_stats'])
             app.access_logger.clear()
 

test/unit/common/test_utils.py

@@ -659,11 +659,15 @@ class TestUtils(unittest.TestCase):
         self.assertRaises(ValueError, utils.split_path, '/a', 2)
         self.assertRaises(ValueError, utils.split_path, '/a', 2, 3)
         self.assertRaises(ValueError, utils.split_path, '/a', 2, 3, True)
+        self.assertRaises(ValueError, utils.split_path, '/v/a//o', 1, 4, True)
+        self.assertRaises(ValueError, utils.split_path, '/v/a//o', 2, 4, True)
         self.assertEqual(utils.split_path('/a/c', 2), ['a', 'c'])
         self.assertEqual(utils.split_path('/a/c/o', 3), ['a', 'c', 'o'])
         self.assertRaises(ValueError, utils.split_path, '/a/c/o/r', 3, 3)
         self.assertEqual(utils.split_path('/a/c/o/r', 3, 3, True),
                          ['a', 'c', 'o/r'])
+        self.assertEqual(utils.split_path('/a/c//o', 1, 3, True),
+                         ['a', 'c', '/o'])
         self.assertEqual(utils.split_path('/a/c', 2, 3, True),
                          ['a', 'c', None])
         self.assertRaises(ValueError, utils.split_path, '/a', 5, 4)