diff --git a/keystoneclient/session.py b/keystoneclient/session.py
index 522a53366..98fe42a8f 100644
--- a/keystoneclient/session.py
+++ b/keystoneclient/session.py
@@ -37,6 +37,8 @@ osprofiler_web = importutils.try_import("osprofiler.web")
USER_AGENT = 'python-keystoneclient'
+_LOG_CONTENT_TYPES = set(['application/json', 'application/text'])
+
_logger = logging.getLogger(__name__)
@@ -216,7 +218,18 @@ class Session(object):
if not logger.isEnabledFor(logging.DEBUG):
return
- text = _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 = _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
string_parts = [
'RESP:',
@@ -224,9 +237,7 @@ class Session(object):
]
for header in six.iteritems(response.headers):
string_parts.append('%s: %s' % self._process_header(header))
- if text:
- string_parts.append('\nRESP BODY: %s\n' %
- strutils.mask_password(text))
+ string_parts.append('\nRESP BODY: %s\n' % strutils.mask_password(text))
logger.debug(' '.join(string_parts))
diff --git a/keystoneclient/tests/unit/test_http.py b/keystoneclient/tests/unit/test_http.py
index 56f116c5a..6e0070aaf 100644
--- a/keystoneclient/tests/unit/test_http.py
+++ b/keystoneclient/tests/unit/test_http.py
@@ -166,22 +166,24 @@ class BasicRequestTests(utils.TestCase):
self.addCleanup(self.logger.setLevel, level)
def request(self, method='GET', response='Test Response', status_code=200,
- url=None, **kwargs):
+ url=None, headers={}, **kwargs):
if not url:
url = self.url
self.requests_mock.register_uri(method, url, text=response,
- status_code=status_code)
+ status_code=status_code,
+ headers=headers)
with self.deprecations.expect_deprecations_here():
- return httpclient.request(url, method, **kwargs)
+ return httpclient.request(url, method, headers=headers, **kwargs)
def test_basic_params(self):
method = 'GET'
response = 'Test Response'
status = 200
- self.request(method=method, status_code=status, response=response)
+ self.request(method=method, status_code=status, response=response,
+ headers={'Content-Type': 'application/json'})
self.assertEqual(self.requests_mock.last_request.method, method)
@@ -209,7 +211,8 @@ class BasicRequestTests(utils.TestCase):
def test_body(self):
data = "BODY DATA"
- self.request(response=data)
+ self.request(response=data,
+ headers={'Content-Type': 'application/json'})
logger_message = self.logger_message.getvalue()
self.assertThat(logger_message, matchers.Contains('BODY:'))
self.assertThat(logger_message, matchers.Contains(data))
diff --git a/keystoneclient/tests/unit/test_session.py b/keystoneclient/tests/unit/test_session.py
index 8fb364ac0..168cbb76e 100644
--- a/keystoneclient/tests/unit/test_session.py
+++ b/keystoneclient/tests/unit/test_session.py
@@ -149,7 +149,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/json'}
security_headers = {'Authorization': uuid.uuid4().hex,
'X-Auth-Token': uuid.uuid4().hex,
'X-Subject-Token': uuid.uuid4().hex, }
@@ -183,12 +184,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 = jsonutils.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 = '...'
+ 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 = jsonutils.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_unicode_data_in_debug_output(self):
"""Verify that ascii-encodable data is logged without modification."""
session = client_session.Session(verify=False)
@@ -315,7 +360,8 @@ class SessionTests(utils.TestCase):
"auth_username": "verybadusername",
"auth_method": "CHAP"}}}
body_json = jsonutils.dumps(body)
- response = mock.Mock(text=body_json, status_code=200, headers={})
+ response = mock.Mock(text=body_json, status_code=200,
+ headers={'content-type': 'application/json'})
session._http_log_response(response, logger)
self.assertEqual(1, logger.debug.call_count)
@@ -772,7 +818,7 @@ class SessionAuthTests(utils.TestCase):
self.stub_url('GET',
text=response,
- headers={'Content-Type': 'text/html'})
+ headers={'Content-Type': 'application/json'})
resp = sess.get(self.TEST_URL, logger=logger)
@@ -968,7 +1014,7 @@ class AdapterTest(utils.TestCase):
response = uuid.uuid4().hex
self.stub_url('GET', text=response,
- headers={'Content-Type': 'text/html'})
+ headers={'Content-Type': 'application/json'})
resp = adpt.get(self.TEST_URL, logger=logger)
diff --git a/releasenotes/notes/bug-1616105-cc8b85eb056e99e2.yaml b/releasenotes/notes/bug-1616105-cc8b85eb056e99e2.yaml
new file mode 100644
index 000000000..91529c622
--- /dev/null
+++ b/releasenotes/notes/bug-1616105-cc8b85eb056e99e2.yaml
@@ -0,0 +1,8 @@
+---
+fixes:
+ - >
+ [`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.