mirror of
https://github.com/lexmount/moli.git
synced 2026-09-26 08:01:32 +00:00
refactor(websocket): remove unused CONNECT header assembly
Write proxy header values directly into the native request and delete the unused CONNECT string and append_proxy_connect_header helper. Native request configuration already validates these fields before connecting. Move rejection coverage to the native configuration entry point, covering CR, LF and NUL in User-Agent and Proxy-Authorization. Check User-Agent on the actual CONNECT request while retaining proxy credential isolation and WSS tunnel coverage. Validation: 96 package tests passed all 3 iterations; cargo fmt --all, strict workspace/all-targets/all-features Clippy and full nextest passed (17322 passed, 13 skipped).
This commit is contained in:
@@ -109,6 +109,23 @@ async fn native_upgrade_retains_socket_and_same_packet_empty_frame() {
|
||||
task.join().unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn native_proxy_headers_reject_cr_lf_and_nul_values() {
|
||||
let mut request = CurlWebSocketRequest::new("ws://example.test/socket".to_owned());
|
||||
request.proxy = Some("http://127.0.0.1:8080".to_owned());
|
||||
for name in ["User-Agent", "Proxy-Authorization"] {
|
||||
request.proxy_headers = vec![(name.to_owned(), "valid".to_owned())];
|
||||
assert!(super::request::configure(&request).is_ok());
|
||||
for value in ["good\rbad", "good\nbad", "good\0bad"] {
|
||||
request.proxy_headers[0].1 = value.to_owned();
|
||||
let error = super::request::configure(&request)
|
||||
.err()
|
||||
.expect("native proxy headers must reject control bytes");
|
||||
assert_eq!(error.to_string(), "invalid WebSocket request header");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn native_decoder_rejects_invalid_framing_before_delivering_payload() {
|
||||
let mut cases = Vec::new();
|
||||
|
||||
@@ -115,20 +115,3 @@ fn split_no_proxy_host_port(token: &str) -> (&str, Option<u16>) {
|
||||
_ => (token, None),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn append_proxy_connect_header(
|
||||
request: &mut String,
|
||||
name: &str,
|
||||
value: &str,
|
||||
) -> Result<(), String> {
|
||||
if value.bytes().any(|byte| matches!(byte, b'\r' | b'\n')) {
|
||||
return Err(format!(
|
||||
"invalid WebSocket proxy CONNECT header `{name}` contains a newline"
|
||||
));
|
||||
}
|
||||
request.push_str(name);
|
||||
request.push_str(": ");
|
||||
request.push_str(value);
|
||||
request.push_str("\r\n");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -6,7 +6,7 @@ use crate::{
|
||||
MAX_PENDING_WEBSOCKET_HANDSHAKES, MAX_WEBSOCKET_CONNECTIONS_PER_RUNTIME,
|
||||
acquire_limited_websocket_slot,
|
||||
},
|
||||
proxy::{append_proxy_connect_header, no_proxy_matches},
|
||||
proxy::no_proxy_matches,
|
||||
request::prepare_websocket_request,
|
||||
test_support::*,
|
||||
};
|
||||
@@ -314,17 +314,6 @@ fn websocket_proxy_url_explicit_empty_proxy_disables_env_fallback() {
|
||||
assert_eq!(proxy, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn websocket_proxy_connect_header_rejects_newline_values() {
|
||||
let mut request = String::new();
|
||||
assert!(append_proxy_connect_header(&mut request, "User-Agent", "Moli").is_ok());
|
||||
assert_eq!(request, "User-Agent: Moli\r\n");
|
||||
assert!(
|
||||
append_proxy_connect_header(&mut request, "Proxy-Authorization", "Bearer good\nbad")
|
||||
.is_err()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn websocket_request_preparation_applies_context_protocols_and_cookie() {
|
||||
let mut context = test_websocket_context();
|
||||
@@ -948,6 +937,7 @@ async fn websocket_transport_uses_explicit_http_proxy_connect_without_forwarding
|
||||
context.http_proxy = Some(proxy_url);
|
||||
context.http_no_proxy = Some(String::new());
|
||||
context.proxy_bearer_token = Some("proxy-token".to_owned());
|
||||
let user_agent = context.user_agent.clone();
|
||||
|
||||
let command_tx = spawn_connection(2, url.clone(), Vec::new(), context, event_tx);
|
||||
let proxy_request = timeout(Duration::from_secs(3), proxy_request_rx)
|
||||
@@ -978,6 +968,7 @@ async fn websocket_transport_uses_explicit_http_proxy_connect_without_forwarding
|
||||
proxy_request.contains("\r\nProxy-Authorization: Bearer proxy-token\r\n"),
|
||||
"proxy bearer token should be sent only on CONNECT: {proxy_request:?}"
|
||||
);
|
||||
assert!(proxy_request.contains(&format!("\r\nUser-Agent: {user_agent}\r\n")));
|
||||
assert_eq!(
|
||||
header_value(&headers, "origin").as_deref(),
|
||||
Some("https://example.com")
|
||||
|
||||
@@ -2,7 +2,7 @@ use crate::{
|
||||
ConnectOptions,
|
||||
handshake::{HandshakeResponse, parse_handshake_response, validate_handshake_response},
|
||||
headers::header_map_entries,
|
||||
proxy::{append_proxy_connect_header, websocket_proxy_url},
|
||||
proxy::websocket_proxy_url,
|
||||
request::PreparedWebSocketRequest,
|
||||
};
|
||||
use moli_curl::{
|
||||
@@ -36,17 +36,13 @@ pub(crate) async fn open_websocket_connection(
|
||||
// origins: https://websockets.spec.whatwg.org/#opening-handshake
|
||||
native.tls = context.tls.clone();
|
||||
if native.proxy.is_some() {
|
||||
let mut validated = String::new();
|
||||
append_proxy_connect_header(&mut validated, "User-Agent", &context.user_agent)?;
|
||||
native
|
||||
.proxy_headers
|
||||
.push(("User-Agent".to_owned(), context.user_agent.clone()));
|
||||
if let Some(token) = &context.proxy_bearer_token {
|
||||
let value = format!("Bearer {token}");
|
||||
append_proxy_connect_header(&mut validated, "Proxy-Authorization", &value)?;
|
||||
native
|
||||
.proxy_headers
|
||||
.push(("Proxy-Authorization".to_owned(), value));
|
||||
.push(("Proxy-Authorization".to_owned(), format!("Bearer {token}")));
|
||||
}
|
||||
} else {
|
||||
let url = &request.url;
|
||||
|
||||
Reference in New Issue
Block a user