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

Support the WebSocket Denial Response ASGI extension #1916

Merged
merged 23 commits into from
Dec 17, 2023
Merged
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
1a32e1a
create test for websocket responses
kristjanvalur Mar 19, 2023
9c37602
update websockets protocol
kristjanvalur Mar 19, 2023
ad649ed
update wsproto
kristjanvalur Mar 19, 2023
ed7a4dd
add multi-body response test
kristjanvalur Mar 19, 2023
8f83cab
Update tests/protocols/test_websocket.py
kristjanvalur Mar 20, 2023
4d74d4d
fix tests
kristjanvalur Mar 20, 2023
3996b48
fix mypy problems
kristjanvalur Mar 20, 2023
4390fb7
Simply fail if application passes an invalid status code
kristjanvalur Mar 20, 2023
4f7f1f4
Add a test for invalid message order
kristjanvalur Mar 20, 2023
2f47ae4
Update uvicorn/protocols/websockets/websockets_impl.py
kristjanvalur Mar 22, 2023
7a745e4
Fix initial response error handling and unit-test
kristjanvalur Mar 22, 2023
c9586bb
Add a similar missing-body test
kristjanvalur Mar 22, 2023
4047a3a
Use httpx to check rejection response body
kristjanvalur Mar 27, 2023
46d654d
Add test showing how content-length/transfer-encoding is automaticall…
kristjanvalur Mar 29, 2023
d2944e0
Do not send the response start until the first response body is received
kristjanvalur Mar 30, 2023
8c9db89
Update unittest to use protocol fixture
kristjanvalur Dec 3, 2023
198be2e
Check wsproto response status code
kristjanvalur Dec 3, 2023
946eb7c
Fix linter
Kludex Dec 17, 2023
95f3b03
Send response headers as soon as we have them
Kludex Dec 17, 2023
db586a1
Add test to make sure a single `websocket.http.response.start` is sent
Kludex Dec 17, 2023
f81f3f0
Modify the conditionals on wsproto
Kludex Dec 17, 2023
0876494
lint
Kludex Dec 17, 2023
798613a
Remove handshake_complete
Kludex Dec 17, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add test showing how content-length/transfer-encoding is automaticall…
…y handled.
kristjanvalur authored and Kludex committed Dec 17, 2023
commit 46d654d47be7cf4dff59966668c133cbb85c79b1
49 changes: 49 additions & 0 deletions tests/protocols/test_websocket.py
Original file line number Diff line number Diff line change
@@ -1131,6 +1131,55 @@ async def websocket_session(url):
await websocket_session(f"ws://127.0.0.1:{unused_tcp_port}")


@pytest.mark.anyio
@pytest.mark.parametrize("ws_protocol_cls", WS_PROTOCOLS)
@pytest.mark.parametrize("http_protocol_cls", HTTP_PROTOCOLS)
async def test_server_reject_connection_with_body_nolength(
ws_protocol_cls, http_protocol_cls, unused_tcp_port: int
):
# test that the server can send a response with a body but no content-length
async def app(scope, receive, send):
assert scope["type"] == "websocket"
assert "websocket.http.response" in scope["extensions"]

# Pull up first recv message.
message = await receive()
assert message["type"] == "websocket.connect"

message = {
"type": "websocket.http.response.start",
"status": 403,
"headers": [],
}
await send(message)
message = {
"type": "websocket.http.response.body",
"body": b"hardbody",
}
await send(message)

async def websocket_session(url):
response = await wsresponse(url)
assert response.status_code == 403
assert response.content == b"hardbody"
if ws_protocol_cls == WSProtocol:
# wsproto automatically makes the message chunked
assert response.headers["transfer-encoding"] == "chunked"
else:
# websockets automatically adds a content-length
assert response.headers["content-length"] == "8"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense for WebSockets to add the content-length, since we can't have a chunked one there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, this is only about what happens if the app doesn't provide a content-length and only provides the raw body data. the defaults for these two protocols are different. Technically, you can provide chunked data over the websockets: The app, provides the "chunked" header, and then provides the chunks, with the appropriate chunk headers (as an application should do if it provides that header (I think). I think that nowadays it is rare for an application to specify content length or transfer encoding...


config = Config(
app=app,
ws=ws_protocol_cls,
http=http_protocol_cls,
lifespan="off",
port=unused_tcp_port,
)
async with run_server(config):
await websocket_session(f"ws://127.0.0.1:{unused_tcp_port}")


@pytest.mark.anyio
@pytest.mark.parametrize("ws_protocol_cls", WS_PROTOCOLS)
@pytest.mark.parametrize("http_protocol_cls", HTTP_PROTOCOLS)