From 7109698192dc067c39a5b1031065a6af7056590d Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Tue, 5 Oct 2021 17:06:02 +0200 Subject: [PATCH] misc: log bytes as string without enclosing b"" (#57253) --- passerelle/base/models.py | 8 ++++++++ passerelle/utils/__init__.py | 4 ++-- passerelle/utils/jsonresponse.py | 2 +- tests/test_proxylogger.py | 30 ++++++++++++++++++++---------- tests/test_requests.py | 14 +++++++------- 5 files changed, 38 insertions(+), 20 deletions(-) diff --git a/passerelle/base/models.py b/passerelle/base/models.py index e86c1211..09b823a4 100644 --- a/passerelle/base/models.py +++ b/passerelle/base/models.py @@ -969,6 +969,14 @@ class ProxyLogger(object): extra.update(kwargs.get('extra', {})) request = extra.get('request') + for k, v in extra.items(): + if isinstance(v, bytes): + try: + extra[k] = v.decode('utf-8') + except UnicodeDecodeError: + # convert bytes to string representation without enclosing b"" + extra[k] = repr(v)[2:-1] + def is_json_serializable(value): return isinstance(value, (list, dict, bool) + six.integer_types + six.string_types) diff --git a/passerelle/utils/__init__.py b/passerelle/utils/__init__.py index 692c9efa..39a4a9d6 100644 --- a/passerelle/utils/__init__.py +++ b/passerelle/utils/__init__.py @@ -199,7 +199,7 @@ def log_http_request(logger, request, response=None, exception=None, error_log=T max_size = settings.LOGGED_REQUESTS_MAX_SIZE if hasattr(logger, 'connector'): max_size = logger.connector.logging_parameters.requests_max_size or max_size - extra['request_payload'] = repr(request.body[:max_size]) + extra['request_payload'] = request.body[:max_size] if response is not None: message = message + ' (=> %s)' % response.status_code extra['response_status'] = response.status_code @@ -211,7 +211,7 @@ def log_http_request(logger, request, response=None, exception=None, error_log=T if hasattr(logger, 'connector'): max_size = logger.connector.logging_parameters.responses_max_size or max_size content = response.content[:max_size] - extra['response_content'] = repr(content) + extra['response_content'] = content if response.status_code // 100 == 3: log_function = logger.warning elif response.status_code // 100 >= 4: diff --git a/passerelle/utils/jsonresponse.py b/passerelle/utils/jsonresponse.py index 65f41918..cca0cf94 100644 --- a/passerelle/utils/jsonresponse.py +++ b/passerelle/utils/jsonresponse.py @@ -140,7 +140,7 @@ class to_json(object): max_size = settings.LOGGED_REQUESTS_MAX_SIZE if hasattr(logger, 'connector'): max_size = logger.connector.logging_parameters.requests_max_size or max_size - extras.update({'body': repr(req.body[:max_size])}) + extras.update({'body': req.body[:max_size]}) if isinstance(e, ObjectDoesNotExist): logger.warning('object not found: %r', e, extra=extras) diff --git a/tests/test_proxylogger.py b/tests/test_proxylogger.py index d458aa8f..ae51fe33 100644 --- a/tests/test_proxylogger.py +++ b/tests/test_proxylogger.py @@ -318,7 +318,7 @@ def test_logged_requests_and_responses_max_size(app, db, monkeypatch, settings): if PY2: assert ResourceLog.objects.all()[2].extra['body'] == "'user_query_var=11111111111111111111'" else: - assert ResourceLog.objects.all()[2].extra['body'] == "b'user_query_var=11111111111111111111'" + assert ResourceLog.objects.all()[2].extra['body'] == 'user_query_var=11111111111111111111' assert ( ResourceLog.objects.all()[2].extra['exception'] == "{'connector_error_var': '44444444444444444444'}" ) @@ -332,19 +332,17 @@ def test_logged_requests_and_responses_max_size(app, db, monkeypatch, settings): assert len(ResourceLog.objects.all()) == 3 # - connector POST queries - assert ( - ResourceLog.objects.all()[1].extra['request_payload'] == "'connector_query_var=22222222222222222222'" - ) + assert ResourceLog.objects.all()[1].extra['request_payload'] == "connector_query_var=22222222222222222222" assert ResourceLog.objects.all()[1].extra.get('response_headers') == {'Content-Type': 'foo/bar'} if PY2: assert ( ResourceLog.objects.all()[1].extra.get('response_content') - == '\'{"service_reply_var": "33333333333333333333"}\'' + == '{"service_reply_var": "33333333333333333333"}' ) else: assert ( ResourceLog.objects.all()[1].extra.get('response_content') - == 'b\'{"service_reply_var": "33333333333333333333"}\'' + == '{"service_reply_var": "33333333333333333333"}' ) # log troncated payloads @@ -360,13 +358,13 @@ def test_logged_requests_and_responses_max_size(app, db, monkeypatch, settings): assert ResourceLog.objects.all()[0].extra['connector_payload'] == 'user_query_var=1111111111' # - connector POST queries - assert ResourceLog.objects.all()[1].extra['request_payload'] == "'connector_query_var=22222'" + assert ResourceLog.objects.all()[1].extra['request_payload'] == 'connector_query_var=22222' # - connector error if PY2: assert ResourceLog.objects.all()[2].extra['body'] == "'user_query_var=1111111111'" else: - assert ResourceLog.objects.all()[2].extra['body'] == "b'user_query_var=1111111111'" + assert ResourceLog.objects.all()[2].extra['body'] == 'user_query_var=1111111111' # log troncated service response parameters = connector.logging_parameters @@ -379,9 +377,9 @@ def test_logged_requests_and_responses_max_size(app, db, monkeypatch, settings): # - connector POST queries if PY2: - assert ResourceLog.objects.all()[1].extra.get('response_content') == '\'{"service_reply_var": "33\'' + assert ResourceLog.objects.all()[1].extra.get('response_content') == '{"service_reply_var": "33' else: - assert ResourceLog.objects.all()[1].extra.get('response_content') == 'b\'{"service_reply_var": "33\'' + assert ResourceLog.objects.all()[1].extra.get('response_content') == '{"service_reply_var": "33' def test_proxy_logger_email_traceback(app, db, email_handler, settings, mailoutbox, connector, monkeypatch): @@ -395,3 +393,15 @@ def test_proxy_logger_email_traceback(app, db, email_handler, settings, mailoutb monkeypatch.setattr(Feed, 'json', json) resp = app.get(endpoint_url, status=500) assert any('Traceback:' in mail.body for mail in mailoutbox) + + +def test_proxy_logger_bytes(db, connector): + base_logger = ProxyLogger(connector) + base_logger.debug('test', extra={'payload': b'bytes'}) + log = ResourceLog.objects.latest('id') + assert log.extra == {'payload': 'bytes'} + + base_logger = ProxyLogger(connector) + base_logger.debug('test', extra={'payload': b'\xff\xff'}) + log = ResourceLog.objects.latest('id') + assert log.extra == {'payload': '\\xff\\xff'} diff --git a/tests/test_requests.py b/tests/test_requests.py index 5cf30fbf..c26ba9a4 100644 --- a/tests/test_requests.py +++ b/tests/test_requests.py @@ -71,8 +71,8 @@ def test_log_level(caplog, log_level): assert record.request_url == url assert record.response_status == response.status_code if logger.level == 10: # DEBUG - assert record.request_payload.replace('b', '') == '\'{"name": "josh"}\'' - assert record.response_content == repr(response.content) + assert record.request_payload == b'{"name": "josh"}' + assert record.response_content == response.content assert record.response_headers else: assert not hasattr(record, 'request_payload') @@ -103,8 +103,8 @@ def test_log_error(caplog, log_level): assert record.request_url == url assert record.response_status == response.status_code if logger.level == 10: # DEBUG - assert record.request_payload.replace('b', '') == '\'{"name": "josh"}\'' - assert record.response_content == repr(response.content) + assert record.request_payload == b'{"name": "josh"}' + assert record.response_content == response.content assert record.response_headers else: assert not hasattr(record, 'request_payload') @@ -128,8 +128,8 @@ def test_log_error_http_max_sizes(caplog, log_level, settings): if logger.level == 10: # DEBUG records = [record for record in caplog.records if record.name == 'requests'] - assert records[0].request_payload.replace('b', '') == '\'{"name":\'' - assert records[0].response_content.replace('b', '') == '\'{"foo":\'' + assert records[0].request_payload == b'{"name":' + assert records[0].response_content == b'{"foo":' @pytest.fixture(params=['xml', 'whatever', 'jpeg', 'pdf']) @@ -179,7 +179,7 @@ def test_skip_content_type(mocked_get, caplog, endpoint_response): if 'xml' in endpoint_response.headers.get('Content-Type'): assert len(records) == 1 - assert records[0].response_content == "'xml test'" + assert records[0].response_content == 'xml test' else: assert len(records) == 1 -- 2.30.2