From 808e35d03f542177845100d43f0d41c30c0dda38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 11 Jun 2026 17:36:42 +0900 Subject: [PATCH] fix(jetsocat): validate JMUX listener destination at CLI parse time Previously, a `tcp-listen:///` argument without a port (e.g. a missing `:22`) was accepted at startup, but every incoming connection was then silently dropped with only a debug-level log. The tunnel appeared to work yet forwarded nothing. The listener destination is now parsed during CLI argument parsing, so a malformed destination fails immediately and visibly with a clear error, before any listener is bound. --- crates/jmux-proto/src/lib.rs | 28 ++++++++++++++-------------- jetsocat/src/listener.rs | 29 +++++++++++++---------------- jetsocat/src/main.rs | 7 +++++-- testsuite/tests/cli/jetsocat.rs | 26 ++++++++++++++++++++++++++ 4 files changed, 58 insertions(+), 32 deletions(-) diff --git a/crates/jmux-proto/src/lib.rs b/crates/jmux-proto/src/lib.rs index a093ce3c8..645d5f515 100644 --- a/crates/jmux-proto/src/lib.rs +++ b/crates/jmux-proto/src/lib.rs @@ -85,27 +85,27 @@ impl DestinationUrl { let scheme = &s[..scheme_end_idx]; let rest = &s[scheme_end_idx + "://".len()..]; - let host_end_idx = rest.rfind(':').ok_or_else(|| Error::InvalidDestinationUrl { - value: s.to_owned(), + Self::with_scheme(scheme, rest) + } + + /// Builds a destination URL from a `:` authority and an explicit scheme. + /// + /// Useful when the scheme is known from context (e.g.: a TCP listener) and only the + /// `:` part comes from user input. + pub fn with_scheme(scheme: &str, host_port: &str) -> Result { + let host_end_idx = host_port.rfind(':').ok_or_else(|| Error::InvalidDestinationUrl { + value: host_port.to_owned(), reason: "port is missing", })?; - let host = &rest[..host_end_idx]; - let port = &rest[host_end_idx + 1..]; + let host = &host_port[..host_end_idx]; + let port = &host_port[host_end_idx + 1..]; let port = port.parse().map_err(|_| Error::InvalidDestinationUrl { - value: s.to_owned(), + value: host_port.to_owned(), reason: "bad port", })?; - let scheme = SmolStr::new(scheme); - let host = SmolStr::new(host); - let inner = SmolStr::new(s); - Ok(Self { - inner, - scheme, - host, - port, - }) + Ok(Self::new(scheme, host, port)) } pub fn as_str(&self) -> &str { diff --git a/jetsocat/src/listener.rs b/jetsocat/src/listener.rs index bee2953e0..2b96d998f 100644 --- a/jetsocat/src/listener.rs +++ b/jetsocat/src/listener.rs @@ -11,15 +11,20 @@ use tracing::Instrument as _; #[derive(Debug, Clone)] pub enum ListenerMode { - Tcp { bind_addr: String, destination_url: String }, - Http { bind_addr: String }, - Socks5 { bind_addr: String }, + Tcp { + bind_addr: String, + destination_url: DestinationUrl, + }, + Http { + bind_addr: String, + }, + Socks5 { + bind_addr: String, + }, } #[instrument(skip(api_request_tx))] -pub async fn tcp_listener_task(api_request_tx: ApiRequestSender, bind_addr: String, destination_url: String) { - let destination_url = format!("tcp://{destination_url}"); - +pub async fn tcp_listener_task(api_request_tx: ApiRequestSender, bind_addr: String, destination_url: DestinationUrl) { let processor = |stream, addr| { let api_request_tx = api_request_tx.clone(); let destination_url = destination_url.clone(); @@ -28,14 +33,6 @@ pub async fn tcp_listener_task(api_request_tx: ApiRequestSender, bind_addr: Stri async move { debug!("Got request"); - let destination_url = match DestinationUrl::parse_str(&destination_url) { - Ok(url) => url, - Err(error) => { - debug!(%error, "Bad request"); - return; - } - }; - let (sender, receiver) = oneshot::channel(); match api_request_tx @@ -63,10 +60,10 @@ pub async fn tcp_listener_task(api_request_tx: ApiRequestSender, bind_addr: Stri .await; } Ok(JmuxApiResponse::Failure { id, reason_code }) => { - debug!(%id, %reason_code, "Channel failure"); + warn!(%id, %reason_code, "Couldn’t open JMUX channel for incoming connection"); } Err(error) => { - debug!(%error, "Couldn't receive API response"); + warn!(%error, "Couldn’t receive JMUX API response"); } } } diff --git a/jetsocat/src/main.rs b/jetsocat/src/main.rs index 6f0c25bb7..ba0ff06fe 100644 --- a/jetsocat/src/main.rs +++ b/jetsocat/src/main.rs @@ -39,7 +39,7 @@ use jetsocat::DoctorOutputFormat; use jetsocat::listener::ListenerMode; use jetsocat::pipe::PipeMode; use jetsocat::proxy::{ProxyConfig, ProxyType, detect_proxy}; -use jmux_proxy::JmuxConfig; +use jmux_proxy::{DestinationUrl, JmuxConfig}; use seahorse::{App, Command, Context, Flag, FlagType}; use tokio::runtime; @@ -894,9 +894,12 @@ fn parse_listener_mode(arg: &str) -> anyhow::Result { let bind_addr = it.next().context("binding address is missing")?; let destination_url = it.next().context("destination URL is missing")?; + let destination_url = + DestinationUrl::with_scheme("tcp", destination_url).context("parse destination URL")?; + Ok(ListenerMode::Tcp { bind_addr: bind_addr.to_owned(), - destination_url: destination_url.to_owned(), + destination_url, }) } "socks5-listen" => Ok(ListenerMode::Socks5 { diff --git a/testsuite/tests/cli/jetsocat.rs b/testsuite/tests/cli/jetsocat.rs index 61d81a45e..571a25fc2 100644 --- a/testsuite/tests/cli/jetsocat.rs +++ b/testsuite/tests/cli/jetsocat.rs @@ -906,6 +906,32 @@ fn forward_negative_repeat_count() { ); } +/// A TCP listener destination without a port is a client-side argument error and must be +/// reported clearly during CLI parsing, before any listener is bound. +#[test] +fn jmux_proxy_listener_missing_destination_port() { + let output = jetsocat_assert_cmd() + .args([ + "jmux-proxy", + "stdio", + "tcp-listen://127.0.0.1:60605/vdownsrv-test1.downhill.loc", + ]) + .assert() + .failure() + .code(1); + + assert_stderr_eq( + &output, + expect![[r#" + Bad : `tcp-listen://127.0.0.1:60605/vdownsrv-test1.downhill.loc` + + Caused by: + 0: parse destination URL + 1: invalid destination URL `vdownsrv-test1.downhill.loc`: port is missing + "#]], + ); +} + #[rstest] #[tokio::test] async fn mcp_proxy_smoke_test(#[values(true, false)] http_transport: bool) {