Skip to content

Commit 3adf24e

Browse files
authored
Middleware: use regular HttpResponse and log the suspicious operation (#9366)
Do not use `raise SuspiciousOperation` here because that ends up into Sentry marked as an error logged via logger `django.security.SuspiciousOperation`. I prefer to use regular `log.info` for these situations so they end up in NewRelic and we can parse them nicely. They are not application errors.
1 parent 9fbe1f9 commit 3adf24e

File tree

2 files changed

+16
-4
lines changed

2 files changed

+16
-4
lines changed

readthedocs/core/middleware.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
MiddlewareNotUsed,
1010
SuspiciousOperation,
1111
)
12+
from django.http import HttpResponse
1213
from django.utils.cache import patch_vary_headers
1314
from django.utils.http import http_date
1415

@@ -211,7 +212,14 @@ def __init__(self, get_response):
211212
def __call__(self, request):
212213
for key, value in request.GET.items():
213214
if "\x00" in value:
214-
raise SuspiciousOperation(
215-
f"There are NULL (0x00) characters in at least one of the parameters ({key}) passed to the request." # noqa
215+
log.info(
216+
"NULL (0x00) characters in GET attributes.",
217+
attribute=key,
218+
value=value,
219+
url=request.build_absolute_uri(),
220+
)
221+
return HttpResponse(
222+
f"There are NULL (0x00) characters in at least one of the parameters ({key}) passed to the request.", # noqa
223+
status=400,
216224
)
217225
return self.get_response(request)

readthedocs/rtd_tests/tests/test_middleware.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
from corsheaders.middleware import CorsMiddleware
44
from django.conf import settings
5-
from django.core.exceptions import SuspiciousOperation
65
from django.http import HttpResponse
76
from django.test import TestCase, override_settings
87
from django.test.client import RequestFactory
@@ -272,4 +271,9 @@ def setUp(self):
272271

273272
def test_request_with_null_chars(self):
274273
request = self.factory.get("/?language=en\x00es&project_slug=myproject")
275-
self.assertRaises(SuspiciousOperation, lambda: self.middleware(request))
274+
response = self.middleware(request)
275+
self.assertContains(
276+
response,
277+
"There are NULL (0x00) characters in at least one of the parameters (language) passed to the request.",
278+
status_code=400,
279+
)

0 commit comments

Comments
 (0)