From a320a49ff940d7a8da7e2da20288ba32c4382d52 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Tue, 25 Jul 2017 16:58:14 +0200 Subject: [PATCH 1/3] Django: get_data_from_request: include exception with unavailable msg --- opbeat/contrib/django/client.py | 9 +++++---- tests/contrib/django/django_tests.py | 12 ++++++++++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/opbeat/contrib/django/client.py b/opbeat/contrib/django/client.py index ac921b2d..33b783a9 100644 --- a/opbeat/contrib/django/client.py +++ b/opbeat/contrib/django/client.py @@ -106,15 +106,16 @@ def get_data_from_request(self, request): if request.method != 'GET': try: - if hasattr(request, 'body'): + try: # Django 1.4+ raw_data = request.body - else: + except AttributeError: raw_data = request.raw_post_data data = raw_data if raw_data else request.POST - except Exception: + except Exception as exc: # assume we had a partial read: - data = '' + data = ''.format( + exc.__class__.__name__, exc) else: data = None diff --git a/tests/contrib/django/django_tests.py b/tests/contrib/django/django_tests.py index b01e7bcb..ccb4e79d 100644 --- a/tests/contrib/django/django_tests.py +++ b/tests/contrib/django/django_tests.py @@ -518,7 +518,11 @@ def test_raw_post_data_partial_read(self): self.assertTrue('http' in event) http = event['http'] self.assertEquals(http['method'], 'POST') - self.assertEquals(http['data'], '') + if django.VERSION >= (1, 7): + expected_exception = 'RawPostDataException' + else: + expected_exception = 'Exception' + self.assertEquals(http['data'], "".format(expected_exception)) def test_post_data(self): request = WSGIRequest(environ={ @@ -619,7 +623,11 @@ def test_request_capture(self): self.assertTrue('http' in event) http = event['http'] self.assertEquals(http['method'], 'POST') - self.assertEquals(http['data'], '') + if django.VERSION >= (1, 7): + expected_exception = 'RawPostDataException' + else: + expected_exception = 'Exception' + self.assertEquals(http['data'], "".format(expected_exception)) self.assertTrue('headers' in http) headers = http['headers'] self.assertTrue('Content-Type' in headers, headers.keys()) From 78917a7ff30bebcaa52f2671e2f1da48f05472db Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Wed, 26 Jul 2017 13:00:16 +0200 Subject: [PATCH 2/3] doc: note about hasattr() on Python 2.7 vs. 3 [ci skip] --- opbeat/contrib/django/client.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/opbeat/contrib/django/client.py b/opbeat/contrib/django/client.py index 33b783a9..d5ab3f5f 100644 --- a/opbeat/contrib/django/client.py +++ b/opbeat/contrib/django/client.py @@ -108,6 +108,9 @@ def get_data_from_request(self, request): try: try: # Django 1.4+ + # NOTE: on Python 2.7 `hasattr(request, 'body')` returns + # False, while it might throw the exception from Django in + # Python 3. raw_data = request.body except AttributeError: raw_data = request.raw_post_data From 691bdf244204cf747c6e01345b96d373f9985dd8 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Thu, 28 Sep 2017 18:03:59 +0200 Subject: [PATCH 3/3] Include request.POST, cope/test for multipart/form-data --- opbeat/contrib/django/client.py | 6 +-- tests/contrib/django/django_tests.py | 57 ++++++++++++++++++++++++++-- 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/opbeat/contrib/django/client.py b/opbeat/contrib/django/client.py index d5ab3f5f..91e6e4a0 100644 --- a/opbeat/contrib/django/client.py +++ b/opbeat/contrib/django/client.py @@ -116,9 +116,9 @@ def get_data_from_request(self, request): raw_data = request.raw_post_data data = raw_data if raw_data else request.POST except Exception as exc: - # assume we had a partial read: - data = ''.format( - exc.__class__.__name__, exc) + # Assume we had a partial read, or multipart/form-data: + data = '\nrequest.POST: {2}'.format( + exc.__class__.__name__, exc, request.POST) else: data = None diff --git a/tests/contrib/django/django_tests.py b/tests/contrib/django/django_tests.py index ccb4e79d..ca6df9d4 100644 --- a/tests/contrib/django/django_tests.py +++ b/tests/contrib/django/django_tests.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- import pytest # isort:skip -django = pytest.importorskip("django") # isort:skip +django = pytest.importorskip("django") # noqa: E402 isort:skip import datetime import logging @@ -21,6 +21,7 @@ from django.test import TestCase from django.test.client import Client as _TestClient from django.test.client import ClientHandler as _TestClientHandler +from django.test.client import FakePayload from django.test.utils import override_settings import mock @@ -522,7 +523,8 @@ def test_raw_post_data_partial_read(self): expected_exception = 'RawPostDataException' else: expected_exception = 'Exception' - self.assertEquals(http['data'], "".format(expected_exception)) + self.assertEquals(http['data'], "\nrequest.POST: {1}".format(expected_exception, request.POST)) + self.assertEquals(request.POST, {}) def test_post_data(self): request = WSGIRequest(environ={ @@ -602,7 +604,7 @@ def test_disallowed_hosts_error_django_18(self): self.assertEqual(event['http']['url'], None) # This test only applies to Django 1.3+ - def test_request_capture(self): + def test_request_capture_partial_read(self): if django.VERSION[:2] < (1, 3): return request = WSGIRequest(environ={ @@ -627,7 +629,8 @@ def test_request_capture(self): expected_exception = 'RawPostDataException' else: expected_exception = 'Exception' - self.assertEquals(http['data'], "".format(expected_exception)) + self.assertEquals(http['data'], "\nrequest.POST: {1}".format(expected_exception, request.POST)) + # self.assertEquals(http['data'], '{}') self.assertTrue('headers' in http) headers = http['headers'] self.assertTrue('Content-Type' in headers, headers.keys()) @@ -638,6 +641,52 @@ def test_request_capture(self): self.assertTrue('SERVER_PORT' in env, env.keys()) self.assertEquals(env['SERVER_PORT'], '80') + def test_request_capture_multipart_formdata(self): + if django.VERSION[:2] < (1, 3): + return + + payload = FakePayload("\r\n".join([ + '--boundary', + 'Content-Disposition: form-data; name="name"', + '', + 'value', + '--boundary--' + ''])) + request = WSGIRequest({ + 'REQUEST_METHOD': 'POST', + 'SERVER_NAME': 'testserver', + 'SERVER_PORT': '80', + 'CONTENT_TYPE': 'multipart/form-data; boundary=boundary', + 'CONTENT_LENGTH': 78, + 'wsgi.input': payload}) + + # Read POST data to trigger exception when accessing request.body. + self.assertEqual(request.POST, {'name': ['value']}) + + self.opbeat.capture('Message', message='foo', request=request) + + self.assertEquals(len(self.opbeat.events), 1) + event = self.opbeat.events.pop(0) + + self.assertTrue('http' in event) + http = event['http'] + self.assertEquals(http['method'], 'POST') + if django.VERSION >= (1, 7): + expected_exception = 'RawPostDataException' + else: + expected_exception = 'Exception' + self.assertEquals(http['data'], "\nrequest.POST: {1}".format(expected_exception, request.POST)) + # self.assertEquals(http['data'], '{}') + self.assertTrue('headers' in http) + headers = http['headers'] + self.assertTrue('Content-Type' in headers, headers.keys()) + self.assertEquals(headers['Content-Type'], 'multipart/form-data; boundary=boundary') + env = http['env'] + self.assertTrue('SERVER_NAME' in env, env.keys()) + self.assertEquals(env['SERVER_NAME'], 'testserver') + self.assertTrue('SERVER_PORT' in env, env.keys()) + self.assertEquals(env['SERVER_PORT'], '80') + def test_transaction_metrics(self): self.opbeat.instrumentation_store.get_all() # clear the store with self.settings(MIDDLEWARE_CLASSES=[