diff --git a/keystoneauth1/session.py b/keystoneauth1/session.py index a69e8f55..40550b49 100644 --- a/keystoneauth1/session.py +++ b/keystoneauth1/session.py @@ -45,7 +45,10 @@ 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']) +# NOTE(jamielennox): Clients will likely want to print more than json. Please +# propose a patch if you have a content type you think is reasonable to print +# here and we'll add it to the list as required. +_LOG_CONTENT_TYPES = set(['application/json']) _logger = utils.get_logger(__name__) @@ -367,8 +370,8 @@ class Session(object): 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 + '%s responses have their bodies logged.') + text = text % (content_type, ', '.join(_LOG_CONTENT_TYPES)) if json: text = self._json.encode(json) diff --git a/keystoneauth1/tests/unit/test_session.py b/keystoneauth1/tests/unit/test_session.py index 46c55bf3..ee2049c9 100644 --- a/keystoneauth1/tests/unit/test_session.py +++ b/keystoneauth1/tests/unit/test_session.py @@ -189,13 +189,13 @@ class SessionTests(utils.TestCase): """ session = client_session.Session(verify=False) headers = {'HEADERA': 'HEADERVALB', - 'Content-Type': 'application/text'} + 'Content-Type': 'application/json'} security_headers = {'Authorization': uuid.uuid4().hex, 'X-Auth-Token': uuid.uuid4().hex, 'X-Subject-Token': uuid.uuid4().hex, 'X-Service-Token': uuid.uuid4().hex} - body = 'BODYRESPONSE' - data = 'BODYDATA' + body = '{"a": "b"}' + data = '{"c": "d"}' all_headers = dict( itertools.chain(headers.items(), security_headers.items())) self.stub_url('POST', text=body, headers=all_headers) @@ -222,26 +222,25 @@ class SessionTests(utils.TestCase): def test_logs_failed_output(self): """Test that output is logged even for failed requests.""" session = client_session.Session() - body = uuid.uuid4().hex + body = {uuid.uuid4().hex: uuid.uuid4().hex} - self.stub_url('GET', text=body, status_code=400, - headers={'Content-Type': 'application/text'}) + self.stub_url('GET', json=body, status_code=400, + headers={'Content-Type': 'application/json'}) resp = session.get(self.TEST_URL, raise_exc=False) self.assertEqual(resp.status_code, 400) - self.assertIn(body, self.logger.output) + self.assertIn(list(body.keys())[0], self.logger.output) + self.assertIn(list(body.values())[0], self.logger.output) - def test_logging_body_only_for_text_and_json_content_types(self): + def test_logging_body_only_for_specified_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. + is set to application/json. 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.') + 'application/json responses have their bodies logged.') session = client_session.Session(verify=False) # Content-Type is not set @@ -266,14 +265,6 @@ class SessionTests(utils.TestCase): 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) @@ -802,22 +793,24 @@ class SessionAuthTests(utils.TestCase): auth = AuthPlugin() sess = client_session.Session(auth=auth) - response = uuid.uuid4().hex + response = {uuid.uuid4().hex: uuid.uuid4().hex} self.stub_url('GET', - text=response, - headers={'Content-Type': 'application/text'}) + json=response, + headers={'Content-Type': 'application/json'}) resp = sess.get(self.TEST_URL, logger=logger) - self.assertEqual(response, resp.text) + self.assertEqual(response, resp.json()) output = io.getvalue() self.assertIn(self.TEST_URL, output) - self.assertIn(response, output) + self.assertIn(list(response.keys())[0], output) + self.assertIn(list(response.values())[0], output) self.assertNotIn(self.TEST_URL, self.logger.output) - self.assertNotIn(response, self.logger.output) + self.assertNotIn(list(response.keys())[0], self.logger.output) + self.assertNotIn(list(response.values())[0], self.logger.output) class AdapterTest(utils.TestCase): @@ -996,21 +989,23 @@ class AdapterTest(utils.TestCase): sess = client_session.Session(auth=auth) adpt = adapter.Adapter(sess, auth=auth, logger=logger) - response = uuid.uuid4().hex + response = {uuid.uuid4().hex: uuid.uuid4().hex} - self.stub_url('GET', text=response, - headers={'Content-Type': 'application/text'}) + self.stub_url('GET', json=response, + headers={'Content-Type': 'application/json'}) resp = adpt.get(self.TEST_URL, logger=logger) - self.assertEqual(response, resp.text) + self.assertEqual(response, resp.json()) output = io.getvalue() self.assertIn(self.TEST_URL, output) - self.assertIn(response, output) + self.assertIn(list(response.keys())[0], output) + self.assertIn(list(response.values())[0], output) self.assertNotIn(self.TEST_URL, self.logger.output) - self.assertNotIn(response, self.logger.output) + self.assertNotIn(list(response.keys())[0], self.logger.output) + self.assertNotIn(list(response.values())[0], self.logger.output) def test_unknown_connection_error(self): self.stub_url('GET', exc=requests.exceptions.RequestException) diff --git a/releasenotes/notes/bug-1616105-cc8b85eb056e99e2.yaml b/releasenotes/notes/bug-1616105-cc8b85eb056e99e2.yaml index 91529c62..e9c1c9c3 100644 --- a/releasenotes/notes/bug-1616105-cc8b85eb056e99e2.yaml +++ b/releasenotes/notes/bug-1616105-cc8b85eb056e99e2.yaml @@ -3,6 +3,6 @@ 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. + ``application/json``. This avoids logging large binary objects (such as + images). Other ``Content-Type`` will not be logged. Additional + ``Content-Type`` strings can be added as required.