79 lines
2.7 KiB
Markdown
79 lines
2.7 KiB
Markdown
# actix-web-0002: WebSocket handshake protocol negotiation O(R×P) per upgrade request
|
||
|
||
**Severity:** MEDIUM
|
||
**CWE:** CWE-407 (Algorithmic Complexity — linear membership test in hot loop)
|
||
**Speedup:** >10x at R=20 client protocols × P=20 server protocols
|
||
**Target:** actix-web-actors (actix/actix-web)
|
||
**File:** `actix-web-actors/src/ws.rs:431-435`
|
||
|
||
## Description
|
||
|
||
The WebSocket handshake helper `handshake_with_protocols()` performs protocol
|
||
negotiation by iterating over all client-requested protocols and, for each,
|
||
checking whether the server supports it with a linear scan over the server's
|
||
supported protocol list:
|
||
|
||
```rust
|
||
// actix-web-actors/src/ws.rs:426-435
|
||
let protocol = req
|
||
.headers()
|
||
.get(&header::SEC_WEBSOCKET_PROTOCOL)
|
||
.and_then(|req_protocols| {
|
||
let req_protocols = req_protocols.to_str().ok()?;
|
||
req_protocols
|
||
.split(',')
|
||
.map(|req_p| req_p.trim())
|
||
.find(|req_p| protocols.iter().any(|p| p == req_p))
|
||
// ^^ O(P) scan per client protocol
|
||
});
|
||
```
|
||
|
||
`protocols` is `&[&str]` (a slice). `protocols.iter().any(|p| p == req_p)` is
|
||
O(P) per client protocol. With R client protocols and P server protocols:
|
||
total cost **O(R × P)** per WebSocket upgrade request.
|
||
|
||
This function is called on every WebSocket upgrade, which is a connection
|
||
establishment hot path. With many concurrent WebSocket upgrades (e.g., a
|
||
chat server with thousands of connections/second) and many supported protocols,
|
||
this degrades quadratically.
|
||
|
||
## Root Cause
|
||
|
||
`protocols` is passed as `&[&str]` and scanned linearly. Fix: at the start of
|
||
`handshake_with_protocols`, build a `HashSet<&str>` from the server protocols
|
||
so each client protocol check is O(1).
|
||
|
||
## Patch
|
||
|
||
```diff
|
||
--- a/actix-web-actors/src/ws.rs
|
||
+++ b/actix-web-actors/src/ws.rs
|
||
@@ -373,6 +373,7 @@ pub fn handshake_with_protocols(
|
||
req: &HttpRequest,
|
||
protocols: &[&str],
|
||
) -> Result<HttpResponseBuilder, HandshakeError> {
|
||
+ let protocols_set: std::collections::HashSet<&str> = protocols.iter().copied().collect();
|
||
// WebSocket accepts only GET
|
||
...
|
||
@@ -426,7 +427,7 @@ pub fn handshake_with_protocols(
|
||
req_protocols
|
||
.split(',')
|
||
.map(|req_p| req_p.trim())
|
||
- .find(|req_p| protocols.iter().any(|p| p == req_p))
|
||
+ .find(|req_p| protocols_set.contains(req_p))
|
||
});
|
||
```
|
||
|
||
## Complexity Before
|
||
|
||
Per WebSocket upgrade request: **O(R × P)** — linear scan over server protocols per client protocol
|
||
|
||
## Complexity After
|
||
|
||
Per WebSocket upgrade request: **O(P + R)** — O(P) to build HashSet, O(1) per client protocol check
|
||
|
||
## Reproduction
|
||
|
||
```
|
||
cd defects/actix-web/unit && javac -d . *.java && java -ea unit.ActixWebIntrospectionTest
|
||
```
|