domain_remap: stop mangling client-provided paths

The root_path option for domain_remap seems to serve two purposes:
 - provide the first component (version) for the backend request
 - be an optional leading component for the client request, which
   should be stripped off

As a result, we have mappings like:

   c.a.example.com/v1/o -> /v1/AUTH_a/c/o

instead of

   c.a.example.com/v1/o -> /v1/AUTH_a/c/v1/o

which is rather bizarre. Why on earth did we *ever* start doing this?

Now, this second behavior is managed by a config option
(mangle_client_paths) with the default being to disable it.

Upgrade Consideration
=====================

If for some reason you *do* want to drop some parts of the
client-supplied path, add

   mangle_client_paths = True

to the [filter:domain_remap] section of your proxy-server.conf. Do this
before upgrading to avoid any loss of availability.

UpgradeImpact
Change-Id: I87944bfbf8b767e1fc36dbc7910305fa1f11eeed

swift/common/middleware/domain_remap.py

@@ -100,7 +100,8 @@ storage end points as sync destinations.
 
 from swift.common.middleware import RewriteContext
 from swift.common.swob import Request, HTTPBadRequest
-from swift.common.utils import list_from_csv, register_swift_info
+from swift.common.utils import config_true_value, list_from_csv, \
+    register_swift_info
 
 
 class _DomainRemapContext(RewriteContext):
@@ -132,6 +133,8 @@ class DomainRemapMiddleware(object):
         self.reseller_prefixes_lower = [x.lower()
                                         for x in self.reseller_prefixes]
         self.default_reseller_prefix = conf.get('default_reseller_prefix')
+        self.mangle_client_paths = config_true_value(
+            conf.get('mangle_client_paths'))
 
     def __call__(self, env, start_response):
         if not self.storage_domain:
@@ -182,7 +185,8 @@ class DomainRemapMiddleware(object):
             new_path_parts = ['', self.path_root[:-1], account]
             if container:
                 new_path_parts.append(container)
-            if (path + '/').startswith(self.path_root):
+            if self.mangle_client_paths and (path + '/').startswith(
+                    self.path_root):
                 path = path[len(self.path_root):]
             new_path_parts.append(path)
             new_path = '/'.join(new_path_parts)

test/unit/common/middleware/test_domain_remap.py

@@ -85,17 +85,17 @@ class TestDomainRemap(unittest.TestCase):
         resp = self.app(req.environ, start_response)
         self.assertEqual(resp, ['Bad domain in host header'])
 
-    def test_domain_remap_account_with_path_root(self):
+    def test_domain_remap_account_with_path_root_container(self):
         req = Request.blank('/v1', environ={'REQUEST_METHOD': 'GET'},
                             headers={'Host': 'AUTH_a.example.com'})
         resp = self.app(req.environ, start_response)
-        self.assertEqual(resp, ['/v1/AUTH_a/'])
+        self.assertEqual(resp, ['/v1/AUTH_a/v1'])
 
-    def test_domain_remap_account_container_with_path_root(self):
+    def test_domain_remap_account_container_with_path_root_obj(self):
         req = Request.blank('/v1', environ={'REQUEST_METHOD': 'GET'},
                             headers={'Host': 'c.AUTH_a.example.com'})
         resp = self.app(req.environ, start_response)
-        self.assertEqual(resp, ['/v1/AUTH_a/c/'])
+        self.assertEqual(resp, ['/v1/AUTH_a/c/v1'])
 
     def test_domain_remap_account_container_with_path_obj_slash_v1(self):
         # Include http://localhost because urlparse used in Request.__init__
@@ -111,7 +111,7 @@ class TestDomainRemap(unittest.TestCase):
                             environ={'REQUEST_METHOD': 'GET'},
                             headers={'Host': 'c.AUTH_a.example.com'})
         resp = self.app(req.environ, start_response)
