Updated channels.http.AsgiHandler to not mutate state. (#1581)

* Don't mutate AsgiHandler state in __call__(). Fixes #1578
* Added a test that fails without the previous commit
* Updated StaticFilesHandler to not rely on self.scope.

Co-authored-by: Alex Hill <alexh@signiq.com>
Co-authored-by: Carlton Gibson <carlton.gibson@noumenal.es>

channels/http.py

@@ -188,16 +188,14 @@ async def __call__(self, scope, receive, send):
                 "The AsgiHandler can only handle HTTP connections, not %s"
                 % scope["type"]
             )
-        self.scope = scope
-        self.send = async_to_sync(send)
 
         # Receive the HTTP request body as a stream object.
         try:
             body_stream = await self.read_body(receive)
         except RequestAborted:
             return
         # Launch into body handling (and a synchronous subthread).
-        await self.handle(body_stream)
+        await self.handle(scope, async_to_sync(send), body_stream)
 
     async def read_body(self, receive):
         """Reads a HTTP body from an ASGI connection."""
@@ -220,19 +218,19 @@ async def read_body(self, receive):
         return body_file
 
     @sync_to_async
-    def handle(self, body):
+    def handle(self, scope, send, body):
         """
         Synchronous message processing.
         """
         # Set script prefix from message root_path, turning None into empty string
-        script_prefix = self.scope.get("root_path", "") or ""
+        script_prefix = scope.get("root_path", "") or ""
         if settings.FORCE_SCRIPT_NAME:
             script_prefix = settings.FORCE_SCRIPT_NAME
         set_script_prefix(script_prefix)
-        signals.request_started.send(sender=self.__class__, scope=self.scope)
+        signals.request_started.send(sender=self.__class__, scope=scope)
         # Run request through view system
         try:
-            request = self.request_class(self.scope, body)
+            request = self.request_class(scope, body)
         except UnicodeDecodeError:
             logger.warning(
                 "Bad Request (UnicodeDecodeError)",
@@ -255,7 +253,7 @@ def handle(self, body):
                 response.block_size = 1024 * 512
         # Transform response into messages, which we yield back to caller
         for response_message in self.encode_response(response):
-            self.send(response_message)
+            send(response_message)
         # Close the response now we're done with it
         response.close()
 

channels/staticfiles.py

@@ -51,11 +51,15 @@ class StaticFilesHandler(AsgiHandler):
 
     # TODO: Review hierarchy here. Do we NEED to inherit BaseHandler, AsgiHandler?
 
+    async def __call__(self, scope, receive, send):
+        self.static_base_url = scope["static_base_url"][2]
+        return await super().__call__(scope, receive, send)
+
     def file_path(self, url):
         """
         Returns the relative path to the media file on disk for the given URL.
         """
-        relative_url = url[len(self.scope["static_base_url"][2]) :]
+        relative_url = url[len(self.static_base_url) :]
         return url2pathname(relative_url)
 
     def serve(self, request):

tests/test_http.py

@@ -351,6 +351,35 @@ async def test_handler_body_ignore_extra():
     assert body_stream.read() == b"chunk one"
 
 
+@pytest.mark.django_db
+@pytest.mark.asyncio
+@pytest.mark.filterwarnings("ignore::DeprecationWarning")
+async def test_handler_concurrent_requests():
+    """
+    Tests request handling ignores anything after more_body: False
+    """
+    scope = {"type": "http", "http_version": "1.1", "method": "GET", "path": "/test/"}
+    handler = MockHandler()
+    comm_1 = ApplicationCommunicator(handler, {**scope})
+    comm_2 = ApplicationCommunicator(handler, {**scope})
+
+    request_1 = comm_1.send_input(
+        {"type": "http.request", "body": b"request 1", "more_body": False}
+    )
+    request_2 = comm_2.send_input(
+        {"type": "http.request", "body": b"request 2", "more_body": False}
+    )
+
+    await request_1
+    await request_2
+
+    await comm_1.receive_output(1)  # response start
+    await comm_1.receive_output(1)  # response body
+
+    await comm_2.receive_output(1)  # response start
+    await comm_2.receive_output(1)  # response body
+
+
 @pytest.mark.django_db(transaction=True)
 @pytest.mark.asyncio
 async def test_sessions():

tests/test_staticfiles.py

@@ -1,6 +1,6 @@
 import pytest
 
-from channels.staticfiles import StaticFilesWrapper
+from channels.staticfiles import StaticFilesHandler, StaticFilesWrapper
 
 
 @pytest.fixture(autouse=True)
@@ -99,3 +99,27 @@ def test_is_single_callable():
         "StaticFilesWrapper should be recognized as a single callable by "
         "asgiref compatibility tools"
     )
+
+
+@pytest.mark.asyncio
+async def test_staticfiles_handler_can_generate_file_path():
+    """
+    StaticFilesHandler.file_path must not rely on scope being assigned to self.
+    """
+
+    class MockedHandler(StaticFilesHandler):
+        async def __call__(self, scope, receive, send):
+            # Equivalent setUp from real __call__.
+            request = self.request_class(scope, "")
+            self.static_base_url = scope["static_base_url"][2]
+            # Method under test.
+            return self.file_path(request.path)
+
+    wrapper = StaticFilesWrapper(
+        MockApplication("application"), staticfiles_handler=MockedHandler
+    )
+    scope = request_for_path("/static/image.png")
+    scope["method"] = "GET"
+    assert (
+        await wrapper(scope, None, None) == "/image.png"
+    ), "StaticFilesWrapper should serve paths under the STATIC_URL path"