openstack/swift
Code Issues Proposed changes

proxy controller: always pass x-backend-* headers to backend

Browse Source 
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
This commit is contained in:
Alistair Coles 2023-04-03 16:23:41 +01:00
parent 629516cbe7
commit e19a165a7b
2 changed files with 75 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
swift/proxy/controllers/base.py
View File

@ -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

68
test/unit/proxy/controllers/test_base.py
View File

@ -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, transfer=True)
expected_headers = {'x-base-meta-owner': '',
'x-base-meta-size': '151M',
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])
self.assertNotIn('new-owner', dst_headers)
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.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])
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():