fix: preserve query parameters in --cdp HTTP URLs (#982)
When --cdp is given an HTTP/HTTPS URL (e.g. http://host:5095?mode=Hello), resolve_cdp_url extracts host and port for CDP discovery but discards the query string. The discovered WebSocket URL therefore never includes the user's original query parameters, breaking relay servers that depend on them. Thread the original query string through discover_cdp_url and append it to the final WebSocket URL after host/port rewriting. WebSocket URLs (ws://, wss://) are already passed through unchanged and are unaffected. Fixes #977 Co-authored-by: ctate <366502+ctate@users.noreply.github.com>
This commit is contained in:
@@ -1324,12 +1324,13 @@ async fn resolve_cdp_url(input: &str) -> Result<String, String> {
|
|||||||
.host_str()
|
.host_str()
|
||||||
.ok_or_else(|| format!("No host in CDP URL: {}", input))?;
|
.ok_or_else(|| format!("No host in CDP URL: {}", input))?;
|
||||||
let port = parsed.port().unwrap_or(9222);
|
let port = parsed.port().unwrap_or(9222);
|
||||||
return discover_cdp_url(host, port).await;
|
let query = parsed.query().map(|q| q.to_string());
|
||||||
|
return discover_cdp_url(host, port, query.as_deref()).await;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Try as numeric port
|
// Try as numeric port
|
||||||
if let Ok(port) = input.parse::<u16>() {
|
if let Ok(port) = input.parse::<u16>() {
|
||||||
return discover_cdp_url("127.0.0.1", port).await;
|
return discover_cdp_url("127.0.0.1", port, None).await;
|
||||||
}
|
}
|
||||||
|
|
||||||
Err(format!(
|
Err(format!(
|
||||||
|
|||||||
@@ -514,7 +514,7 @@ pub async fn auto_connect_cdp() -> Result<String, String> {
|
|||||||
for dir in &user_data_dirs {
|
for dir in &user_data_dirs {
|
||||||
if let Some((port, ws_path)) = read_devtools_active_port(dir) {
|
if let Some((port, ws_path)) = read_devtools_active_port(dir) {
|
||||||
// Try HTTP endpoint first (pre-M144)
|
// Try HTTP endpoint first (pre-M144)
|
||||||
if let Ok(ws_url) = discover_cdp_url("127.0.0.1", port).await {
|
if let Ok(ws_url) = discover_cdp_url("127.0.0.1", port, None).await {
|
||||||
return Ok(ws_url);
|
return Ok(ws_url);
|
||||||
}
|
}
|
||||||
// M144+: direct WebSocket — verify the port is actually listening
|
// M144+: direct WebSocket — verify the port is actually listening
|
||||||
@@ -533,7 +533,7 @@ pub async fn auto_connect_cdp() -> Result<String, String> {
|
|||||||
|
|
||||||
// Fallback: probe common ports
|
// Fallback: probe common ports
|
||||||
for port in [9222u16, 9229] {
|
for port in [9222u16, 9229] {
|
||||||
if let Ok(ws_url) = discover_cdp_url("127.0.0.1", port).await {
|
if let Ok(ws_url) = discover_cdp_url("127.0.0.1", port, None).await {
|
||||||
return Ok(ws_url);
|
return Ok(ws_url);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -13,21 +13,30 @@ const DEFAULT_DISCOVERY_TIMEOUT: Duration = Duration::from_secs(2);
|
|||||||
/// Tries three methods in order: `/json/version`, `/json/list`, and a direct
|
/// Tries three methods in order: `/json/version`, `/json/list`, and a direct
|
||||||
/// WebSocket connection to `/devtools/browser`. The returned URL has its
|
/// WebSocket connection to `/devtools/browser`. The returned URL has its
|
||||||
/// host/port rewritten to match the requested target.
|
/// host/port rewritten to match the requested target.
|
||||||
pub async fn discover_cdp_url(host: &str, port: u16) -> Result<String, String> {
|
///
|
||||||
discover_cdp_url_with_timeout(host, port, DEFAULT_DISCOVERY_TIMEOUT).await
|
/// An optional `query` string (without the leading `?`) is appended to the
|
||||||
|
/// final WebSocket URL so that user-supplied URL parameters (e.g.
|
||||||
|
/// `?mode=Hello`) are forwarded to the remote endpoint.
|
||||||
|
pub async fn discover_cdp_url(
|
||||||
|
host: &str,
|
||||||
|
port: u16,
|
||||||
|
query: Option<&str>,
|
||||||
|
) -> Result<String, String> {
|
||||||
|
discover_cdp_url_with_timeout(host, port, query, DEFAULT_DISCOVERY_TIMEOUT).await
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Like [`discover_cdp_url`] but with a custom request timeout.
|
/// Like [`discover_cdp_url`] but with a custom request timeout.
|
||||||
pub async fn discover_cdp_url_with_timeout(
|
pub async fn discover_cdp_url_with_timeout(
|
||||||
host: &str,
|
host: &str,
|
||||||
port: u16,
|
port: u16,
|
||||||
|
query: Option<&str>,
|
||||||
timeout: Duration,
|
timeout: Duration,
|
||||||
) -> Result<String, String> {
|
) -> Result<String, String> {
|
||||||
// Primary: /json/version (standard path)
|
// Primary: /json/version (standard path)
|
||||||
let version_err = match fetch_cdp_info(host, port, timeout).await {
|
let version_err = match fetch_cdp_info(host, port, timeout).await {
|
||||||
Ok(info) => {
|
Ok(info) => {
|
||||||
if let Some(ws_url) = info.web_socket_debugger_url {
|
if let Some(ws_url) = info.web_socket_debugger_url {
|
||||||
return Ok(rewrite_ws_host(&ws_url, host, port));
|
return Ok(append_query(&rewrite_ws_host(&ws_url, host, port), query));
|
||||||
}
|
}
|
||||||
format!(
|
format!(
|
||||||
"No webSocketDebuggerUrl in /json/version at {}:{}",
|
"No webSocketDebuggerUrl in /json/version at {}:{}",
|
||||||
@@ -39,7 +48,7 @@ pub async fn discover_cdp_url_with_timeout(
|
|||||||
|
|
||||||
// Fallback: /json/list (returns target list; look for the browser target)
|
// Fallback: /json/list (returns target list; look for the browser target)
|
||||||
let list_err = match fetch_cdp_list(host, port, timeout).await {
|
let list_err = match fetch_cdp_list(host, port, timeout).await {
|
||||||
Ok(ws_url) => return Ok(rewrite_ws_host(&ws_url, host, port)),
|
Ok(ws_url) => return Ok(append_query(&rewrite_ws_host(&ws_url, host, port), query)),
|
||||||
Err(e) => e,
|
Err(e) => e,
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -47,7 +56,7 @@ pub async fn discover_cdp_url_with_timeout(
|
|||||||
// Chrome 136+ with UI-based remote debugging (chrome://inspect) exposes
|
// Chrome 136+ with UI-based remote debugging (chrome://inspect) exposes
|
||||||
// CDP over WebSocket but does not serve HTTP discovery endpoints.
|
// CDP over WebSocket but does not serve HTTP discovery endpoints.
|
||||||
match discover_cdp_ws(host, port, timeout).await {
|
match discover_cdp_ws(host, port, timeout).await {
|
||||||
Ok(ws_url) => Ok(ws_url),
|
Ok(ws_url) => Ok(append_query(&ws_url, query)),
|
||||||
Err(ws_err) => Err(format!(
|
Err(ws_err) => Err(format!(
|
||||||
"All CDP discovery methods failed for {}:{}: /json/version: {}; /json/list: {}; WebSocket: {}",
|
"All CDP discovery methods failed for {}:{}: /json/version: {}; /json/list: {}; WebSocket: {}",
|
||||||
host, port, version_err, list_err, ws_err
|
host, port, version_err, list_err, ws_err
|
||||||
@@ -94,6 +103,29 @@ fn rewrite_ws_host(ws_url: &str, host: &str, port: u16) -> String {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Append a query string to a URL, preserving any existing query parameters.
|
||||||
|
fn append_query(url: &str, query: Option<&str>) -> String {
|
||||||
|
match query {
|
||||||
|
Some(q) if !q.is_empty() => {
|
||||||
|
if let Ok(mut parsed) = url::Url::parse(url) {
|
||||||
|
{
|
||||||
|
let mut pairs = parsed.query_pairs_mut();
|
||||||
|
pairs.extend_pairs(url::form_urlencoded::parse(q.as_bytes()));
|
||||||
|
}
|
||||||
|
parsed.to_string()
|
||||||
|
} else {
|
||||||
|
// Fallback: raw string append
|
||||||
|
if url.contains('?') {
|
||||||
|
format!("{}&{}", url, q)
|
||||||
|
} else {
|
||||||
|
format!("{}?{}", url, q)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
_ => url.to_string(),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Fetch `/json/list` and extract the `webSocketDebuggerUrl` from the first
|
/// Fetch `/json/list` and extract the `webSocketDebuggerUrl` from the first
|
||||||
/// target with `type == "browser"`, or the first target if none has that type.
|
/// target with `type == "browser"`, or the first target if none has that type.
|
||||||
async fn fetch_cdp_list(host: &str, port: u16, timeout: Duration) -> Result<String, String> {
|
async fn fetch_cdp_list(host: &str, port: u16, timeout: Duration) -> Result<String, String> {
|
||||||
@@ -210,7 +242,7 @@ mod tests {
|
|||||||
.await;
|
.await;
|
||||||
});
|
});
|
||||||
|
|
||||||
let ws_url = discover_cdp_url("127.0.0.1", port).await.unwrap();
|
let ws_url = discover_cdp_url("127.0.0.1", port, None).await.unwrap();
|
||||||
assert_eq!(ws_url, format!("ws://127.0.0.1:{}/", port));
|
assert_eq!(ws_url, format!("ws://127.0.0.1:{}/", port));
|
||||||
server.await.unwrap();
|
server.await.unwrap();
|
||||||
}
|
}
|
||||||
@@ -224,7 +256,7 @@ mod tests {
|
|||||||
// /json/list and ws fallback both fail (server closes)
|
// /json/list and ws fallback both fail (server closes)
|
||||||
});
|
});
|
||||||
|
|
||||||
let err = discover_cdp_url("127.0.0.1", port).await.unwrap_err();
|
let err = discover_cdp_url("127.0.0.1", port, None).await.unwrap_err();
|
||||||
assert!(err.contains("Invalid /json/version response"));
|
assert!(err.contains("Invalid /json/version response"));
|
||||||
server.await.unwrap();
|
server.await.unwrap();
|
||||||
}
|
}
|
||||||
@@ -241,7 +273,7 @@ mod tests {
|
|||||||
).await;
|
).await;
|
||||||
});
|
});
|
||||||
|
|
||||||
let ws_url = discover_cdp_url("127.0.0.1", port).await.unwrap();
|
let ws_url = discover_cdp_url("127.0.0.1", port, None).await.unwrap();
|
||||||
assert!(ws_url.contains("/devtools/browser/abc"));
|
assert!(ws_url.contains("/devtools/browser/abc"));
|
||||||
assert!(ws_url.contains(&port.to_string()));
|
assert!(ws_url.contains(&port.to_string()));
|
||||||
server.await.unwrap();
|
server.await.unwrap();
|
||||||
@@ -271,7 +303,7 @@ mod tests {
|
|||||||
let _ = ws.close(None).await;
|
let _ = ws.close(None).await;
|
||||||
});
|
});
|
||||||
|
|
||||||
let ws_url = discover_cdp_url("127.0.0.1", port).await.unwrap();
|
let ws_url = discover_cdp_url("127.0.0.1", port, None).await.unwrap();
|
||||||
assert_eq!(ws_url, format!("ws://127.0.0.1:{}/devtools/browser", port));
|
assert_eq!(ws_url, format!("ws://127.0.0.1:{}/devtools/browser", port));
|
||||||
server.await.unwrap();
|
server.await.unwrap();
|
||||||
}
|
}
|
||||||
@@ -289,4 +321,67 @@ mod tests {
|
|||||||
let rewritten = rewrite_ws_host(original, "::1", 9222);
|
let rewritten = rewrite_ws_host(original, "::1", 9222);
|
||||||
assert_eq!(rewritten, "ws://[::1]:9222/devtools/browser/abc");
|
assert_eq!(rewritten, "ws://[::1]:9222/devtools/browser/abc");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn append_query_adds_params_to_url_without_query() {
|
||||||
|
let url = "ws://127.0.0.1:9222/devtools/browser/abc";
|
||||||
|
let result = append_query(url, Some("mode=Hello"));
|
||||||
|
assert_eq!(
|
||||||
|
result,
|
||||||
|
"ws://127.0.0.1:9222/devtools/browser/abc?mode=Hello"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn append_query_merges_with_existing_query() {
|
||||||
|
let url = "ws://127.0.0.1:9222/devtools/browser/abc?token=xyz";
|
||||||
|
let result = append_query(url, Some("mode=Hello"));
|
||||||
|
assert_eq!(
|
||||||
|
result,
|
||||||
|
"ws://127.0.0.1:9222/devtools/browser/abc?token=xyz&mode=Hello"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn append_query_noop_for_none() {
|
||||||
|
let url = "ws://127.0.0.1:9222/devtools/browser/abc";
|
||||||
|
let result = append_query(url, None);
|
||||||
|
assert_eq!(result, url);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn append_query_noop_for_empty() {
|
||||||
|
let url = "ws://127.0.0.1:9222/devtools/browser/abc";
|
||||||
|
let result = append_query(url, Some(""));
|
||||||
|
assert_eq!(result, url);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn append_query_handles_multiple_params() {
|
||||||
|
let url = "ws://127.0.0.1:9222/devtools/browser/abc";
|
||||||
|
let result = append_query(url, Some("mode=Hello&token=abc"));
|
||||||
|
assert_eq!(
|
||||||
|
result,
|
||||||
|
"ws://127.0.0.1:9222/devtools/browser/abc?mode=Hello&token=abc"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn discover_preserves_query_params() {
|
||||||
|
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
|
||||||
|
let port = listener.local_addr().unwrap().port();
|
||||||
|
let server = tokio::spawn(async move {
|
||||||
|
accept_http(
|
||||||
|
&listener,
|
||||||
|
&http_200(r#"{"webSocketDebuggerUrl":"ws://127.0.0.1:1234/"}"#),
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
});
|
||||||
|
|
||||||
|
let ws_url = discover_cdp_url("127.0.0.1", port, Some("mode=Hello"))
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(ws_url, format!("ws://127.0.0.1:{}/?mode=Hello", port));
|
||||||
|
server.await.unwrap();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -257,7 +257,9 @@ async fn wait_for_lightpanda_ready(
|
|||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
|
||||||
match discover_cdp_url_with_timeout("127.0.0.1", port, LIGHTPANDA_DISCOVERY_TIMEOUT).await {
|
match discover_cdp_url_with_timeout("127.0.0.1", port, None, LIGHTPANDA_DISCOVERY_TIMEOUT)
|
||||||
|
.await
|
||||||
|
{
|
||||||
Ok(ws_url) => return Ok(ws_url),
|
Ok(ws_url) => return Ok(ws_url),
|
||||||
Err(err) => last_probe_error = Some(err),
|
Err(err) => last_probe_error = Some(err),
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user