-        self.assertEqual(resp, ['/v1/AUTH_a/c//v1'])
+        self.assertEqual(resp, ['/v1/AUTH_a/c/v1//v1'])
 
     def test_domain_remap_account_container_with_path_trailing_slash(self):
         req = Request.blank('/obj/', environ={'REQUEST_METHOD': 'GET'},
@@ -129,7 +129,7 @@ class TestDomainRemap(unittest.TestCase):
         req = Request.blank('/v1/obj', environ={'REQUEST_METHOD': 'GET'},
                             headers={'Host': 'c.AUTH_a.example.com'})
         resp = self.app(req.environ, start_response)
-        self.assertEqual(resp, ['/v1/AUTH_a/c/obj'])
+        self.assertEqual(resp, ['/v1/AUTH_a/c/v1/obj'])
 
     def test_domain_remap_with_path_root_and_path_no_slash(self):
         req = Request.blank('/v1obj', environ={'REQUEST_METHOD': 'GET'},
@@ -255,6 +255,58 @@ class TestDomainRemap(unittest.TestCase):
                          'http://cont.auth-uuid.example.com/test/')
 
 
+class TestDomainRemapClientMangling(unittest.TestCase):
+    def setUp(self):
+        self.app = domain_remap.DomainRemapMiddleware(FakeApp(), {
+            'mangle_client_paths': True})
+
+    def test_domain_remap_account_with_path_root_container(self):
+        req = Request.blank('/v1', environ={'REQUEST_METHOD': 'GET'},
+                            headers={'Host': 'AUTH_a.example.com'})
+        resp = self.app(req.environ, start_response)
+        self.assertEqual(resp, ['/v1/AUTH_a/'])
+
+    def test_domain_remap_account_container_with_path_root_obj(self):
+        req = Request.blank('/v1', environ={'REQUEST_METHOD': 'GET'},
+                            headers={'Host': 'c.AUTH_a.example.com'})
+        resp = self.app(req.environ, start_response)
+        self.assertEqual(resp, ['/v1/AUTH_a/c/'])
+
+    def test_domain_remap_account_container_with_path_obj_slash_v1(self):
+        # Include http://localhost because urlparse used in Request.__init__
+        # parse //v1 as http://v1
+        req = Request.blank('http://localhost//v1',
+                            environ={'REQUEST_METHOD': 'GET'},
+                            headers={'Host': 'c.AUTH_a.example.com'})
+        resp = self.app(req.environ, start_response)
+        self.assertEqual(resp, ['/v1/AUTH_a/c//v1'])
+
+    def test_domain_remap_account_container_with_root_path_obj_slash_v1(self):
+        req = Request.blank('/v1//v1',
+                            environ={'REQUEST_METHOD': 'GET'},
+                            headers={'Host': 'c.AUTH_a.example.com'})
+        resp = self.app(req.environ, start_response)
+        self.assertEqual(resp, ['/v1/AUTH_a/c//v1'])
+
+    def test_domain_remap_account_container_with_path_trailing_slash(self):
+        req = Request.blank('/obj/', environ={'REQUEST_METHOD': 'GET'},
+                            headers={'Host': 'c.AUTH_a.example.com'})
+        resp = self.app(req.environ, start_response)
+        self.assertEqual(resp, ['/v1/AUTH_a/c/obj/'])
+
+    def test_domain_remap_account_container_with_path_root_and_path(self):
+        req = Request.blank('/v1/obj', environ={'REQUEST_METHOD': 'GET'},
+                            headers={'Host': 'c.AUTH_a.example.com'})
+        resp = self.app(req.environ, start_response)
+        self.assertEqual(resp, ['/v1/AUTH_a/c/obj'])
+
+    def test_domain_remap_with_path_root_and_path_no_slash(self):
+        req = Request.blank('/v1obj', environ={'REQUEST_METHOD': 'GET'},
+                            headers={'Host': 'c.AUTH_a.example.com'})
+        resp = self.app(req.environ, start_response)
+        self.assertEqual(resp, ['/v1/AUTH_a/c/v1obj'])
+
+
 class TestSwiftInfo(unittest.TestCase):
     def setUp(self):
         utils._swift_info = {}
@@ -263,17 +315,17 @@ class TestSwiftInfo(unittest.TestCase):
     def test_registered_defaults(self):
         domain_remap.filter_factory({})
         swift_info = utils.get_swift_info()
-        self.assertTrue('domain_remap' in swift_info)
+        self.assertIn('domain_remap', swift_info)
-        self.assertTrue(
+        self.assertEqual(swift_info['domain_remap'], {
-            swift_info['domain_remap'].get('default_reseller_prefix') is None)
+            'default_reseller_prefix': None})
 
     def test_registered_nondefaults(self):
-        domain_remap.filter_factory({'default_reseller_prefix': 'cupcake'})
+        domain_remap.filter_factory({'default_reseller_prefix': 'cupcake',
+                                     'mangle_client_paths': 'yes'})
         swift_info = utils.get_swift_info()
-        self.assertTrue('domain_remap' in swift_info)
+        self.assertIn('domain_remap', swift_info)
-        self.assertEqual(
+        self.assertEqual(swift_info['domain_remap'], {
-            swift_info['domain_remap'].get('default_reseller_prefix'),
+            'default_reseller_prefix': 'cupcake'})
-            'cupcake')
 
 
 if __name__ == '__main__':