adamchainz / django-permissions-policy

Set the draft security HTTP header Permissions-Policy (previously Feature-Policy) on your Django app.
MIT License
98 stars 6 forks source link

Error in the middleware when using in async mode and Python 3.12 #425

Closed Alexerson closed 9 months ago

Alexerson commented 9 months ago

Python Version

3.12.1

Django Version

5.0.1

Package Version

4.18.0

Description

When using the middleware with other middlewares, I get the following warning: RuntimeWarning: coroutine 'PermissionsPolicyMiddleware.__acall__' was never awaited.

I’m not quite sure what changed in 3.12 (something about _is_coroutine it seems), but regardless, the if self._is_coroutine test returns False when it shouldn’t.

I’ve looked at the test-suites and it seems it should be tested, so I’m not quite sure what’s up.

Edit: it seems to only happen if used in combination with another middleware… I’m trying to figure out why.

Alexerson commented 9 months ago

I know how to fix the issue (basically use the same strategy as asgiref: use the inspect.iscoroutinefunction and inspect.markcoroutinefunction if they are defined, otherwise keep the current strategy), but I couldn’t create a test that reproduces the error. I’ll look again tomorrow.

Alexerson commented 9 months ago

I looked into this more and simply adding django.middleware.security.SecurityMiddleware before your middleware triggers this issue. I tried with this one because that’s the one you mention in the docs. But I’m assuming that’s not the only one that could cause the issue.


index e19721b..0796732 100644
--- a/tests/test_django_permissions_policy.py
+++ b/tests/test_django_permissions_policy.py
@@ -9,6 +9,7 @@ from django.test import RequestFactory
 from django.test import SimpleTestCase

 from django_permissions_policy import PermissionsPolicyMiddleware
+from django.middleware.security import SecurityMiddleware

 class PermissionsPolicyMiddlewareTests(SimpleTestCase):
@@ -112,7 +113,7 @@ class PermissionsPolicyMiddlewareTests(SimpleTestCase):
         async def dummy_async_view(request):
             return HttpResponse("Hello!")

-        middleware = PermissionsPolicyMiddleware(dummy_async_view)
+        middleware = SecurityMiddleware(PermissionsPolicyMiddleware(dummy_async_view))
         request = self.request_factory.get("/", HTTP_HX_REQUEST="true")

         with override_settings(PERMISSIONS_POLICY={"geolocation": "self"}):```