diff --git a/moli-curl/src/websocket/tests.rs b/moli-curl/src/websocket/tests.rs index 9e622bfcc..dc38a0ecb 100644 --- a/moli-curl/src/websocket/tests.rs +++ b/moli-curl/src/websocket/tests.rs @@ -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(); diff --git a/moli-websocket/src/proxy.rs b/moli-websocket/src/proxy.rs index 60fd1f2c8..cdad371f6 100644 --- a/moli-websocket/src/proxy.rs +++ b/moli-websocket/src/proxy.rs @@ -115,20 +115,3 @@ fn split_no_proxy_host_port(token: &str) -> (&str, Option) { _ => (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(()) -} diff --git a/moli-websocket/src/tests.rs b/moli-websocket/src/tests.rs index 63533f55a..b16a45032 100644 --- a/moli-websocket/src/tests.rs +++ b/moli-websocket/src/tests.rs @@ -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") diff --git a/moli-websocket/src/transport.rs b/moli-websocket/src/transport.rs index df9b07095..6a1d708c1 100644 --- a/moli-websocket/src/transport.rs +++ b/moli-websocket/src/transport.rs @@ -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;