From 107ba797708e07071028e021f2a261151562c9d7 Mon Sep 17 00:00:00 2001 From: Romain LE DISEZ Date: Sat, 18 Feb 2017 16:22:11 +0100 Subject: [PATCH] Rewrite redirection in cname_lookup & domain_remap cname_lookup and domain_remap are currently not catching redirections (eg: staticweb). This behavior makes the domain to change when a call through cname_lookup and domain_remap end up being redirected. Example: commit fixes it. - original request: http://mysite.com/subdir - redirected: http://cont.acct.storage.domain.com/v1/AUTH_acct/cont/subdir/ - expected: http://mysite.com/subdir/ This patch is fixing this. Closes-Bug: #1190625 Co-Authored-By: Tim Burke Change-Id: I67f642b8b070bc21e7760477d0a1e3b902ba7896 --- swift/common/middleware/__init__.py | 36 ++++++++ swift/common/middleware/cname_lookup.py | 14 +++- swift/common/middleware/domain_remap.py | 11 ++- .../common/middleware/test_cname_lookup.py | 58 ++++++++----- .../common/middleware/test_domain_remap.py | 84 +++++++++++++------ 5 files changed, 155 insertions(+), 48 deletions(-) diff --git a/swift/common/middleware/__init__.py b/swift/common/middleware/__init__.py index e69de29bb2..00c60fda13 100644 --- a/swift/common/middleware/__init__.py +++ b/swift/common/middleware/__init__.py @@ -0,0 +1,36 @@ +# Copyright (c) 2010-2017 OpenStack Foundation +# +# 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 re +from swift.common.wsgi import WSGIContext + + +class RewriteContext(WSGIContext): + base_re = None + + def __init__(self, app, requested, rewritten): + super(RewriteContext, self).__init__(app) + self.requested = requested + self.rewritten_re = re.compile(self.base_re % re.escape(rewritten)) + + def handle_request(self, env, start_response): + resp_iter = self._app_call(env) + for i, (header, value) in enumerate(self._response_headers): + if header.lower() in ('location', 'content-location'): + self._response_headers[i] = (header, self.rewritten_re.sub( + r'\1%s\2' % self.requested, value)) + start_response(self._response_status, self._response_headers, + self._response_exc_info) + return resp_iter diff --git a/swift/common/middleware/cname_lookup.py b/swift/common/middleware/cname_lookup.py index abfeabce5d..d5e7715605 100644 --- a/swift/common/middleware/cname_lookup.py +++ b/swift/common/middleware/cname_lookup.py @@ -41,6 +41,7 @@ except ImportError: else: # executed if the try block finishes with no errors MODULE_DEPENDENCY_MET = True +from swift.common.middleware import RewriteContext from swift.common.swob import Request, HTTPBadRequest from swift.common.utils import cache_from_env, get_logger, list_from_csv, \ register_swift_info @@ -80,6 +81,10 @@ def is_ip(domain): return False +class _CnameLookupContext(RewriteContext): + base_re = r'^(https?://)%s(/.*)?$' + + class CNAMELookupMiddleware(object): """ CNAME Lookup Middleware @@ -117,9 +122,9 @@ class CNAMELookupMiddleware(object): if not self.storage_domain: return self.app(env, start_response) if 'HTTP_HOST' in env: - given_domain = env['HTTP_HOST'] + requested_host = given_domain = env['HTTP_HOST'] else: - given_domain = env['SERVER_NAME'] + requested_host = given_domain = env['SERVER_NAME'] port = '' if ':' in given_domain: given_domain, port = given_domain.rsplit(':', 1) @@ -175,6 +180,11 @@ class CNAMELookupMiddleware(object): resp = HTTPBadRequest(request=Request(env), body=msg, content_type='text/plain') return resp(env, start_response) + else: + context = _CnameLookupContext(self.app, requested_host, + env['HTTP_HOST']) + return context.handle_request(env, start_response) + return self.app(env, start_response) diff --git a/swift/common/middleware/domain_remap.py b/swift/common/middleware/domain_remap.py index 23bd72ffaa..13cfee95cf 100644 --- a/swift/common/middleware/domain_remap.py +++ b/swift/common/middleware/domain_remap.py @@ -50,10 +50,15 @@ advised. With container sync, you should use the true 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 +class _DomainRemapContext(RewriteContext): + base_re = r'^(https?://[^/]+)%s(.*)$' + + class DomainRemapMiddleware(object): """ Domain Remap Middleware @@ -124,7 +129,7 @@ class DomainRemapMiddleware(object): # account prefix is not in config list. bail. return self.app(env, start_response) - path = env['PATH_INFO'] + requested_path = path = env['PATH_INFO'] new_path_parts = [self.path_root, account] if container: new_path_parts.append(container) @@ -135,6 +140,10 @@ class DomainRemapMiddleware(object): new_path_parts.append(path) new_path = '/'.join(new_path_parts) env['PATH_INFO'] = new_path + + context = _DomainRemapContext(self.app, requested_path, new_path) + return context.handle_request(env, start_response) + return self.app(env, start_response) diff --git a/test/unit/common/middleware/test_cname_lookup.py b/test/unit/common/middleware/test_cname_lookup.py index d900661bca..9d1f85984a 100644 --- a/test/unit/common/middleware/test_cname_lookup.py +++ b/test/unit/common/middleware/test_cname_lookup.py @@ -27,13 +27,21 @@ else: # executed if the try has no errors skip = False from swift.common import utils from swift.common.middleware import cname_lookup -from swift.common.swob import Request +from swift.common.swob import Request, HTTPMovedPermanently class FakeApp(object): def __call__(self, env, start_response): - return "FAKE APP" + start_response('200 OK', []) + return ["FAKE APP"] + + +class RedirectSlashApp(object): + + def __call__(self, env, start_response): + loc = env['PATH_INFO'] + '/' + return HTTPMovedPermanently(location=loc)(env, start_response) def start_response(*args): @@ -52,12 +60,12 @@ class TestCNAMELookup(unittest.TestCase): req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': '10.134.23.198'}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, 'FAKE APP') + self.assertEqual(resp, ['FAKE APP']) req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'fc00:7ea1:f155::6321:8841'}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, 'FAKE APP') + self.assertEqual(resp, ['FAKE APP']) @mock.patch('swift.common.middleware.cname_lookup.lookup_cname', new=lambda d: (0, d)) @@ -65,16 +73,16 @@ class TestCNAMELookup(unittest.TestCase): req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'foo.example.com'}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, 'FAKE APP') + self.assertEqual(resp, ['FAKE APP']) req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'foo.example.com:8080'}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, 'FAKE APP') + self.assertEqual(resp, ['FAKE APP']) req = Request.blank('/', environ={'REQUEST_METHOD': 'GET', 'SERVER_NAME': 'foo.example.com'}, headers={'Host': None}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, 'FAKE APP') + self.assertEqual(resp, ['FAKE APP']) @mock.patch('swift.common.middleware.cname_lookup.lookup_cname', new=lambda d: (0, '%s.example.com' % d)) @@ -82,16 +90,16 @@ class TestCNAMELookup(unittest.TestCase): req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'mysite.com'}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, 'FAKE APP') + self.assertEqual(resp, ['FAKE APP']) req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'mysite.com:8080'}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, 'FAKE APP') + self.assertEqual(resp, ['FAKE APP']) req = Request.blank('/', environ={'REQUEST_METHOD': 'GET', 'SERVER_NAME': 'mysite.com'}, headers={'Host': None}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, 'FAKE APP') + self.assertEqual(resp, ['FAKE APP']) def test_lookup_chain_too_long(self): req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}, @@ -146,12 +154,12 @@ class TestCNAMELookup(unittest.TestCase): 'swift.cache': memcache}, headers={'Host': 'mysite.com'}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, 'FAKE APP') + self.assertEqual(resp, ['FAKE APP']) req = Request.blank('/', environ={'REQUEST_METHOD': 'GET', 'swift.cache': memcache}, headers={'Host': 'mysite.com'}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, 'FAKE APP') + self.assertEqual(resp, ['FAKE APP']) def test_caching(self): fail_to_resolve = ['CNAME lookup failed to resolve to a valid domain'] @@ -176,7 +184,7 @@ class TestCNAMELookup(unittest.TestCase): 'swift.cache': memcache}, headers={'Host': 'mysite2.com'}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, 'FAKE APP') + self.assertEqual(resp, ['FAKE APP']) self.assertEqual(m.call_count, 1) self.assertEqual(memcache.cache.get('cname-mysite2.com'), 'c.example.com') @@ -184,7 +192,7 @@ class TestCNAMELookup(unittest.TestCase): 'swift.cache': memcache}, headers={'Host': 'mysite2.com'}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, 'FAKE APP') + self.assertEqual(resp, ['FAKE APP']) self.assertEqual(m.call_count, 1) self.assertEqual(memcache.cache.get('cname-mysite2.com'), 'c.example.com') @@ -245,7 +253,7 @@ class TestCNAMELookup(unittest.TestCase): req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'c.a.example.com'}) resp = app(req.environ, start_response) - self.assertEqual(resp, 'FAKE APP') + self.assertEqual(resp, ['FAKE APP']) def test_storage_domains_conf_format(self): conf = {'storage_domain': 'foo.com'} @@ -281,10 +289,10 @@ class TestCNAMELookup(unittest.TestCase): return app(req.environ, start_response) resp = do_test('c.storage1.com') - self.assertEqual(resp, 'FAKE APP') + self.assertEqual(resp, ['FAKE APP']) resp = do_test('c.storage2.com') - self.assertEqual(resp, 'FAKE APP') + self.assertEqual(resp, ['FAKE APP']) bad_domain = ['CNAME lookup failed to resolve to a valid domain'] resp = do_test('c.badtest.com') @@ -305,7 +313,7 @@ class TestCNAMELookup(unittest.TestCase): self.assertEqual(resp, bad_domain) resp = do_test('storage.example.com') - self.assertEqual(resp, 'FAKE APP') + self.assertEqual(resp, ['FAKE APP']) def test_resolution_to_storage_domain_exactly(self): conf = {'storage_domain': 'example.com', @@ -317,7 +325,19 @@ class TestCNAMELookup(unittest.TestCase): module = 'swift.common.middleware.cname_lookup.lookup_cname' with mock.patch(module, lambda x: (0, 'example.com')): resp = app(req.environ, start_response) - self.assertEqual(resp, 'FAKE APP') + self.assertEqual(resp, ['FAKE APP']) + + def test_redirect(self): + app = cname_lookup.CNAMELookupMiddleware(RedirectSlashApp(), {}) + + module = 'swift.common.middleware.cname_lookup.lookup_cname' + with mock.patch(module, lambda x: (0, 'cont.acct.example.com')): + req = Request.blank('/test', environ={'REQUEST_METHOD': 'GET'}, + headers={'Host': 'mysite.com'}) + resp = req.get_response(app) + self.assertEqual(resp.status_int, 301) + self.assertEqual(resp.headers.get('Location'), + 'http://mysite.com/test/') class TestSwiftInfo(unittest.TestCase): diff --git a/test/unit/common/middleware/test_domain_remap.py b/test/unit/common/middleware/test_domain_remap.py index 4013a1a1f1..0b5e2fab74 100644 --- a/test/unit/common/middleware/test_domain_remap.py +++ b/test/unit/common/middleware/test_domain_remap.py @@ -15,7 +15,7 @@ import unittest -from swift.common.swob import Request +from swift.common.swob import Request, HTTPMovedPermanently from swift.common.middleware import domain_remap from swift.common import utils @@ -23,7 +23,15 @@ from swift.common import utils class FakeApp(object): def __call__(self, env, start_response): - return env['PATH_INFO'] + start_response('200 OK', []) + return [env['PATH_INFO']] + + +class RedirectSlashApp(object): + + def __call__(self, env, start_response): + loc = env['PATH_INFO'] + '/' + return HTTPMovedPermanently(location=loc)(env, start_response) def start_response(*args): @@ -40,36 +48,36 @@ class TestDomainRemap(unittest.TestCase): 'SERVER_NAME': 'example.com'}, headers={'Host': None}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, '/') + self.assertEqual(resp, ['/']) req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'example.com'}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, '/') + self.assertEqual(resp, ['/']) req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'example.com:8080'}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, '/') + self.assertEqual(resp, ['/']) def test_domain_remap_account(self): req = Request.blank('/', environ={'REQUEST_METHOD': 'GET', 'SERVER_NAME': 'AUTH_a.example.com'}, headers={'Host': None}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, '/v1/AUTH_a/') + self.assertEqual(resp, ['/v1/AUTH_a/']) req = Request.blank('/', 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/']) req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'AUTH-uuid.example.com'}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, '/v1/AUTH_uuid/') + self.assertEqual(resp, ['/v1/AUTH_uuid/']) def test_domain_remap_account_container(self): req = Request.blank('/', 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/']) def test_domain_remap_extra_subdomains(self): req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}, @@ -81,13 +89,13 @@ class TestDomainRemap(unittest.TestCase): 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/']) def test_domain_remap_account_container_with_path_root(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/']) def test_domain_remap_account_container_with_path_obj_slash_v1(self): # Include http://localhost because urlparse used in Request.__init__ @@ -96,38 +104,38 @@ 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']) 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') + 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/') + self.assertEqual(resp, ['/v1/AUTH_a/c/obj/']) def test_domain_remap_account_container_with_path(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') + 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') + self.assertEqual(resp, ['/v1/AUTH_a/c/obj']) def test_domain_remap_account_matching_ending_not_domain(self): req = Request.blank('/dontchange', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'c.aexample.com'}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, '/dontchange') + self.assertEqual(resp, ['/dontchange']) def test_domain_remap_configured_with_empty_storage_domain(self): self.app = domain_remap.DomainRemapMiddleware(FakeApp(), @@ -135,7 +143,7 @@ class TestDomainRemap(unittest.TestCase): req = Request.blank('/test', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'c.AUTH_a.example.com'}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, '/test') + self.assertEqual(resp, ['/test']) def test_storage_domains_conf_format(self): conf = {'storage_domain': 'foo.com'} @@ -164,7 +172,7 @@ class TestDomainRemap(unittest.TestCase): req = Request.blank('/test', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'c.prefix_uuid.example.com'}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, '/v1/PREFIX_uuid/c/test') + self.assertEqual(resp, ['/v1/PREFIX_uuid/c/test']) def test_domain_remap_configured_with_bad_prefixes(self): conf = {'reseller_prefixes': 'UNKNOWN'} @@ -172,7 +180,7 @@ class TestDomainRemap(unittest.TestCase): req = Request.blank('/test', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'c.prefix_uuid.example.com'}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, '/test') + self.assertEqual(resp, ['/test']) def test_domain_remap_configured_with_no_prefixes(self): conf = {'reseller_prefixes': ''} @@ -180,7 +188,7 @@ class TestDomainRemap(unittest.TestCase): req = Request.blank('/test', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'c.uuid.example.com'}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, '/v1/uuid/c/test') + self.assertEqual(resp, ['/v1/uuid/c/test']) def test_domain_remap_add_prefix(self): conf = {'default_reseller_prefix': 'FOO'} @@ -188,7 +196,7 @@ class TestDomainRemap(unittest.TestCase): req = Request.blank('/test', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'uuid.example.com'}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, '/v1/FOO_uuid/test') + self.assertEqual(resp, ['/v1/FOO_uuid/test']) def test_domain_remap_add_prefix_already_there(self): conf = {'default_reseller_prefix': 'AUTH'} @@ -196,7 +204,7 @@ class TestDomainRemap(unittest.TestCase): req = Request.blank('/test', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'auth-uuid.example.com'}) resp = self.app(req.environ, start_response) - self.assertEqual(resp, '/v1/AUTH_uuid/test') + self.assertEqual(resp, ['/v1/AUTH_uuid/test']) def test_multiple_storage_domains(self): conf = {'storage_domain': 'storage1.com, storage2.com'} @@ -208,13 +216,37 @@ class TestDomainRemap(unittest.TestCase): return self.app(req.environ, start_response) resp = do_test('auth-uuid.storage1.com') - self.assertEqual(resp, '/v1/AUTH_uuid/test') + self.assertEqual(resp, ['/v1/AUTH_uuid/test']) resp = do_test('auth-uuid.storage2.com') - self.assertEqual(resp, '/v1/AUTH_uuid/test') + self.assertEqual(resp, ['/v1/AUTH_uuid/test']) resp = do_test('auth-uuid.storage3.com') - self.assertEqual(resp, '/test') + self.assertEqual(resp, ['/test']) + + def test_domain_remap_redirect(self): + app = domain_remap.DomainRemapMiddleware(RedirectSlashApp(), {}) + + req = Request.blank('/cont', environ={'REQUEST_METHOD': 'GET'}, + headers={'Host': 'auth-uuid.example.com'}) + resp = req.get_response(app) + self.assertEqual(resp.status_int, 301) + self.assertEqual(resp.headers.get('Location'), + 'http://auth-uuid.example.com/cont/') + + req = Request.blank('/cont/test', environ={'REQUEST_METHOD': 'GET'}, + headers={'Host': 'auth-uuid.example.com'}) + resp = req.get_response(app) + self.assertEqual(resp.status_int, 301) + self.assertEqual(resp.headers.get('Location'), + 'http://auth-uuid.example.com/cont/test/') + + req = Request.blank('/test', environ={'REQUEST_METHOD': 'GET'}, + headers={'Host': 'cont.auth-uuid.example.com'}) + resp = req.get_response(app) + self.assertEqual(resp.status_int, 301) + self.assertEqual(resp.headers.get('Location'), + 'http://cont.auth-uuid.example.com/test/') class TestSwiftInfo(unittest.TestCase):