From b65d8b10c5ea1c897bcb02c97943af82fd27d578 Mon Sep 17 00:00:00 2001
From: Tim Burke <tim.burke@gmail.com>
Date: Sat, 14 Dec 2019 01:15:06 +0000
Subject: [PATCH] Early-return on non-Swift get_info requests

Change-Id: Iadc61a1c3bcbfbc47f65ec65df36d8da3694ee74
---
 swift/proxy/controllers/base.py          | 10 ++++
 test/unit/proxy/controllers/test_base.py | 58 ++++++++++++++++++++++++
 2 files changed, 68 insertions(+)

diff --git a/swift/proxy/controllers/base.py b/swift/proxy/controllers/base.py
index 1d25a95d9f..98721755b8 100644
--- a/swift/proxy/controllers/base.py
+++ b/swift/proxy/controllers/base.py
@@ -331,6 +331,12 @@ def get_container_info(env, app, swift_source=None):
     """
     (version, wsgi_account, wsgi_container, unused) = \
         split_path(env['PATH_INFO'], 3, 4, True)
+
+    if not constraints.valid_api_version(version):
+        # Not a valid Swift request; return 0 like we do
+        # if there's an account failure
+        return headers_to_container_info({}, 0)
+
     account = wsgi_to_str(wsgi_account)
     container = wsgi_to_str(wsgi_container)
 
@@ -404,6 +410,10 @@ def get_account_info(env, app, swift_source=None):
     :raises ValueError: when path doesn't contain an account
     """
     (version, wsgi_account, _junk) = split_path(env['PATH_INFO'], 2, 3, True)
+
+    if not constraints.valid_api_version(version):
+        return headers_to_account_info({}, 0)
+
     account = wsgi_to_str(wsgi_account)
 
     # Check in environment cache and in memcache (in that order)
diff --git a/test/unit/proxy/controllers/test_base.py b/test/unit/proxy/controllers/test_base.py
index e636980f78..2f97380bc9 100644
--- a/test/unit/proxy/controllers/test_base.py
+++ b/test/unit/proxy/controllers/test_base.py
@@ -367,6 +367,47 @@ class TestFuncs(unittest.TestCase):
         info = get_container_info(req.environ, app)
         self.assertEqual(info['status'], 0)
 
+    def test_get_container_info_no_container_gets_cached(self):
+        fake_cache = FakeCache({})
+        app = FakeApp(statuses=[200, 404])
+        req = Request.blank("/v1/AUTH_account/does_not_exist",
+                            environ={'swift.cache': fake_cache})
+        info = get_container_info(req.environ, app)
+        self.assertEqual(info['status'], 404)
+        key = get_cache_key("AUTH_account", "does_not_exist")
+        self.assertIn(key, fake_cache.store)
+        self.assertEqual(fake_cache.store[key]['status'], 404)
+
+    def test_get_container_info_bad_path(self):
+        fake_cache = FakeCache({})
+        req = Request.blank("/non-swift/AUTH_account/does_not_exist",
+                            environ={'swift.cache': fake_cache})
+        info = get_container_info(req.environ, FakeApp(statuses=[400]))
+        self.assertEqual(info['status'], 0)
+        # *not* cached
+        key = get_cache_key("AUTH_account", "does_not_exist")
+        self.assertNotIn(key, fake_cache.store)
+        # not even the "account" is cached
+        key = get_cache_key("AUTH_account")
+        self.assertNotIn(key, fake_cache.store)
+
+        # but if for some reason the account *already was* cached...
+        fake_cache.store[key] = headers_to_account_info({}, 200)
+        req = Request.blank("/non-swift/AUTH_account/does_not_exist",
+                            environ={'swift.cache': fake_cache})
+        info = get_container_info(req.environ, FakeApp(statuses=[400]))
+        self.assertEqual(info['status'], 0)
+        # resp *still* not cached
+        key = get_cache_key("AUTH_account", "does_not_exist")
+        self.assertNotIn(key, fake_cache.store)
+
+        # still nothing, even if the container is already cached, too
+        fake_cache.store[key] = headers_to_container_info({}, 200)
+        req = Request.blank("/non-swift/AUTH_account/does_not_exist",
+                            environ={'swift.cache': fake_cache})
+        info = get_container_info(req.environ, FakeApp(statuses=[400]))
+        self.assertEqual(info['status'], 0)
+
     def test_get_container_info_no_auto_account(self):
         app = FakeApp(statuses=[200])
         req = Request.blank("/v1/.system_account/cont")
@@ -498,6 +539,23 @@ class TestFuncs(unittest.TestCase):
         resp = get_account_info(req.environ, 'xxx')
         self.assertEqual(resp['bytes'], 3867)
 
+    def test_get_account_info_bad_path(self):
+        fake_cache = FakeCache({})
+        req = Request.blank("/non-swift/AUTH_account",
+                            environ={'swift.cache': fake_cache})
+        info = get_account_info(req.environ, FakeApp(statuses=[400]))
+        self.assertEqual(info['status'], 0)
+        # *not* cached
+        key = get_cache_key("AUTH_account")
+        self.assertNotIn(key, fake_cache.store)
+
+        # but if for some reason the account *already was* cached...
+        fake_cache.store[key] = headers_to_account_info({}, 200)
+        req = Request.blank("/non-swift/AUTH_account/does_not_exist",
+                            environ={'swift.cache': fake_cache})
+        info = get_account_info(req.environ, FakeApp(statuses=[400]))
+        self.assertEqual(info['status'], 0)
+
     def test_get_object_info_env(self):
         cached = {'status': 200,
                   'length': 3333,