Repository navigation
fix: drain oversized polling POST bodies so a rejected connection event cannot stall the page - #7621
Conversation
 [Medium impact] The reviewed changes appear safe to merge. SummaryThe PR drains oversized polling POST bodies before returning 400, then closes the rejected session without waiting for another poll.
Reviews (5) · Last reviewed commit: "Decode the drained POST's query string a..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Merging this PR will not alter performance1 warning
Comparing 
|
After the drain, answer the 400 from _DrainOversizedBodyMiddleware itself and close the engine.io session with socket.close(wait=False) instead of delegating to engine.io. engine.io rejects by disconnecting the session, whose close(wait=True) blocks in queue.join() until a poll GET consumes the CLOSE packet; the engine.io client races the CONNECT POST against the first poll GET, so with no poll parked the POST response was held forever and later polls got 'Session is disconnected'. The CLOSE packet still reaches a parked poll, but a missing poll no longer holds the response. Add a no-parked-poll regression test under asyncio.wait_for; it times out on the unpatched middleware and passes with the fix.
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
|
The 11 workflows on the new head are awaiting maintainer approval. Could you approve the CI run? Local tests pass, but GitHub CI hasn't verified this head yet. |
|
Both points addressed in 762788c. The middleware drains the oversized POST, returns 400 itself, and closes the engine.io socket with wait=False. The new no-parked-poll test times out on the previous revision and passes now. Local checks pass, and full CI is green on this head (166 passed, 3 skipped). |
- Take the engine.io server as a required argument and read its buffer limit, instead of a separate max_size and an optional eio. - Drain with a single more_body loop, read Content-Length through starlette Headers, and drop the ValueError and broad exception guards (servers validate Content-Length; engine.io logs handler errors). - Import engineio only for type checking. - Extract the handshake and oversized POST test helpers, and add a test that drives an oversized POST through the App's event endpoint. - Describe the remaining user-facing case in the news fragment: reflex-dev#7551 stops oversized hydrate packets in the client, so the stall now comes from oversized events.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
engine.io treats a query string that is not UTF-8 as empty. A strict decode raised after the 400 was sent, so the session was not closed. Also word the news fragment as 'can reconnect': the client's reconnect policy controls the timing.
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
Summary
Fixes #7620.
With
transport="polling", when the client sends a CONNECT packet larger thanREFLEX_SOCKET_MAX_HTTP_BUFFER_SIZE(default 1 MB), engine.io comparesContent-Lengthagainstmax_http_buffer_sizeand answers 400 without reading the request body. The browser is still sending at that point, and the unanswered remainder wedges its connection pool, so the reconnect handshake is never served: the page stalls un-hydrated with no connection toast and no further retry.This wraps the socket ASGI app in a small middleware (
_DrainOversizedBodyMiddleware) that reads an oversized POST body to completion (discarding the bytes) before engine.io sees the request. The 400 then reaches a client that has finished sending, so it records the failure and retries with backoff, matching the websocket transport's behavior. Requests within the size limit pass through untouched.Tests
New tests in
tests/units/test_app.pydrive a real engine.io server at the ASGI level (a parked long-poll mirrors the browser's in-flight poll, which the server needs in order to deliver its CLOSE packet when the session is rejected):test_engineio_rejects_oversized_polling_post_without_draining_bodydocuments the dependency behavior being worked around (400 with the body unread).test_drain_oversized_body_middleware_drains_before_engineio_rejectsasserts the body is fully drained, the 400 still returns, and a fresh handshake is served afterwards.test_drain_oversized_body_middleware_passes_regular_requests_throughasserts in-limit POSTs and GETs pass through untouched.pytest tests/units/test_app.py: 197 passed.ruff check/ruff format --checkclean.cc @masenf