Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 31 commits into from
Nov 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
f997afa
Handling asgi body in the right way. For real
antonpirker Nov 16, 2023
b516862
Better url handling
antonpirker Nov 17, 2023
aa47870
Better handling of body read
antonpirker Nov 17, 2023
543d79d
Cleanup
antonpirker Nov 17, 2023
44254bd
Cleanup
antonpirker Nov 17, 2023
f4abc66
Fixed test
antonpirker Nov 17, 2023
7f4300e
Fixed bug with rest framework
antonpirker Nov 17, 2023
577437e
Merge branch 'master' into antonpirker/django-asgi-request-body
antonpirker Nov 17, 2023
2bd5729
silence mypy
antonpirker Nov 17, 2023
ff211b1
Capture eventual errors caused by DjangoRestFramework (or our test se…
antonpirker Nov 17, 2023
93e1e70
cleanup
antonpirker Nov 17, 2023
c64ebcd
Except more specific error
antonpirker Nov 17, 2023
6f43733
Fix
antonpirker Nov 17, 2023
b5d4594
fixe typing ignore
antonpirker Nov 17, 2023
d6ecf29
Remove type ignore
antonpirker Nov 17, 2023
efe3b32
Make it work in Python 2.7
antonpirker Nov 17, 2023
3762a19
Increased test timeout
antonpirker Nov 20, 2023
935116b
Merge branch 'master' into antonpirker/django-asgi-request-body
antonpirker Nov 20, 2023
4edbf0f
Update test matrix
antonpirker Nov 20, 2023
3a1f15a
is_coroutine_function was removed from quart, taking from asyncio dir…
antonpirker Nov 20, 2023
ba68dc9
fix
antonpirker Nov 20, 2023
a38bef8
Merge branch 'antonpirker/fix-qart-tests' into antonpirker/django-asg…
antonpirker Nov 20, 2023
aa9718d
trying something
antonpirker Nov 20, 2023
a02aade
It is not a timeout problem
antonpirker Nov 20, 2023
538feac
hopalla
antonpirker Nov 20, 2023
ea3a4b3
Better readablye diff
antonpirker Nov 20, 2023
bc72552
Fix hanging test
antonpirker Nov 20, 2023
dfab15f
Reactivated tests
antonpirker Nov 20, 2023
ab0076d
Merge branch 'master' into antonpirker/django-asgi-request-body
antonpirker Nov 20, 2023
9eab8a6
enabled forking again (was just for local testing)
antonpirker Nov 20, 2023
2fa2973
Merge branch 'antonpirker/django-asgi-request-body' of github.com:get…
antonpirker Nov 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion sentry_sdk/integrations/_wsgi_common.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import absolute_import

import json
from copy import deepcopy

Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down
6 changes: 0 additions & 6 deletions sentry_sdk/integrations/django/asgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions sentry_sdk/integrations/django/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Binary file added tests/integrations/django/asgi/image.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
127 changes: 116 additions & 11 deletions tests/integrations/django/asgi/test_asgi.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import base64
import json
import os

import django
import pytest
Expand Down Expand Up @@ -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
Expand All @@ -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"]
6 changes: 3 additions & 3 deletions tests/integrations/django/myapp/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down