fix(ssh): a typed port of 22 outranks an alias that sets its own

Quick connect merged the typed line over a matching `~/.ssh/config` alias
by asking whether the parsed port was 22. Port 22 is also what the parser
fills in when nothing named a port, so `myalias -p 22` against an alias
carrying `Port 2222` read as silence and connected to 2222. Checked
against the reference: `ssh -G -F cfg -p 22 myalias` reports port 22, the
command line outranking the config file.

The parser already knew — it tracks `Option<u16>` and only collapses it at
the end — so the answer is carried out as `port_given` rather than
recovered from a value that cannot hold it.

The merge itself moves out of the event handler into
`merge_typed_ssh_over_alias`, which is what let this be tested at all: the
rule is now stated in one place and covered for the typed `:22` spelling,
the silent case, an ordinary non-default port, and no alias at all.

The other `port == 22` comparisons are display code deciding whether to
print `:22`, which is correct and unchanged.
This commit is contained in:
l0ng-ai
2026-08-16 17:19:09 +08:00
parent 89426fb5c4
commit 32029f4733
+89 -21
View File
@@ -2757,23 +2757,8 @@ impl Tty7App {
fn open_typed_ssh_connect(&mut self, input: &str, window: &mut Window, cx: &mut Context<Self>) {
match parse_ssh_connect_input(input) {
Ok(parsed) => {
let (profile, proxy_jump) =
match ssh_config::resolve_alias_to_profile(&parsed.profile.host) {
Some(resolved) => {
let mut p = resolved.profile;
if !parsed.profile.user.is_empty() {
p.user = parsed.profile.user;
}
if parsed.profile.port != 22 {
p.port = parsed.profile.port;
}
if !parsed.profile.identity_files.is_empty() {
p.identity_files = parsed.profile.identity_files;
}
(p, parsed.proxy_jump.or(resolved.proxy_jump))
}
None => (parsed.profile, parsed.proxy_jump),
};
let resolved = ssh_config::resolve_alias_to_profile(&parsed.profile.host);
let (profile, proxy_jump) = merge_typed_ssh_over_alias(parsed, resolved);
let verify = cx.global::<Config>().verify_host_keys;
let spec = crate::ui::ssh_connect::native_spec_from_transient_profile(
&profile,
@@ -8054,6 +8039,15 @@ pub(crate) fn parse_ssh_option_words(input: &str) -> Result<Vec<String>, ()> {
pub(crate) struct ParsedSshConnect {
pub profile: crate::core::ssh_profile::SshProfile,
pub proxy_jump: Option<String>,
/// Whether the typed text named a port at all, either as `-p`/`-o Port=`
/// or as a `:port` on the target.
///
/// `profile.port` cannot answer this: it holds 22 both for text that asked
/// for 22 and for text that asked for nothing, and the two have to merge
/// differently against an alias from `~/.ssh/config` that sets its own
/// port. `ssh -p 22 myalias` reaches port 22 even when the alias says
/// 2222, because the command line outranks the config file.
pub port_given: bool,
}
pub(crate) fn parse_ssh_connect_input(input: &str) -> Result<ParsedSshConnect, String> {
@@ -8133,7 +8127,8 @@ pub(crate) fn parse_ssh_connect_input(input: &str) -> Result<ParsedSshConnect, S
let mut profile = SshProfile::new(qc.host.clone());
profile.host = qc.host;
profile.port = port.or(qc.port).unwrap_or(22);
let port = port.or(qc.port);
profile.port = port.unwrap_or(22);
if let Some(user) = user.or(qc.user) {
profile.user = user;
}
@@ -8142,9 +8137,37 @@ pub(crate) fn parse_ssh_connect_input(input: &str) -> Result<ParsedSshConnect, S
Ok(ParsedSshConnect {
profile,
proxy_jump: jump,
port_given: port.is_some(),
})
}
/// What the typed text and a matching `~/.ssh/config` alias add up to.
///
/// The alias supplies the ground, and anything the text actually named wins
/// over it — the order `ssh` itself uses, where the command line outranks the
/// config file. "Actually named" is the whole difficulty: an empty user and an
/// empty identity list say nothing, but a port of 22 is indistinguishable from
/// silence unless the parser is asked, which is what `port_given` is for.
pub(crate) fn merge_typed_ssh_over_alias(
parsed: ParsedSshConnect,
resolved: Option<crate::core::ssh_config::ResolvedAlias>,
) -> (crate::core::ssh_profile::SshProfile, Option<String>) {
let Some(resolved) = resolved else {
return (parsed.profile, parsed.proxy_jump);
};
let mut p = resolved.profile;
if !parsed.profile.user.is_empty() {
p.user = parsed.profile.user;
}
if parsed.port_given {
p.port = parsed.profile.port;
}
if !parsed.profile.identity_files.is_empty() {
p.identity_files = parsed.profile.identity_files;
}
(p, parsed.proxy_jump.or(resolved.proxy_jump))
}
fn ssh_short_flag(word: &str) -> Option<(char, String)> {
let rest = word.strip_prefix('-')?;
if rest.is_empty() || rest.starts_with('-') {
@@ -8404,9 +8427,9 @@ mod window_drag_tests {
mod tests {
use super::{
CloseReason, ForwardRoute, TERMINAL_MIN_W, TabAgentSession, clear_window_override_values,
close_prompt, join_shell_args, leaf_shares_the_window_daemon, mru_order, pane_free_for,
parse_ssh_connect_input, parse_ssh_option_words, side_panel_max, split_shell_args,
wd_path_saveable,
close_prompt, join_shell_args, leaf_shares_the_window_daemon, merge_typed_ssh_over_alias,
mru_order, pane_free_for, parse_ssh_connect_input, parse_ssh_option_words, side_panel_max,
split_shell_args, wd_path_saveable,
};
/// A teardown that was never delivered must not answer the way a teardown
@@ -8799,6 +8822,51 @@ mod tests {
assert_eq!(p.profile.port, 2200);
}
#[test]
fn a_typed_port_of_22_still_outranks_an_alias_on_another_port() {
use crate::core::ssh_config::ResolvedAlias;
use crate::core::ssh_profile::SshProfile;
// `ssh -G -p 22 myalias` reports port 22 even where the alias sets
// 2222, checked against the real ssh. Port 22 doubles as the default,
// so asking `profile.port != 22` read the explicit request as silence
// and connected to the alias's port instead.
let alias = || {
let mut a = SshProfile::new("myalias".to_string());
a.host = "example.com".to_string();
a.user = "alice".to_string();
a.port = 2222;
Some(ResolvedAlias {
profile: a,
proxy_jump: None,
})
};
let typed = parse_ssh_connect_input("myalias -p 22").unwrap();
assert!(typed.port_given);
let (merged, _) = merge_typed_ssh_over_alias(typed, alias());
assert_eq!(merged.port, 22, "a typed -p 22 was ignored");
assert_eq!(merged.user, "alice", "the alias still supplies the user");
// The `:port` spelling of the same request.
let typed = parse_ssh_connect_input("myalias:22").unwrap();
assert!(typed.port_given);
assert_eq!(merge_typed_ssh_over_alias(typed, alias()).0.port, 22);
// Saying nothing about the port still leaves the alias in charge.
let typed = parse_ssh_connect_input("myalias").unwrap();
assert!(!typed.port_given);
assert_eq!(merge_typed_ssh_over_alias(typed, alias()).0.port, 2222);
// And a typed port that is not 22 was never in doubt.
let typed = parse_ssh_connect_input("myalias -p 2200").unwrap();
assert_eq!(merge_typed_ssh_over_alias(typed, alias()).0.port, 2200);
// With no alias at all the typed line stands on its own.
let typed = parse_ssh_connect_input("myalias -p 22").unwrap();
assert_eq!(merge_typed_ssh_over_alias(typed, None).0.port, 22);
}
#[test]
fn rejects_bad_typed_connect_lines() {
assert!(parse_ssh_connect_input("ssh -p 2222").is_err());