Only log application/json in session to start
When whitelisting content types to debug print from session we chose application/json and application/text. application/text is not a real mime type, text is typically text/plain. Rather than guess at mime types only print application/json to start with, but make it easy for additional types to be added later. Change-Id: Ica5fee076cdab8b1d5167161d28af7313fad9477 Related-Bug: 1616105
This commit is contained in:
Jamie Lennox
Steve Martinelli
parent
9d3ae3ef94
commit
d73fd3ee84
@ -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)
|
||||
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -3,6 +3,6 @@ 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.
|
||||
``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.
|
||||
|
||||
Loading…
Reference in New Issue
Block a user