From e19a165a7b56cd88c4fef6c72418f103140c94ae Mon Sep 17 00:00:00 2001 From: Alistair Coles Date: Mon, 3 Apr 2023 16:23:41 +0100 Subject: proxy controller: always pass x-backend-* headers to backend X-Backend-* headers were previously passed to the backend server with only a subset of all request types: * all object requests * container GET, HEAD * account GET, HEAD In these cases, X-Backend-* headers were transferred to backend requests implicitly as a consequence of *all* the headers in the request that the proxy is handling being copied to the backend request. With this change, X-Backend-* headers are explicitly copied from the request that the proxy is handling to the backend request, for every request type. Note: X-Backend-* headers are typically added to a request by the proxy app or middleware, prior to creating a backend request. X-Backend-* headers are removed from client requests by the gatekeeper middleware, so clients cannot send X-Backend-* headers to backend servers. An exception is an InternalClient that does not have gatekeeper middleware, deliberately so that internal daemons such as the sharder can send X-Backend-* headers to the backend servers. Also, BaseController.generate_request_headers() is fixed to prevent accessing a None type when transfer is True but the orig_req is None. Change-Id: I05fb9a3e1c98d96bbe01da2ee28474e0f57297e6 --- swift/proxy/controllers/base.py | 18 ++++++--- test/unit/proxy/controllers/test_base.py | 66 ++++++++++++++++++++++++++++++-- 2 files changed, 74 insertions(+), 10 deletions(-) diff --git a/swift/proxy/controllers/base.py b/swift/proxy/controllers/base.py index 758aed72b..463fb5aed 100644 --- a/swift/proxy/controllers/base.py +++ b/swift/proxy/controllers/base.py @@ -1848,16 +1848,22 @@ class Controller(object): :param transfer: If True, transfer headers from original client request :returns: a dictionary of headers """ - # Use the additional headers first so they don't overwrite the headers - # we require. - headers = HeaderKeyDict(additional) if additional else HeaderKeyDict() - if transfer: - self.transfer_headers(orig_req.headers, headers) - headers.setdefault('x-timestamp', Timestamp.now().internal) + headers = HeaderKeyDict() if orig_req: + headers.update((k.lower(), v) + for k, v in orig_req.headers.items() + if k.lower().startswith('x-backend-')) referer = orig_req.as_referer() else: referer = '' + # additional headers can override x-backend-* headers from orig_req + if additional: + headers.update(additional) + if orig_req and transfer: + # transfer headers from orig_req can override additional headers + self.transfer_headers(orig_req.headers, headers) + headers.setdefault('x-timestamp', Timestamp.now().internal) + # orig_req and additional headers cannot override the following... headers['x-trans-id'] = self.trans_id headers['connection'] = 'close' headers['user-agent'] = self.app.backend_user_agent diff --git a/test/unit/proxy/controllers/test_base.py b/test/unit/proxy/controllers/test_base.py index 73d61c6ef..a84dd53c2 100644 --- a/test/unit/proxy/controllers/test_base.py +++ b/test/unit/proxy/controllers/test_base.py @@ -1155,17 +1155,74 @@ class TestFuncs(BaseTest): base = Controller(self.app) src_headers = {'x-remove-base-meta-owner': 'x', 'x-base-meta-size': '151M', + 'x-base-sysmeta-mysysmeta': 'myvalue', + 'x-Backend-No-Timestamp-Update': 'true', + 'X-Backend-Storage-Policy-Index': '3', + 'x-backendoftheworld': 'ignored', 'new-owner': 'Kun'} req = Request.blank('/v1/a/c/o', headers=src_headers) + dst_headers = base.generate_request_headers(req) + expected_headers = {'x-backend-no-timestamp-update': 'true', + 'x-backend-storage-policy-index': '3', + 'x-timestamp': mock.ANY, + 'x-trans-id': '-', + 'Referer': 'GET http://localhost/v1/a/c/o', + 'connection': 'close', + 'user-agent': 'proxy-server %d' % os.getpid()} + for k, v in expected_headers.items(): + self.assertIn(k, dst_headers) + self.assertEqual(v, dst_headers[k]) + for k, v in expected_headers.items(): + dst_headers.pop(k) + self.assertFalse(dst_headers) + + # with transfer=True + req = Request.blank('/v1/a/c/o', headers=src_headers) dst_headers = base.generate_request_headers(req, transfer=True) - expected_headers = {'x-base-meta-owner': '', - 'x-base-meta-size': '151M', + expected_headers.update({'x-base-meta-owner': '', + 'x-base-meta-size': '151M', + 'x-base-sysmeta-mysysmeta': 'myvalue'}) + for k, v in expected_headers.items(): + self.assertIn(k, dst_headers) + self.assertEqual(v, dst_headers[k]) + for k, v in expected_headers.items(): + dst_headers.pop(k) + self.assertFalse(dst_headers) + + # with additional + req = Request.blank('/v1/a/c/o', headers=src_headers) + dst_headers = base.generate_request_headers( + req, transfer=True, + additional=src_headers) + expected_headers.update({'x-remove-base-meta-owner': 'x', + 'x-backendoftheworld': 'ignored', + 'new-owner': 'Kun'}) + for k, v in expected_headers.items(): + self.assertIn(k, dst_headers) + self.assertEqual(v, dst_headers[k]) + for k, v in expected_headers.items(): + dst_headers.pop(k) + self.assertFalse(dst_headers) + + # with additional, verify precedence + req = Request.blank('/v1/a/c/o', headers=src_headers) + dst_headers = base.generate_request_headers( + req, transfer=False, + additional={'X-Backend-Storage-Policy-Index': '2', + 'X-Timestamp': '1234.56789'}) + expected_headers = {'x-backend-no-timestamp-update': 'true', + 'x-backend-storage-policy-index': '2', + 'x-timestamp': '1234.56789', + 'x-trans-id': '-', + 'Referer': 'GET http://localhost/v1/a/c/o', 'connection': 'close', 'user-agent': 'proxy-server %d' % os.getpid()} for k, v in expected_headers.items(): self.assertIn(k, dst_headers) self.assertEqual(v, dst_headers[k]) - self.assertNotIn('new-owner', dst_headers) + for k, v in expected_headers.items(): + dst_headers.pop(k) + self.assertFalse(dst_headers) def test_generate_request_headers_change_backend_user_agent(self): base = Controller(self.app) @@ -1205,7 +1262,8 @@ class TestFuncs(BaseTest): 'x-base-meta-size': '151M', 'new-owner': 'Kun'} dst_headers = base.generate_request_headers(None, - additional=src_headers) + additional=src_headers, + transfer=True) expected_headers = {'x-base-meta-size': '151M', 'connection': 'close'} for k, v in expected_headers.items(): -- cgit v1.2.1