From 6f183831420b24c9695616622198d82f030b6ee9 Mon Sep 17 00:00:00 2001 From: Jeremy Bowman Date: Wed, 18 Sep 2019 11:59:24 -0400 Subject: [PATCH] Fix bytes/string handling in tracking middleware --- common/djangoapps/track/middleware.py | 10 +++++++--- .../djangoapps/track/tests/test_middleware.py | 20 +++++++++---------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/common/djangoapps/track/middleware.py b/common/djangoapps/track/middleware.py index 6ccb7617af..77cfda2650 100644 --- a/common/djangoapps/track/middleware.py +++ b/common/djangoapps/track/middleware.py @@ -144,7 +144,10 @@ class TrackMiddleware(object): # HTTP headers may contain Latin1 characters. Decoding using Latin1 encoding here # avoids encountering UnicodeDecodeError exceptions when these header strings are # output to tracking logs. - context[context_key] = request.META.get(header_name, '').decode('latin1') + context_value = request.META.get(header_name, '') + if isinstance(context_value, six.binary_type): + context_value = context_value.decode('latin1') + context[context_key] = context_value # Google Analytics uses the clientId to keep track of unique visitors. A GA cookie looks like # this: _ga=GA1.2.1033501218.1368477899. The clientId is this part: 1033501218.1368477899. @@ -183,8 +186,9 @@ class TrackMiddleware(object): # Using a known-insecure hash to shorten is silly. # Also, why do we need same length? key_salt = "common.djangoapps.track" + self.__class__.__name__ - key = hashlib.md5(key_salt + settings.SECRET_KEY).digest() - encrypted_session_key = hmac.new(key, msg=session_key, digestmod=hashlib.md5).hexdigest() + key_bytes = (key_salt + settings.SECRET_KEY).encode('utf-8') + key = hashlib.md5(key_bytes).digest() + encrypted_session_key = hmac.new(key, msg=session_key.encode('utf-8'), digestmod=hashlib.md5).hexdigest() return encrypted_session_key def get_user_primary_key(self, request): diff --git a/common/djangoapps/track/tests/test_middleware.py b/common/djangoapps/track/tests/test_middleware.py index 8cf800f290..8fe672324b 100644 --- a/common/djangoapps/track/tests/test_middleware.py +++ b/common/djangoapps/track/tests/test_middleware.py @@ -49,9 +49,7 @@ class TrackMiddlewareTestCase(TestCase): request.META[meta_key] = 'test latin1 \xd3 \xe9 \xf1' # pylint: disable=no-member context = self.get_context_for_request(request) - # The bytes in the string on the right are utf8 encoded in the source file, so we decode them to construct - # a valid unicode string. - self.assertEqual(context[context_key], 'test latin1 Ó é ñ'.decode('utf8')) + self.assertEqual(context[context_key], u'test latin1 Ó é ñ') def test_default_filters_do_not_render_view(self): for url in ['/event', '/event/1', '/login', '/heartbeat']: @@ -79,7 +77,7 @@ class TrackMiddlewareTestCase(TestCase): def test_default_request_context(self): context = self.get_context_for_path('/courses/') - self.assertEquals(context, { + self.assertEqual(context, { 'accept_language': '', 'referer': '', 'user_id': '', @@ -101,7 +99,7 @@ class TrackMiddlewareTestCase(TestCase): request.META['REMOTE_ADDR'] = remote_addr context = self.get_context_for_request(request) - self.assertEquals(context['ip'], remote_addr) + self.assertEqual(context['ip'], remote_addr) def test_single_forward_for_header_ip_context(self): request = self.request_factory.get('/courses/') @@ -112,7 +110,7 @@ class TrackMiddlewareTestCase(TestCase): request.META['HTTP_X_FORWARDED_FOR'] = forwarded_ip context = self.get_context_for_request(request) - self.assertEquals(context['ip'], forwarded_ip) + self.assertEqual(context['ip'], forwarded_ip) def test_multiple_forward_for_header_ip_context(self): request = self.request_factory.get('/courses/') @@ -123,7 +121,7 @@ class TrackMiddlewareTestCase(TestCase): request.META['HTTP_X_FORWARDED_FOR'] = forwarded_ip context = self.get_context_for_request(request) - self.assertEquals(context['ip'], '11.22.33.44') + self.assertEqual(context['ip'], '11.22.33.44') def get_context_for_path(self, path): """Extract the generated event tracking context for a given request for the given path.""" @@ -138,7 +136,7 @@ class TrackMiddlewareTestCase(TestCase): finally: self.track_middleware.process_response(request, None) - self.assertEquals( + self.assertEqual( tracker.get_tracker().resolve_context(), {} ) @@ -156,7 +154,7 @@ class TrackMiddlewareTestCase(TestCase): def assert_dict_subset(self, superset, subset): """Assert that the superset dict contains all of the key-value pairs found in the subset dict.""" for key, expected_value in six.iteritems(subset): - self.assertEquals(superset[key], expected_value) + self.assertEqual(superset[key], expected_value) def test_request_with_user(self): user_id = 1 @@ -177,7 +175,7 @@ class TrackMiddlewareTestCase(TestCase): request.session.save() session_key = request.session.session_key expected_session_key = self.track_middleware.encrypt_session_key(session_key) - self.assertEquals(len(session_key), len(expected_session_key)) + self.assertEqual(len(session_key), len(expected_session_key)) context = self.get_context_for_request(request) self.assert_dict_subset(context, { 'session': expected_session_key, @@ -188,7 +186,7 @@ class TrackMiddlewareTestCase(TestCase): session_key = '665924b49a93e22b46ee9365abf28c2a' expected_session_key = '3b81f559d14130180065d635a4f35dd2' encrypted_session_key = self.track_middleware.encrypt_session_key(session_key) - self.assertEquals(encrypted_session_key, expected_session_key) + self.assertEqual(encrypted_session_key, expected_session_key) def test_request_headers(self): ip_address = '10.0.0.0'