Handling asgi body in the right way. For real (#2513)

Handling the request body in ASGI applications. By reading the body first it gets cached (by for example Django) which makes it possible to read the body multiple times.

sentry_sdk/integrations/_wsgi_common.py

@@ -1,3 +1,5 @@
+from __future__ import absolute_import
+
 import json
 from copy import deepcopy
 
@@ -7,6 +9,12 @@
 
 from sentry_sdk._types import TYPE_CHECKING
 
+try:
+    from django.http.request import RawPostDataException
+except ImportError:
+    RawPostDataException = None
+
+
 if TYPE_CHECKING:
     import sentry_sdk
 
@@ -67,10 +75,22 @@ def extract_into_event(self, event):
         if not request_body_within_bounds(client, content_length):
             data = AnnotatedValue.removed_because_over_size_limit()
         else:
+            # First read the raw body data
+            # It is important to read this first because if it is Django
+            # it will cache the body and then we can read the cached version
+            # again in parsed_body() (or json() or wherever).
+            raw_data = None
+            try:
+                raw_data = self.raw_data()
+            except (RawPostDataException, ValueError):
+                # If DjangoRestFramework is used it already read the body for us
+                # so reading it here will fail. We can ignore this.
+                pass
+
             parsed_body = self.parsed_body()
             if parsed_body is not None:
                 data = parsed_body
-            elif self.raw_data():
+            elif raw_data:
                 data = AnnotatedValue.removed_because_raw_data()
             else:
                 data = None

sentry_sdk/integrations/django/asgi.py

@@ -94,12 +94,6 @@ def sentry_patched_create_request(self, *args, **kwargs):
 
             with hub.configure_scope() as scope:
                 request, error_response = old_create_request(self, *args, **kwargs)
-
-                # read the body once, to signal Django to cache the body stream
-                # so we can read the body in our event processor
-                # (otherwise Django closes the body stream and makes it impossible to read it again)
-                _ = request.body
-
                 scope.add_event_processor(_make_asgi_request_event_processor(request))
 
                 return request, error_response

sentry_sdk/integrations/django/views.py

@@ -47,13 +47,13 @@ def sentry_patched_make_view_atomic(self, *args, **kwargs):
 
         hub = Hub.current
         integration = hub.get_integration(DjangoIntegration)
-
         if integration is not None and integration.middleware_spans:
-            if (
+            is_async_view = (
                 iscoroutinefunction is not None
                 and wrap_async_view is not None
                 and iscoroutinefunction(callback)
-            ):
+            )
+            if is_async_view:
                 sentry_wrapped_callback = wrap_async_view(hub, callback)
             else:
                 sentry_wrapped_callback = _wrap_sync_view(hub, callback)

tests/integrations/django/asgi/test_asgi.py

@@ -1,4 +1,6 @@
+import base64
 import json
+import os
 
 import django
 import pytest
@@ -370,16 +372,105 @@ async def test_trace_from_headers_if_performance_disabled(sentry_init, capture_e
     assert error_event["contexts"]["trace"]["trace_id"] == trace_id
 
 
+PICTURE = os.path.join(os.path.dirname(os.path.abspath(__file__)), "image.png")
+BODY_FORM = """--fd721ef49ea403a6\r\nContent-Disposition: form-data; name="username"\r\n\r\nJane\r\n--fd721ef49ea403a6\r\nContent-Disposition: form-data; name="password"\r\n\r\nhello123\r\n--fd721ef49ea403a6\r\nContent-Disposition: form-data; name="photo"; filename="image.png"\r\nContent-Type: image/png\r\nContent-Transfer-Encoding: base64\r\n\r\n{{image_data}}\r\n--fd721ef49ea403a6--\r\n""".replace(
+    "{{image_data}}", base64.b64encode(open(PICTURE, "rb").read()).decode("utf-8")
+).encode(
+    "utf-8"
+)
+BODY_FORM_CONTENT_LENGTH = str(len(BODY_FORM)).encode("utf-8")
+
+
 @pytest.mark.parametrize("application", APPS)
 @pytest.mark.parametrize(
-    "body,expected_return_data",
+    "send_default_pii,method,headers,url_name,body,expected_data",
     [
         (
+            True,
+            "POST",
+            [(b"content-type", b"text/plain")],
+            "post_echo_async",
+            b"",
+            None,
+        ),
+        (
+            True,
+            "POST",
+            [(b"content-type", b"text/plain")],
+            "post_echo_async",
+            b"some raw text body",
+            "",
+        ),
+        (
+            True,
+            "POST",
+            [(b"content-type", b"application/json")],
+            "post_echo_async",
             b'{"username":"xyz","password":"xyz"}',
             {"username": "xyz", "password": "xyz"},
         ),
-        (b"hello", ""),
-        (b"", None),
+        (
+            True,
+            "POST",
+            [(b"content-type", b"application/xml")],
+            "post_echo_async",
+            b'<?xml version="1.0" encoding="UTF-8"?><root></root>',
+            "",
+        ),
+        (
+            True,
+            "POST",
+            [
+                (b"content-type", b"multipart/form-data; boundary=fd721ef49ea403a6"),
+                (b"content-length", BODY_FORM_CONTENT_LENGTH),
+            ],
+            "post_echo_async",
+            BODY_FORM,
+            {"password": "hello123", "photo": "", "username": "Jane"},
+        ),
+        (
+            False,
+            "POST",
+            [(b"content-type", b"text/plain")],
+            "post_echo_async",
+            b"",
+            None,
+        ),
+        (
+            False,
+            "POST",
+            [(b"content-type", b"text/plain")],
+            "post_echo_async",
+            b"some raw text body",
+            "",
+        ),
+        (
+            False,
+            "POST",
+            [(b"content-type", b"application/json")],
+            "post_echo_async",
+            b'{"username":"xyz","password":"xyz"}',
+            {"username": "xyz", "password": "[Filtered]"},
+        ),
+        (
+            False,
+            "POST",
+            [(b"content-type", b"application/xml")],
+            "post_echo_async",
+            b'<?xml version="1.0" encoding="UTF-8"?><root></root>',
+            "",
+        ),
+        (
+            False,
+            "POST",
+            [
+                (b"content-type", b"multipart/form-data; boundary=fd721ef49ea403a6"),
+                (b"content-length", BODY_FORM_CONTENT_LENGTH),
+            ],
+            "post_echo_async",
+            BODY_FORM,
+            {"password": "[Filtered]", "photo": "", "username": "Jane"},
+        ),
     ],
 )
 @pytest.mark.asyncio
@@ -388,28 +479,42 @@ async def test_trace_from_headers_if_performance_disabled(sentry_init, capture_e
     django.VERSION < (3, 1), reason="async views have been introduced in Django 3.1"
 )
 async def test_asgi_request_body(
-    sentry_init, capture_envelopes, application, body, expected_return_data
+    sentry_init,
+    capture_envelopes,
+    application,
+    send_default_pii,
+    method,
+    headers,
+    url_name,
+    body,
+    expected_data,
 ):
-    sentry_init(integrations=[DjangoIntegration()], send_default_pii=True)
+    sentry_init(
+        send_default_pii=send_default_pii,
+        integrations=[
+            DjangoIntegration(),
+        ],
+    )
 
     envelopes = capture_envelopes()
 
     comm = HttpCommunicator(
         application,
-        method="POST",
-        path=reverse("post_echo_async"),
+        method=method,
+        headers=headers,
+        path=reverse(url_name),
         body=body,
-        headers=[(b"content-type", b"application/json")],
     )
     response = await comm.get_response()
-
     assert response["status"] == 200
+
+    await comm.wait()
     assert response["body"] == body
 
     (envelope,) = envelopes
     event = envelope.get_event()
 
-    if expected_return_data is not None:
-        assert event["request"]["data"] == expected_return_data
+    if expected_data is not None:
+        assert event["request"]["data"] == expected_data
     else:
         assert "data" not in event["request"]

tests/integrations/django/myapp/views.py

@@ -237,10 +237,10 @@ def thread_ids_sync(*args, **kwargs):
     )
 
     exec(
-        """@csrf_exempt
-def post_echo_async(request):
+        """async def post_echo_async(request):
     sentry_sdk.capture_message("hi")
-    return HttpResponse(request.body)"""
+    return HttpResponse(request.body)
+post_echo_async.csrf_exempt = True"""
     )
 else:
     async_message = None