Prevent MemoryError when logging response bodies
Response bodies are loaded into memory prior to being logged. Loading huge response bodies may result in a MemoryError. This patch proposes that only JSON and TEXT responses be logged, i.e when the Content-Type header is application/json or application/text. Responses that do not include or have a different Content-Type header will have their body omitted. Closes-bug: 1616105 Change-Id: I93b6fff73368c4f58bdebf8566c4948b50980cee
This commit is contained in:
parent
014db209d8
commit
f345559a06
|
@ -45,6 +45,8 @@ DEFAULT_USER_AGENT = 'keystoneauth1/%s %s %s/%s' % (
|
|||
keystoneauth1.__version__, requests.utils.default_user_agent(),
|
||||
platform.python_implementation(), platform.python_version())
|
||||
|
||||
_LOG_CONTENT_TYPES = set(['application/json', 'application/text'])
|
||||
|
||||
_logger = utils.get_logger(__name__)
|
||||
|
||||
|
||||
|
@ -354,7 +356,19 @@ class Session(object):
|
|||
if not headers:
|
||||
headers = response.headers
|
||||
if not text:
|
||||
text = self._remove_service_catalog(response.text)
|
||||
# NOTE(samueldmq): If the response does not provide enough info
|
||||
# about the content type to decide whether it is useful and
|
||||
# safe to log it or not, just do not log the body. Trying to
|
||||
# read the response body anyways may result on reading a long
|
||||
# stream of bytes and getting an unexpected MemoryError. See
|
||||
# bug 1616105 for further details.
|
||||
content_type = response.headers.get('content-type', None)
|
||||
if content_type in _LOG_CONTENT_TYPES:
|
||||
text = self._remove_service_catalog(response.text)
|
||||
else:
|
||||
text = ('Omitted, Content-Type is set to %s. Only '
|
||||
'application/json and application/text responses '
|
||||
'have their bodies logged.') % content_type
|
||||
if json:
|
||||
text = self._json.encode(json)
|
||||
|
||||
|
|
|
@ -188,7 +188,8 @@ class SessionTests(utils.TestCase):
|
|||
in order to redact secure headers while debug is true.
|
||||
"""
|
||||
session = client_session.Session(verify=False)
|
||||
headers = {'HEADERA': 'HEADERVALB'}
|
||||
headers = {'HEADERA': 'HEADERVALB',
|
||||
'Content-Type': 'application/text'}
|
||||
security_headers = {'Authorization': uuid.uuid4().hex,
|
||||
'X-Auth-Token': uuid.uuid4().hex,
|
||||
'X-Subject-Token': uuid.uuid4().hex, }
|
||||
|
@ -222,12 +223,56 @@ class SessionTests(utils.TestCase):
|
|||
session = client_session.Session()
|
||||
body = uuid.uuid4().hex
|
||||
|
||||
self.stub_url('GET', text=body, status_code=400)
|
||||
self.stub_url('GET', text=body, status_code=400,
|
||||
headers={'Content-Type': 'application/text'})
|
||||
resp = session.get(self.TEST_URL, raise_exc=False)
|
||||
|
||||
self.assertEqual(resp.status_code, 400)
|
||||
self.assertIn(body, self.logger.output)
|
||||
|
||||
def test_logging_body_only_for_text_and_json_content_types(self):
|
||||
"""Verify response body is only logged in specific content types.
|
||||
|
||||
Response bodies are logged only when the response's Content-Type header
|
||||
is set to application/json or application/text. This prevents us to get
|
||||
an unexpected MemoryError when reading arbitrary responses, such as
|
||||
streams.
|
||||
"""
|
||||
OMITTED_BODY = ('Omitted, Content-Type is set to %s. Only '
|
||||
'application/json and application/text responses '
|
||||
'have their bodies logged.')
|
||||
session = client_session.Session(verify=False)
|
||||
|
||||
# Content-Type is not set
|
||||
body = json.dumps({'token': {'id': '...'}})
|
||||
self.stub_url('POST', text=body)
|
||||
session.post(self.TEST_URL)
|
||||
self.assertNotIn(body, self.logger.output)
|
||||
self.assertIn(OMITTED_BODY % None, self.logger.output)
|
||||
|
||||
# Content-Type is set to text/xml
|
||||
body = '<token><id>...</id></token>'
|
||||
self.stub_url('POST', text=body, headers={'Content-Type': 'text/xml'})
|
||||
session.post(self.TEST_URL)
|
||||
self.assertNotIn(body, self.logger.output)
|
||||
self.assertIn(OMITTED_BODY % 'text/xml', self.logger.output)
|
||||
|
||||
# Content-Type is set to application/json
|
||||
body = json.dumps({'token': {'id': '...'}})
|
||||
self.stub_url('POST', text=body,
|
||||
headers={'Content-Type': 'application/json'})
|
||||
session.post(self.TEST_URL)
|
||||
self.assertIn(body, self.logger.output)
|
||||
self.assertNotIn(OMITTED_BODY % 'application/json', self.logger.output)
|
||||
|
||||
# Content-Type is set to application/text
|
||||
body = uuid.uuid4().hex
|
||||
self.stub_url('POST', text=body,
|
||||
headers={'Content-Type': 'application/text'})
|
||||
session.post(self.TEST_URL)
|
||||
self.assertIn(body, self.logger.output)
|
||||
self.assertNotIn(OMITTED_BODY % 'application/text', self.logger.output)
|
||||
|
||||
def test_logging_cacerts(self):
|
||||
path_to_certs = '/path/to/certs'
|
||||
session = client_session.Session(verify=path_to_certs)
|
||||
|
@ -760,7 +805,7 @@ class SessionAuthTests(utils.TestCase):
|
|||
|
||||
self.stub_url('GET',
|
||||
text=response,
|
||||
headers={'Content-Type': 'text/html'})
|
||||
headers={'Content-Type': 'application/text'})
|
||||
|
||||
resp = sess.get(self.TEST_URL, logger=logger)
|
||||
|
||||
|
@ -953,7 +998,7 @@ class AdapterTest(utils.TestCase):
|
|||
response = uuid.uuid4().hex
|
||||
|
||||
self.stub_url('GET', text=response,
|
||||
headers={'Content-Type': 'text/html'})
|
||||
headers={'Content-Type': 'application/text'})
|
||||
|
||||
resp = adpt.get(self.TEST_URL, logger=logger)
|
||||
|
||||
|
|
|
@ -0,0 +1,8 @@
|
|||
---
|
||||
fixes:
|
||||
- >
|
||||
[`bug 1616105 <https://bugs.launchpad.net/keystoneauth/+bug/1616105>`_]
|
||||
Only log the response body when the ``Content-Type`` header is set to
|
||||
``application/json`` or ``application/text``. This avoids logging large
|
||||
binary objects (such as images). Other ``Content-Type`` will not be
|
||||
logged.
|
Loading…
Reference in New Issue