From 8885373fb7d4a83542ac8a31acdd544a97aeac01 Mon Sep 17 00:00:00 2001
From: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date: Fri, 22 May 2020 14:10:54 +0200
Subject: [PATCH] misc: log HTTP response headers safely (#43201)

---
 passerelle/utils/__init__.py | 14 ++++++++++++--
 tests/test_requests.py       | 33 ++++++++++++++++++++++++++++++++-
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/passerelle/utils/__init__.py b/passerelle/utils/__init__.py
index ad6add67..c660f2ac 100644
--- a/passerelle/utils/__init__.py
+++ b/passerelle/utils/__init__.py
@@ -168,15 +168,25 @@ def content_type_match(ctype):
     return False
 
 
+def make_headers_safe(headers):
+    '''Convert dict of HTTP headers to text safely, as some services returns 8-bits encoding in headers.
+    '''
+    return {
+        force_text(key, errors='replace'): force_text(value, errors='replace')
+        for key, value in headers.items()
+    }
+
+
 def log_http_request(logger, request, response=None, exception=None, error_log=True, extra=None):
     log_function = logger.info
     message = ''
     extra = extra or {}
+
     if request is not None:
         message = '%s %s' % (request.method, request.url)
         extra['request_url'] = request.url
     if logger.level == 10 and request:  # DEBUG
-        extra['request_headers'] = dict(request.headers.items())
+        extra['request_headers'] = make_headers_safe(request.headers)
         if request.body:
             if hasattr(logger, 'connector'):
                 max_size = logger.connector.logging_parameters.requests_max_size
@@ -187,7 +197,7 @@ def log_http_request(logger, request, response=None, exception=None, error_log=T
         message = message + ' (=> %s)' % response.status_code
         extra['response_status'] = response.status_code
         if logger.level == 10:  # DEBUG
-            extra['response_headers'] = dict(response.headers.items())
+            extra['response_headers'] = make_headers_safe(response.headers)
             # log body only if content type is allowed
             if content_type_match(response.headers.get('Content-Type')):
                 if hasattr(logger, 'connector'):
diff --git a/tests/test_requests.py b/tests/test_requests.py
index 4dbcbd72..88058de8 100644
--- a/tests/test_requests.py
+++ b/tests/test_requests.py
@@ -3,16 +3,19 @@ import logging
 import pytest
 import mohawk
 import mock
+import requests
+
 from httmock import urlmatch, HTTMock, response
 
 from django.test import override_settings
 
-from passerelle.utils import Request, CaseInsensitiveDict
+from passerelle.utils import Request, CaseInsensitiveDict, log_http_request
 from passerelle.utils.http_authenticators import HawkAuth
 import utils
 from utils import FakedResponse
 
 
+
 class MockFileField(object):
     def __init__(self, path):
         self.path = path
@@ -383,3 +386,31 @@ def test_timeout(mocked_get, caplog, endpoint_response):
         assert mocked_get.call_args[1]['timeout'] == 42
         Request(logger=logger).get('http://example.net/whatever', timeout=None)
         assert mocked_get.call_args[1]['timeout'] is None
+
+
+def test_log_http_request(caplog):
+    @urlmatch()
+    def bad_headers(url, request):
+        return response(200, 'coin',
+                        headers={'Error Webservice': b'\xe9'},
+                        request=request)
+    with HTTMock(bad_headers):
+        resp = requests.get('https://example.com/')
+    caplog.set_level(logging.DEBUG)
+    assert len(caplog.records) == 0
+    log_http_request(logging.getLogger(), resp.request, resp)
+    assert len(caplog.records) == 1
+    extra = {key: value for key, value in caplog.records[0].__dict__.items() if key.startswith(('request_', 'response_'))}
+    del extra['request_headers']['User-Agent']
+    assert extra == {
+        'request_headers': {
+            u'Accept': u'*/*',
+            u'Accept-Encoding': u'gzip, deflate',
+            u'Connection': u'keep-alive',
+        },
+        'request_url': 'https://example.com/',
+        'response_headers': {
+            u'Error Webservice': u'\ufffd'
+        },
+        'response_status': 200
+    }
-- 
2.26.2