Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug in WebSocketProtocol.asgi_receive #1272

Conversation

kylebebak
Copy link
Contributor

@kylebebak kylebebak commented Nov 28, 2021

@euri10 @Kludex

There were 2 things to fix in #1230:

  • ensure the server can read frames in the read queue after the client has sent the close frame
  • ensure the server can read frames in the read queue after the server has sent the close frame

The first item was fixed was fixed by this PR: #1252. But I realized after this PR was merged that the second item hadn't been fixed.

This PR fixes the second item. It ensures that the server can "drain" unread frames in the read queue even after the server has sent the close frame, as long as those frames were sent before the server sent the close frame.

As with the first item, the second item is part of the websockets spec. The following code, using just the websockets library, makes it clear that this is the intended behavior of this library.

I did not include code in this PR that fixes this bug in wsproto_impl.py, because I'm not familiar with wsproto. That said, I don't think we should gate this fix on the corresponding fix in wsproto.

client.py

import asyncio

import websockets


async def hello():
    async with websockets.connect("ws://localhost:8765") as websocket:
        await websocket.send("message 1")
        await websocket.send("message 2")
        await websocket.send("message 3")

        # Sleep until after server has sent close frame
        await asyncio.sleep(0.3)

        try:
            await websocket.send("message 4")
        except websockets.exceptions.ConnectionClosed as e:
            print("message 4 not sent, connection closed:", e)


asyncio.run(hello())

server.py

import asyncio

import websockets


async def handler(websocket, *args):
    # Give client time to send frames that go into read queue before server sends close frame
    await asyncio.sleep(0.2)
    await websocket.close(1000)

    # Server can still read these messages from read queue after sending close frame
    print(await websocket.recv())
    print(await websocket.recv())
    print(await websocket.recv())


async def main():
    async with websockets.serve(handler, "localhost", 8765):
        await asyncio.Future()  # Run forever


asyncio.run(main())

@kylebebak
Copy link
Contributor Author

kylebebak commented Dec 15, 2021

@euri10 @Kludex , just pinging you guys in case you have time to look at this. It's small and pretty well-tested

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant