Skip to content

FIX: Stream Auth through protocol bug fixed #1463

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.

Sign up for GitHub

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

Open
Atlas1225 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

FIX: Stream Auth through protocol bug fixed #1463

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 12 additions & 0 deletions engine/src/workers/stream/stream.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,18 @@ async fn ws_handler(
) -> impl IntoResponse {
let module = module.clone();

let has_authorization_protocol = headers
.get("sec-websocket-protocol")
.and_then(|v| v.to_str().ok())
.map(|s| s.split(',').any(|p| p.trim() == "Authorization"))
.unwrap_or(false);
Comment on lines +77 to +81
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 11, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the file structure and the specific lines mentioned
head -n 100 engine/src/workers/stream/stream.rs | tail -n 50

Repository: iii-hq/iii

Length of output: 1505

🏁 Script executed:

# Check the full imports to understand what HeaderMap is being used
head -n 30 engine/src/workers/stream/stream.rs

Repository: iii-hq/iii

Length of output: 949

🏁 Script executed:

# Look at the context around lines 77-81 more carefully
sed -n '70,95p' engine/src/workers/stream/stream.rs

Repository: iii-hq/iii

Length of output: 890

🏁 Script executed:

# Search for HeaderMap imports and types
rg "use.*HeaderMap\|use.*headers" engine/src/workers/stream/stream.rs

Repository: iii-hq/iii

Length of output: 36

🏁 Script executed:

# Check if there are any tests for this specific logic
rg -A 10 "sec-websocket-protocol\|SecWebSocketProtocol" engine/src/workers/stream/

Repository: iii-hq/iii

Length of output: 36

🏁 Script executed:

# Check if axum's HeaderMap has get_all method and what it supports
rg "HeaderMap" engine/src/workers/stream/stream.rs -A 5 -B 5

Repository: iii-hq/iii

Length of output: 559

🏁 Script executed:

# Look for Cargo.toml to see axum version
fd Cargo.toml engine/ | head -5 | xargs head -n 50

Repository: iii-hq/iii

Length of output: 1106

🏁 Script executed:

web_search
axum http HeaderMap get_all method API

Repository: iii-hq/iii

Length of output: 147

🏁 Script executed:

web_search
HTTP Sec-WebSocket-Protocol header multiple instances RFC 6455

Repository: iii-hq/iii

Length of output: 147

🏁 Script executed:

# Check if there are any SDK or documentation files that document this protocol behavior
find . -type f \( -name "*.md" -o -name "*.rs" \) | xargs rg -l "Sec-WebSocket-Protocol|sec-websocket-protocol" | grep -E "sdk/|docs/" | head -10

Repository: iii-hq/iii

Length of output: 36

🏁 Script executed:

# Check the actual axum version in dependencies
rg "^axum" engine/Cargo.toml

Repository: iii-hq/iii

Length of output: 109

🌐 Web query:

axum 0.8 HeaderMap get_all method documentation

💡 Result:

In axum 0.8, HeaderMap is re-exported from the http crate (version 1.0+). The get_all method returns a view over all values associated with a given header key without allocations. Signature: pub fn get_all(&self, key: K) -> GetAll<'_, T> where K: AsHeaderName Description: Returns a view of all values associated with a key. The returned GetAll allows iterating the values in insertion order. Returns None if no values are associated with the key. Example: let mut map = HeaderMap::new; map.insert(HOST, "hello".parse.unwrap); map.append(HOST, "goodbye".parse.unwrap); let view = map.get_all("host"); let mut iter = view.iter; assert_eq!(&"hello", iter.next.unwrap); assert_eq!(&"goodbye", iter.next.unwrap); assert!(iter.next.is_none); GetAll provides iter which returns ValueIter<'a, T>, an iterator over &'a T in insertion order. This API is unchanged from previous versions and matches the standard http::header::HeaderMap documentation.

Citations:

🌐 Web query:

HTTP RFC 6455 Sec-WebSocket-Protocol header multiple values repeated header

💡 Result:

In RFC 6455 (The WebSocket Protocol), the Sec-WebSocket-Protocol header handles multiple subprotocol values differently for client requests and server responses. For client requests (Sec-WebSocket-Protocol-Client): The ABNF is 1#token, allowing multiple values either as a comma-separated list in a single header (e.g., Sec-WebSocket-Protocol: chat, superchat) or as repeated headers (e.g., Sec-WebSocket-Protocol: chat Sec-WebSocket-Protocol: superchat). This lists subprotocols in order of preference. The server selects at most one from this list. For server responses (Sec-WebSocket-Protocol-Server): The ABNF is token (a single token), so only one value is permitted—exactly one subprotocol selected from the client's list, sent in a single header instance (e.g., Sec-WebSocket-Protocol: chat). Sending multiple values or repeated headers is invalid and causes handshake failure, as confirmed by implementations and docs stating "the server can't send more than one Sec-WebSocket-Protocol header" and "server must return exactly one value". This contrasts with Sec-WebSocket-Extensions, which explicitly allows multiple instances or comma lists for extensions. HTTP/1.1 semantics support repeated headers for list-valued fields, but WebSocket restricts the server response to a single token. No updates in later RFCs (e.g., 7936, 8441) change this behavior.

Citations:

🏁 Script executed:

# Check the dependencies to see what http crate version is used
rg "^http\s*=" engine/Cargo.toml

Repository: iii-hq/iii

Length of output: 36

🏁 Script executed:

# Look for any existing patterns in the codebase of handling multiple headers
rg "get_all\|iter\(\)" engine/src/ | grep -i header | head -20

Repository: iii-hq/iii

Length of output: 36

🏁 Script executed:

# Check for cargo fmt compliance by looking at the file formatting
head -n 90 engine/src/workers/stream/stream.rs | tail -n 20

Repository: iii-hq/iii

Length of output: 642

🏁 Script executed:

# Verify cargo fmt compliance - check if file passes formatting
cd engine && cargo fmt --check -- src/workers/stream/stream.rs 2>&1 || true

Repository: iii-hq/iii

Length of output: 300

🏁 Script executed:

# Search more broadly for SDK changes or documentation that might reference WebSocket protocol handling
find . -type f -name "*.rs" -path "*/sdk/*" | xargs rg -l "protocol\|subprotocol" 2>/dev/null | head -5

Repository: iii-hq/iii

Length of output: 36

🏁 Script executed:

# Check if there are framework-level references to WebSocket subprotocols
find . -type f \( -name "*.md" -o -name "*.rs" \) -path "*/frameworks/*" | xargs rg -l "WebSocket\|subprotocol" 2>/dev/null | head -5

Repository: iii-hq/iii

Length of output: 36

🏁 Script executed:

# Check if there are any WebSocket-related tests or integration tests that might show expected behavior
find . -type f -name "*.rs" | xargs rg -l "WebSocketUpgrade\|ws_handler\|protocols" | grep test | head -5

Repository: iii-hq/iii

Length of output: 36

🏁 Script executed:

# Look for any protocol-related documentation or comments in the codebase
rg "Authorization.*protocol\|protocol.*Authorization" engine/src/ -B 2 -A 2

Repository: iii-hq/iii

Length of output: 36

Handle all Sec-WebSocket-Protocol header instances, not just one.

RFC 6455 allows clients to send Sec-WebSocket-Protocol as either comma-separated values or repeated header instances. The current code using headers.get() retrieves only the first header value and will miss the Authorization protocol if it appears in a subsequent repeated header, causing protocol negotiation to fail.

Use headers.get_all() to iterate over all header instances:

Proposed fix 
-    let has_authorization_protocol = headers
-        .get("sec-websocket-protocol")
-        .and_then(|v| v.to_str().ok())
-        .map(|s| s.split(',').any(|p| p.trim() == "Authorization"))
-        .unwrap_or(false);
+    let has_authorization_protocol = headers
+        .get_all("sec-websocket-protocol")
+        .iter()
+        .filter_map(|v| v.to_str().ok())
+        .flat_map(|s| s.split(','))
+        .any(|p| p.trim() == "Authorization");
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let has_authorization_protocol = headers
.get("sec-websocket-protocol")
.and_then(|v| v.to_str().ok())
.map(|s| s.split(',').any(|p| p.trim() == "Authorization"))
.unwrap_or(false);
let has_authorization_protocol = headers
.get_all("sec-websocket-protocol")
.iter()
.filter_map(|v| v.to_str().ok())
.flat_map(|s| s.split(','))
.any(|p| p.trim() == "Authorization");
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@engine/src/workers/stream/stream.rs` around lines 77 - 81, The code that sets
has_authorization_protocol currently uses headers.get(...) which only returns
the first Sec-WebSocket-Protocol header instance; change the logic to iterate
over headers.get_all("sec-websocket-protocol"), for each header value try
to_str().ok(), split by ',' and trim each token, and set
has_authorization_protocol = true if any token equals "Authorization"
(short-circuiting once found). Update the usage around the
has_authorization_protocol variable to rely on this aggregated check so repeated
header instances or comma-separated lists are both handled.


let ws = if has_authorization_protocol {
ws.protocols(["Authorization"])
} else {
ws
};

if let Some(auth_function) = module.auth_function.clone() {
let engine = module.engine.clone();
let input = StreamAuthInput {
Expand Down
Loading