ASGI http.disconnect not handled on requests with body.

[Carlton Gibson]

Noticed whilst reviewing ​PR 15704 for #33699, we're not handling the ASGI http.disconnect message correctly. Since it's only dealt with whilst reading the request body, http.disconnect is not processed on a request that includes a body.

​https://github.com/django/django/blob/241fe59b74bb6031fa644f3ad55e6ad6a9187510/django/core/handlers/asgi.py#L189

    async def read_body(self, receive):
        """Reads an HTTP body from an ASGI connection."""
        # Use the tempfile that auto rolls-over to a disk file as it fills up.
        body_file = tempfile.SpooledTemporaryFile(
            max_size=settings.FILE_UPLOAD_MAX_MEMORY_SIZE, mode="w+b"
        )
        while True:
            message = await receive()
            if message["type"] == "http.disconnect":    # This is the only place `http.disconnect` is handled. 
                body_file.close()
                # Early client disconnect.
                raise RequestAborted()
            # Add a body chunk from the message, if provided.
            if "body" in message:
                body_file.write(message["body"])
            # Quit out if that's the end.
            if not message.get("more_body", False):
                break
        body_file.seek(0)
        return body_file

http.disconnect is designed for long-polling — so we imagine a client opening a request, with a request body, and then disconnecting before the response is generated.

The protocol server (Daphne/uvicorn/...) will send the http.diconnect message, but it's not handled.

This test fails on main (at 9f5548952906c6ea97200c016734b4f519520a64 — 4.2 pre-alpha)

diff --git a/tests/asgi/tests.py b/tests/asgi/tests.py
index ef7b55724e..a68ca8a473 100644
--- a/tests/asgi/tests.py
+++ b/tests/asgi/tests.py
@@ -188,6 +188,18 @@ class ASGITest(SimpleTestCase):
         with self.assertRaises(asyncio.TimeoutError):
             await communicator.receive_output()
 
+    async def test_disconnect_with_body(self):
+        application = get_asgi_application()
+        scope = self.async_request_factory._base_scope(path="/")
+        communicator = ApplicationCommunicator(application, scope)
+        await communicator.send_input({
+            "type": "http.request",
+            "body": b"some body",
+        })
+        await communicator.send_input({"type": "http.disconnect"})
+        with self.assertRaises(asyncio.TimeoutError):
+            await communicator.receive_output()
+
     async def test_wrong_connection_type(self):
         application = get_asgi_application()
         scope = self.async_request_factory._base_scope(path="/", type="other")

To handle this correctly it looks like we'd need something like Channel's ​`await_many_dispatch()` to keep receiving from the input queue whilst dispatching the request. 🤔

[Mariusz Felisiak]

Thanks! Don't you think it's a bug? 🤔

[Carlton Gibson]

Don't you think it's a bug? 🤔

I had it down as such, but it's never worked, and once I started thinking about the fix I thought it's probably a non-minor adjustment so... (But happy if you want to re-classify :)

[rahul negi]

Replying to Carlton Gibson:

    async def read_body(self, receive):
        """Reads an HTTP body from an ASGI connection."""
        # Use the tempfile that auto rolls-over to a disk file as it fills up.
        body_file = tempfile.SpooledTemporaryFile(
            max_size=settings.FILE_UPLOAD_MAX_MEMORY_SIZE, mode="w+b"
        )
        while True:
            message = await receive()
            if message["type"] == "http.disconnect":    # This is the only place `http.disconnect` is handled. 
                body_file.close()
                # Early client disconnect.
                raise RequestAborted()
            # Add a body chunk from the message, if provided.
            if "body" in message:
                body_file.write(message["body"])
            # Quit out if that's the end.
            if not message.get("more_body", False):
                break
        body_file.seek(0)
        return body_file

I'm following this ticket for some days and trying to understand the code base learned more about the asynchronous processes and long-polling,
IDK this is a solution or not,

if we perform a loop only while receiving the message and break it after X interval of time like

      while True:
          message = await receive()
          message_queue.append(message)
          if time_waited == X :
              break 

and do store it in a queue and then perform a loop on the queue data to read the message?

Please correct me if I'm wrong or if this sounds immature,

thanks

[Carlton Gibson]

Hi Rahul.

... if we perform a loop only while receiving the message and break it after X interval of time...

This won't work I'm afraid. We still need to be able to handle the incoming http.disconnect at any time.

It's not 100% straightforward but something like Channels' await_many_dispatch() will be needed. See ​the usage here: it's able to route multiple messages.
(I don't think the example there is 100% properly handling the disconnect, as a kind of interrupt there either, since the dispatch task will only process one message at a time, where we're looking to handle a disconnect whilst processing a long-running request… 🤔)