From 6994a2e392be4096beb49bd33e1a507dd04d491e Mon Sep 17 00:00:00 2001 From: Samuel Merritt Date: Wed, 31 Jan 2018 17:05:28 -0800 Subject: [PATCH] ratelimit: ignore requests with invalid API versions If you've got things like domain_remap, swift3, or other such middlewares in your pipeline, you wind up with requests that aren't of the form "/v1///". When encountering such an oddball request, it's not useful to call get_account_info() on the second path component since it's probably not an account. This commit makes the ratelimit middleware skip requests that don't start with either "/v1" or "/v1.0". The requests will still be handled, but they won't be rate-limited. Change-Id: I9980cd0e902610ac99d13a502ae955bca2d99df3 Closes-Bug: 1669888 Closes-Bug: 1695273 --- swift/common/middleware/ratelimit.py | 3 ++ test/unit/common/middleware/test_ratelimit.py | 48 ++++++++++++++----- 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/swift/common/middleware/ratelimit.py b/swift/common/middleware/ratelimit.py index 5f50048a35..c9c863ccb1 100644 --- a/swift/common/middleware/ratelimit.py +++ b/swift/common/middleware/ratelimit.py @@ -19,6 +19,7 @@ import eventlet from swift.common.utils import cache_from_env, get_logger, register_swift_info from swift.proxy.controllers.base import get_account_info, get_container_info +from swift.common.constraints import valid_api_version from swift.common.memcached import MemcacheConnectionError from swift.common.swob import Request, Response @@ -296,6 +297,8 @@ class RateLimitMiddleware(object): version, account, container, obj = req.split_path(1, 4, True) except ValueError: return self.app(env, start_response) + if not valid_api_version(version): + return self.app(env, start_response) ratelimit_resp = self.handle_ratelimit(req, account, container, obj) if ratelimit_resp is None: return self.app(env, start_response) diff --git a/test/unit/common/middleware/test_ratelimit.py b/test/unit/common/middleware/test_ratelimit.py index 3e2a744d67..3389afc836 100644 --- a/test/unit/common/middleware/test_ratelimit.py +++ b/test/unit/common/middleware/test_ratelimit.py @@ -223,7 +223,7 @@ class TestRateLimit(unittest.TestCase): lambda *args, **kwargs: {}): for meth, exp_time in [('DELETE', 9.8), ('GET', 0), ('POST', 0), ('PUT', 9.8)]: - req = Request.blank('/v/a%s/c' % meth) + req = Request.blank('/v1/a%s/c' % meth) req.method = meth req.environ['swift.cache'] = FakeMemcache() make_app_call = lambda: self.test_ratelimit(req.environ, @@ -239,7 +239,7 @@ class TestRateLimit(unittest.TestCase): num_calls = 50 conf_dict = {'account_ratelimit': current_rate} self.test_ratelimit = ratelimit.filter_factory(conf_dict)(FakeApp()) - req = Request.blank('/v/a/c') + req = Request.blank('/v1/a/c') req.method = 'PUT' req.environ['swift.cache'] = FakeMemcache() req.environ['swift.cache'].init_incr_return_neg = True @@ -278,7 +278,7 @@ class TestRateLimit(unittest.TestCase): 'account_whitelist': 'a', 'account_blacklist': 'b'} self.test_ratelimit = ratelimit.filter_factory(conf_dict)(FakeApp()) - req = Request.blank('/v/a/c') + req = Request.blank('/v1/a/c') req.environ['swift.cache'] = FakeMemcache() class rate_caller(threading.Thread): @@ -320,7 +320,7 @@ class TestRateLimit(unittest.TestCase): self.test_ratelimit = ratelimit.filter_factory(conf_dict)(FakeApp()) self.test_ratelimit.logger = FakeLogger() self.test_ratelimit.BLACK_LIST_SLEEP = 0 - req = Request.blank('/v/b/c') + req = Request.blank('/v1/b/c') req.environ['swift.cache'] = FakeMemcache() class rate_caller(threading.Thread): @@ -361,7 +361,7 @@ class TestRateLimit(unittest.TestCase): 'max_sleep_time_seconds': 1} self.test_ratelimit = ratelimit.filter_factory(conf_dict)(FakeApp()) self.test_ratelimit.log_sleep_time_seconds = .00001 - req = Request.blank('/v/a/c') + req = Request.blank('/v1/a/c') req.method = 'PUT' req.environ['swift.cache'] = FakeMemcache() @@ -391,7 +391,7 @@ class TestRateLimit(unittest.TestCase): 'max_sleep_time_seconds': 1} self.test_ratelimit = ratelimit.filter_factory(conf_dict)(FakeApp()) self.test_ratelimit.log_sleep_time_seconds = .00001 - req = Request.blank('/v/a/c/o') + req = Request.blank('/v1/a/c/o') req.method = 'PUT' req.environ['swift.cache'] = FakeMemcache() req.environ['swift.cache'].set( @@ -424,7 +424,7 @@ class TestRateLimit(unittest.TestCase): 'max_sleep_time_seconds': 1} self.test_ratelimit = ratelimit.filter_factory(conf_dict)(FakeApp()) self.test_ratelimit.log_sleep_time_seconds = .00001 - req = Request.blank('/v/a/c') + req = Request.blank('/v1/a/c') req.method = 'GET' req.environ['swift.cache'] = FakeMemcache() req.environ['swift.cache'].set( @@ -511,14 +511,36 @@ class TestRateLimit(unittest.TestCase): def __call__(self, *args, **kwargs): pass resp = rate_mid.__call__(env, a_callable()) - self.assertTrue('fake_app' == resp[0]) + self.assertEqual('fake_app', resp[0]) + + def test_call_non_swift_api_path(self): + env = {'REQUEST_METHOD': 'GET', + 'SCRIPT_NAME': '', + 'PATH_INFO': '/ive/got/a/lovely/bunch/of/coconuts', + 'SERVER_NAME': '127.0.0.1', + 'SERVER_PORT': '80', + 'swift.cache': FakeMemcache(), + 'SERVER_PROTOCOL': 'HTTP/1.0'} + + app = lambda *args, **kwargs: ['some response'] + rate_mid = ratelimit.filter_factory({})(app) + + class a_callable(object): + + def __call__(self, *args, **kwargs): + pass + + with mock.patch('swift.common.middleware.ratelimit.get_account_info', + side_effect=Exception("you shouldn't call this")): + resp = rate_mid(env, a_callable()) + self.assertEqual(resp[0], 'some response') def test_no_memcache(self): current_rate = 13 num_calls = 5 conf_dict = {'account_ratelimit': current_rate} self.test_ratelimit = ratelimit.filter_factory(conf_dict)(FakeApp()) - req = Request.blank('/v/a') + req = Request.blank('/v1/a') req.environ['swift.cache'] = None make_app_call = lambda: self.test_ratelimit(req.environ, start_response) @@ -532,7 +554,7 @@ class TestRateLimit(unittest.TestCase): num_calls = 5 conf_dict = {'account_ratelimit': current_rate} self.test_ratelimit = ratelimit.filter_factory(conf_dict)(FakeApp()) - req = Request.blank('/v/a/c') + req = Request.blank('/v1/a/c') req.method = 'PUT' req.environ['swift.cache'] = FakeMemcache() req.environ['swift.cache'].error_on_incr = True @@ -553,7 +575,7 @@ class TestSwiftInfo(unittest.TestCase): def test_registered_defaults(self): - def check_key_is_absnet(key): + def check_key_is_absent(key): try: swift_info[key] except KeyError as err: @@ -571,7 +593,7 @@ class TestSwiftInfo(unittest.TestCase): ratelimit.filter_factory(test_limits)('have to pass in an app') swift_info = utils.get_swift_info() - self.assertTrue('ratelimit' in swift_info) + self.assertIn('ratelimit', swift_info) self.assertEqual(swift_info['ratelimit'] ['account_ratelimit'], 1.0) self.assertEqual(swift_info['ratelimit'] @@ -605,7 +627,7 @@ class TestSwiftInfo(unittest.TestCase): for key in ['log_sleep_time_seconds', 'clock_accuracy', 'rate_buffer_seconds', 'ratelimit_whitelis', 'ratelimit_blacklist']: - check_key_is_absnet(key) + check_key_is_absent(key) if __name__ == '__main__':