fix(settings): refuse a half-filled SSH profile instead of saving it

The SSH profile form saved whatever was in it. An empty host wrote a
profile that renders as a blank row in the host list and hands
`TcpStream::connect` an empty name — and Connect had no gate at all, so
the first thing the user heard about it was a DNS error naming nothing.
A jump host with a typo in it resolved to `None` and saved as a direct
connection, with the field blank the next time the form opened. And
`parse_host_port` was `parse().unwrap_or(0)`, so `proxy.example.com`,
`proxy.example.com:` and `proxy.example.com:88O` all saved a proxy on
port 0, which the socket layer got to explain (#492, #493, #494).

The rules now live in `validate_ssh_draft`, a plain function over a
plain-String snapshot of the form, which returns both the profile the
form would save and what is wrong with it. Both, always: the Escape
prompt asks whether the form differs from the config, and handing back
only the errors would make a brand-new invalid profile compare equal to
the nothing on disk — Escape would throw the typing away without asking.

Only the host is required. A name is not, because the list already falls
back to the address and every host imported from ~/.ssh/config arrives
without one. A blank port still means 22, but a non-empty one has to be
a port, so "0", "abc" and "70000" are refused rather than saved as
written or quietly rewritten. A proxy address with no port takes the
scheme's default (1080 / 8080) and `host_port_text` writes that back
into the field, so the number it picked is visible; a colon with nothing
usable after it is an error. `map_proxy` also stops treating port 0 as a
proxy, because configs written before this are already on disk.

Each complaint prints under the field it is about, and Save and Connect
are disabled while any of them stands; a section holding one unfolds so
the disabled button always has a visible reason. The "needs a host" line
waits until the name/host/port/user group has something in it — every
field notifies per keystroke, so otherwise a new host would be told off
before anyone had typed a character. Consequence: on a pristine new form
Save is now disabled where it used to be enabled.

src/ui/i18n/en.rs

@@ -229,6 +229,8 @@ pub fn translate_en(key: L10nKey) -> &'static str {
         L10nKey::SettingsNameDesc => "A label for this connection.",
         L10nKey::SettingsHost => "Host",
         L10nKey::SettingsHostDesc => "Hostname or IP address.",
+        L10nKey::SettingsHostRequired => "Needs a host — won't be saved.",
+        L10nKey::SettingsPortInvalid => "Port must be 1-65535 — blank means 22.",
         L10nKey::SettingsUser => "User",
         L10nKey::SettingsUserDesc => "Login user (blank = resolve at connect).",
         L10nKey::SettingsAuth => "Auth",
@@ -242,6 +244,8 @@ pub fn translate_en(key: L10nKey) -> &'static str {
         L10nKey::SettingsJumpHostDesc => {
             "Name of another profile to tunnel through (blank = direct)."
         }
+        L10nKey::SettingsJumpHostUnknown => "No host profile named {jump_name} — won't be saved.",
+        L10nKey::SettingsJumpHostSelf => "A host can't be its own jump host — won't be saved.",
         L10nKey::SettingsNoneSummary => "(none)",
         L10nKey::SettingsPortForwarding => "Port forwarding",
         L10nKey::SettingsRulesOpenedWithConnection => "1 rule, opened with the connection",
@@ -274,6 +278,9 @@ pub fn translate_en(key: L10nKey) -> &'static str {
         L10nKey::SettingsSocks5ProxyDesc => "host:port (blank = none).",
         L10nKey::SettingsHttpProxy => "HTTP proxy",
         L10nKey::SettingsHttpProxyDesc => "host:port (blank = none).",
+        L10nKey::SettingsProxyPortInvalid => {
+            "Port must be 1-65535 — the host on its own takes the default port."
+        }
         L10nKey::SettingsKexAlgorithms => "KEX algorithms",
         L10nKey::SettingsKexAlgorithmsDesc => "Comma-separated (blank = library default).",
         L10nKey::SettingsCiphers => "Ciphers",

src/ui/i18n/ja.rs

@@ -226,6 +226,8 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> {
         L10nKey::SettingsNameDesc => "この接続の表示名",
         L10nKey::SettingsHost => "ホスト名",
         L10nKey::SettingsHostDesc => "ホスト名または IP アドレス",
+        L10nKey::SettingsHostRequired => "ホスト名が必要です — 保存されません",
+        L10nKey::SettingsPortInvalid => "ポートは 1-65535 の範囲です — 空欄なら 22 です",
         L10nKey::SettingsUser => "ユーザー名",
         L10nKey::SettingsUserDesc => "ログインユーザー (空欄 = 接続時に解決)",
         L10nKey::SettingsAuth => "認証方式",
@@ -239,6 +241,12 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> {
         L10nKey::SettingsJumpHostDesc => {
             "トンネリングに使用する別のプロファイル名 (空欄 = 直接接続)"
         }
+        L10nKey::SettingsJumpHostUnknown => {
+            "{jump_name} という名前のホストプロファイルはありません — 保存されません"
+        }
+        L10nKey::SettingsJumpHostSelf => {
+            "ホストを自分自身のジャンプホストにはできません — 保存されません"
+        }
         L10nKey::SettingsNoneSummary => "(なし)",
         L10nKey::SettingsPortForwarding => "ポートフォワーディング",
         L10nKey::SettingsRulesOpenedWithConnection => "接続と同時に開くルール 1 件",
@@ -271,6 +279,9 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> {
         L10nKey::SettingsSocks5ProxyDesc => "host:port（空欄 = なし）",
         L10nKey::SettingsHttpProxy => "HTTP プロキシ",
         L10nKey::SettingsHttpProxyDesc => "host:port（空欄 = なし）",
+        L10nKey::SettingsProxyPortInvalid => {
+            "ポートは 1-65535 の範囲です — ホストだけならデフォルトポートを使います"
+        }
         L10nKey::SettingsKexAlgorithms => "KEX アルゴリズム",
         L10nKey::SettingsKexAlgorithmsDesc => "カンマ区切り（空欄 = ライブラリのデフォルト）",
         L10nKey::SettingsCiphers => "暗号方式",

src/ui/i18n/mod.rs

@@ -244,6 +244,8 @@ l10n_keys! {
     SettingsNameDesc,
     SettingsHost,
     SettingsHostDesc,
+    SettingsHostRequired,
+    SettingsPortInvalid,
     SettingsUser,
     SettingsUserDesc,
     SettingsAuth,
@@ -255,6 +257,8 @@ l10n_keys! {
     SettingsAuthMode2Fa,
     SettingsJumpHost,
     SettingsJumpHostDesc,
+    SettingsJumpHostUnknown,
+    SettingsJumpHostSelf,
     SettingsNoneSummary,
     SettingsNoneLower,
     SettingsPortForwarding,
@@ -284,6 +288,7 @@ l10n_keys! {
     SettingsSocks5ProxyDesc,
     SettingsHttpProxy,
     SettingsHttpProxyDesc,
+    SettingsProxyPortInvalid,
     SettingsKexAlgorithms,
     SettingsKexAlgorithmsDesc,
     SettingsCiphers,

src/ui/i18n/zh.rs

@@ -204,6 +204,8 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> {
         L10nKey::SettingsNameDesc => "此连接的标签。",
         L10nKey::SettingsHost => "主机",
         L10nKey::SettingsHostDesc => "主机名或 IP 地址。",
+        L10nKey::SettingsHostRequired => "需要填写主机——不会被保存。",
+        L10nKey::SettingsPortInvalid => "端口必须在 1-65535 之间——留空表示 22。",
         L10nKey::SettingsUser => "用户",
         L10nKey::SettingsUserDesc => "登录用户（留空表示连接时解析）。",
         L10nKey::SettingsAuth => "认证",
@@ -215,6 +217,8 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> {
         L10nKey::SettingsAuthMode2Fa => "2FA",
         L10nKey::SettingsJumpHost => "跳板主机",
         L10nKey::SettingsJumpHostDesc => "用于中转的另一个主机配置的名称（留空 = 直连）。",
+        L10nKey::SettingsJumpHostUnknown => "没有名为 {jump_name} 的主机配置——不会被保存。",
+        L10nKey::SettingsJumpHostSelf => "主机不能把自己当作跳板——不会被保存。",
         L10nKey::SettingsNoneSummary => "（无）",
         L10nKey::SettingsPortForwarding => "端口转发",
         L10nKey::SettingsRulesOpenedWithConnection => "1 条规则，随连接打开",
@@ -243,6 +247,7 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> {
         L10nKey::SettingsSocks5ProxyDesc => "host:port（留空 = 无）。",
         L10nKey::SettingsHttpProxy => "HTTP 代理",
         L10nKey::SettingsHttpProxyDesc => "host:port（留空 = 无）。",
+        L10nKey::SettingsProxyPortInvalid => "端口必须在 1-65535 之间——只写主机则使用默认端口。",
         L10nKey::SettingsKexAlgorithms => "KEX 算法",
         L10nKey::SettingsKexAlgorithmsDesc => "逗号分隔（留空 = 库默认值）。",
         L10nKey::SettingsCiphers => "加密算法",

src/ui/settings.rs

@@ -840,6 +840,19 @@ pub(crate) struct SshProfileForm {
     _subs: Vec<Subscription>,
 }
 
+impl SshProfileForm {
+    /// Whether the group that identifies the host — name, host, port, user —
+    /// is still untouched. Every field notifies on change, so the form
+    /// re-renders on each keystroke; without this a new host would be told it
+    /// needs a host before anyone had the chance to type one. Same deal the
+    /// forward rows strike with `ForwardRuleForm::is_blank`.
+    fn core_is_blank(&self, cx: &App) -> bool {
+        [&self.name, &self.host, &self.port, &self.user]
+            .iter()
+            .all(|e| e.read(cx).value().trim().is_empty())
+    }
+}
+
 pub(crate) struct ForwardRuleForm {
     pub(crate) kind: ForwardKind,
     pub(crate) bind_host: Entity<InputState>,
@@ -911,14 +924,39 @@ pub(crate) fn humanize_action(action: &str) -> String {
     out
 }
 
-fn parse_host_port(s: &str) -> Option<HostPort> {
+/// What a blank port field means. The same number `SshProfile`'s serde default
+/// writes for a config that never mentioned a port, which is why leaving the
+/// field empty has to stay legal: every host imported from `~/.ssh/config`
+/// leaves it empty.
+const DEFAULT_SSH_PORT: u16 = 22;
+
+/// The port each proxy scheme listens on when the field names only a host.
+const DEFAULT_SOCKS_PORT: u16 = 1080;
+const DEFAULT_HTTP_PROXY_PORT: u16 = 8080;
+
+/// A port as a form field spells it. Nothing here accepts 0: every port in a
+/// profile is one something has to connect to, and no listener answers on 0.
+fn parse_port(s: &str) -> Option<u16> {
+    s.trim().parse::<u16>().ok().filter(|p| *p > 0)
+}
+
+/// A proxy address as the form spells it: blank is "no proxy", a bare host
+/// takes the scheme's default port, and anything else has to carry a port that
+/// exists. This used to be `parse().unwrap_or(0)`, so `proxy.example.com:88O`
+/// saved a proxy on port 0 and the failure surfaced far away, in the socket
+/// layer. The default port is not a secret either — `host_port_text` writes it
+/// back into the field the next time the form opens.
+fn parse_host_port_checked(s: &str, default_port: u16) -> Result<Option<HostPort>, SshFieldError> {
     let s = s.trim();
     if s.is_empty() {
-        return None;
+        return Ok(None);
     }
     match s.rsplit_once(':') {
-        Some((h, p)) => Some(HostPort::new(h.trim(), p.trim().parse().unwrap_or(0))),
-        None => Some(HostPort::new(s, 0)),
+        Some((h, p)) => match parse_port(p) {
+            Some(port) => Ok(Some(HostPort::new(h.trim(), port))),
+            None => Err(SshFieldError::ProxyPortRange),
+        },
+        None => Ok(Some(HostPort::new(s, default_port))),
     }
 }
 
@@ -944,6 +982,207 @@ fn split_lines(s: &str) -> Vec<String> {
         .collect()
 }
 
+/// Why one field of the SSH profile form cannot be saved. A value rather than
+/// a finished sentence, so the rules stay a plain function a test can call —
+/// the wording, and the locale it is written in, belong to the render pass.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub(crate) enum SshFieldError {
+    /// Nothing to connect to. Saved anyway, the profile used to render as an
+    /// empty row in the host list and hand `TcpStream::connect` an empty name.
+    HostMissing,
+    /// A port field that is neither blank nor a port.
+    PortRange,
+    /// The same, for the port half of a proxy address.
+    ProxyPortRange,
+    /// The jump field names a profile no host list has.
+    JumpUnknown(String),
+    /// The jump field names the profile being edited.
+    JumpIsSelf,
+}
+
+impl SshFieldError {
+    fn message(&self) -> String {
+        match self {
+            Self::HostMissing => t(L10nKey::SettingsHostRequired).to_string(),
+            Self::PortRange => t(L10nKey::SettingsPortInvalid).to_string(),
+            Self::ProxyPortRange => t(L10nKey::SettingsProxyPortInvalid).to_string(),
+            Self::JumpUnknown(name) => {
+                t_fmt(L10nKey::SettingsJumpHostUnknown, &[("jump_name", name)])
+            }
+            Self::JumpIsSelf => t(L10nKey::SettingsJumpHostSelf).to_string(),
+        }
+    }
+}
+
+/// What the form has to fix before it can be saved, one slot per field so each
+/// complaint can be printed under the control it is about.
+#[derive(Debug, Clone, Default, PartialEq, Eq)]
+pub(crate) struct SshFormErrors {
+    host: Option<SshFieldError>,
+    port: Option<SshFieldError>,
+    jump: Option<SshFieldError>,
+    socks: Option<SshFieldError>,
+    http: Option<SshFieldError>,
+}
+
+impl SshFormErrors {
+    fn is_empty(&self) -> bool {
+        self.host.is_none()
+            && self.port.is_none()
+            && self.jump.is_none()
+            && self.socks.is_none()
+            && self.http.is_none()
+    }
+}
+
+/// The SSH profile form as plain text, lifted out of the `InputState` entities
+/// it lives in. The rules that turn it into a profile are the part worth
+/// testing, and a GPUI entity is not something a unit test can hand them, so
+/// the window layer's job stops at reading the strings out.
+#[derive(Debug, Clone, Default)]
+pub(crate) struct SshFormDraft {
+    id: Uuid,
+    name: String,
+    group: Option<String>,
+    host: String,
+    port: String,
+    user: String,
+    jump: String,
+    proxy_command: String,
+    socks: String,
+    http: String,
+    auth: AuthMode,
+    identity_files: String,
+    agent_forward: bool,
+    credential_ref: Option<CredentialRef>,
+    forwards: Vec<ForwardRule>,
+    keepalive_interval: String,
+    keepalive_count: String,
+    connect_timeout: String,
+    warn_on_close: Option<bool>,
+    skip_banner: bool,
+    shell_integration: bool,
+    login_scripts: String,
+    x11: bool,
+    kex: String,
+    cipher: String,
+    mac: String,
+    hostkey: String,
+    compression: String,
+    verify_host_keys: Option<bool>,
+}
+
+/// The one place that decides what the form would save and what is wrong with
+/// it. Both, always — never one or the other: the Escape prompt asks whether
+/// the form differs from what is on disk, and a form that is merely invalid
+/// still holds everything the user typed. Handing back only the errors would
+/// make a brand-new invalid profile compare equal to the nothing on disk, and
+/// Escape would throw the typing away without asking.
+///
+/// A missing `name` is deliberately not an error: the host list already falls
+/// back to the host for a nameless profile, and requiring one would refuse
+/// every host imported from `~/.ssh/config`.
+fn validate_ssh_draft(draft: SshFormDraft, profiles: &[SshProfile]) -> (SshProfile, SshFormErrors) {
+    let mut errors = SshFormErrors::default();
+
+    let host = draft.host.trim().to_string();
+    if host.is_empty() {
+        errors.host = Some(SshFieldError::HostMissing);
+    }
+
+    let port_text = draft.port.trim();
+    let port = match port_text.is_empty() {
+        true => DEFAULT_SSH_PORT,
+        false => parse_port(port_text).unwrap_or_else(|| {
+            errors.port = Some(SshFieldError::PortRange);
+            DEFAULT_SSH_PORT
+        }),
+    };
+
+    // The field is a name but the profile stores an id, so a jump host already
+    // survives its target being renamed. What it never survived was a name
+    // nobody has: the lookup returned `None`, the profile saved as a direct
+    // connection, and reopening the form showed an empty field.
+    let jump_name = draft.jump.trim();
+    let jump_host = if jump_name.is_empty() {
+        None
+    } else {
+        let named = |p: &&SshProfile| p.name == jump_name;
+        // Duplicate names resolve to whichever profile comes first, as they
+        // always have. The one profile that can never be the answer is the one
+        // being edited, and typing its own name is worth saying out loud
+        // rather than quietly connecting direct.
+        match profiles.iter().filter(named).find(|p| p.id != draft.id) {
+            Some(p) => Some(p.id),
+            None => {
+                errors.jump = Some(match profiles.iter().any(|p| p.name == jump_name) {
+                    true => SshFieldError::JumpIsSelf,
+                    false => SshFieldError::JumpUnknown(jump_name.to_string()),
+                });
+                None
+            }
+        }
+    };
+
+    let proxy = |text: &str, default_port: u16, slot: &mut Option<SshFieldError>| {
+        match parse_host_port_checked(text, default_port) {
+            Ok(hp) => hp,
+            Err(e) => {
+                *slot = Some(e);
+                None
+            }
+        }
+    };
+    let socks_proxy = proxy(&draft.socks, DEFAULT_SOCKS_PORT, &mut errors.socks);
+    let http_proxy = proxy(&draft.http, DEFAULT_HTTP_PROXY_PORT, &mut errors.http);
+
+    let proxy_command = draft.proxy_command.trim();
+    let profile = SshProfile {
+        id: draft.id,
+        name: draft.name.trim().to_string(),
+        group: draft.group,
+        host,
+        port,
+        user: draft.user.trim().to_string(),
+        jump_host,
+        proxy_command: (!proxy_command.is_empty()).then(|| proxy_command.to_string()),
+        socks_proxy,
+        http_proxy,
+        auth: draft.auth,
+        identity_files: split_lines(&draft.identity_files),
+        agent_forward: draft.agent_forward,
+        credential_ref: draft.credential_ref,
+        forwards: draft.forwards,
+        keepalive_interval_s: draft.keepalive_interval.trim().parse().ok(),
+        keepalive_count_max: draft.keepalive_count.trim().parse().ok(),
+        connect_timeout_s: draft.connect_timeout.trim().parse().ok(),
+        warn_on_close: draft.warn_on_close,
+        skip_banner: draft.skip_banner,
+        shell_integration: draft.shell_integration,
+        login_scripts: split_lines(&draft.login_scripts),
+        x11: draft.x11,
+        algorithms: Algorithms {
+            kex: split_list(&draft.kex),
+            cipher: split_list(&draft.cipher),
+            mac: split_list(&draft.mac),
+            hostkey: split_list(&draft.hostkey),
+            compression: split_list(&draft.compression),
+        },
+        verify_host_keys: draft.verify_host_keys,
+    };
+    (profile, errors)
+}
+
+/// The inline complaint under a field: one line, in the danger colour, in the
+/// column the control sits in. Built before the row rather than inside a
+/// `when` closure so it borrows the app for the length of one call.
+fn field_error(message: impl Into<String>, cx: &App) -> Div {
+    div()
+        .text_xs()
+        .text_color(cx.theme().danger)
+        .child(message.into())
+}
+
 fn forward_row_inputs(row: &ForwardRuleForm) -> [&Entity<InputState>; 5] {
     [
         &row.bind_host,
@@ -3049,59 +3288,62 @@ impl Tty7App {
         cx.notify();
     }
 
-    fn ssh_form_collect(&self, cx: &App) -> Option<SshProfile> {
+    /// Reads the form out of its entities and runs it past
+    /// [`validate_ssh_draft`]. The profile that comes back is what the form
+    /// would save; the errors are what stands in the way.
+    fn ssh_form_collect(&self, cx: &App) -> Option<(SshProfile, SshFormErrors)> {
         let form = self.active_settings()?.ssh_form.as_ref()?;
-        let id = form.editing;
         let val = |e: &Entity<InputState>| e.read(cx).value().trim().to_string();
+        // The multi-line and comma-separated fields do their own splitting, so
+        // they travel whole rather than trimmed.
+        let raw = |e: &Entity<InputState>| e.read(cx).value().to_string();
 
-        let jump_name = val(&form.jump);
-        let jump_host = if jump_name.is_empty() {
-            None
-        } else {
-            cx.global::<Config>()
-                .ssh_profiles
-                .iter()
-                .find(|p| p.name == jump_name && p.id != id)
-                .map(|p| p.id)
-        };
-
-        Some(SshProfile {
-            id,
+        let draft = SshFormDraft {
+            id: form.editing,
             name: val(&form.name),
             group: form.carry_group.clone(),
             host: val(&form.host),
-            port: val(&form.port).parse().unwrap_or(22),
+            port: val(&form.port),
             user: val(&form.user),
-            jump_host,
-            proxy_command: (!val(&form.proxy_command).is_empty()).then(|| val(&form.proxy_command)),
-            socks_proxy: parse_host_port(&val(&form.socks)),
-            http_proxy: parse_host_port(&val(&form.http)),
+            jump: val(&form.jump),
+            proxy_command: val(&form.proxy_command),
+            socks: val(&form.socks),
+            http: val(&form.http),
             auth: form.auth,
-            identity_files: split_lines(&form.identity_files.read(cx).value()),
+            identity_files: raw(&form.identity_files),
             agent_forward: form.agent_forward,
             credential_ref: form.carry_credential_ref.clone(),
             forwards: form.forwards.iter().filter_map(|r| r.collect(cx)).collect(),
-            keepalive_interval_s: val(&form.keepalive_interval).parse().ok(),
-            keepalive_count_max: val(&form.keepalive_count).parse().ok(),
-            connect_timeout_s: val(&form.connect_timeout).parse().ok(),
+            keepalive_interval: val(&form.keepalive_interval),
+            keepalive_count: val(&form.keepalive_count),
+            connect_timeout: val(&form.connect_timeout),
             warn_on_close: form.warn_on_close,
             skip_banner: form.skip_banner,
             shell_integration: form.shell_integration,
-            login_scripts: split_lines(&form.login_scripts.read(cx).value()),
+            login_scripts: raw(&form.login_scripts),
             x11: form.x11,
-            algorithms: Algorithms {
-                kex: split_list(&form.kex.read(cx).value()),
-                cipher: split_list(&form.cipher.read(cx).value()),
-                mac: split_list(&form.mac.read(cx).value()),
-                hostkey: split_list(&form.hostkey.read(cx).value()),
-                compression: split_list(&form.compression.read(cx).value()),
-            },
+            kex: raw(&form.kex),
+            cipher: raw(&form.cipher),
+            mac: raw(&form.mac),
+            hostkey: raw(&form.hostkey),
+            compression: raw(&form.compression),
             verify_host_keys: form.verify_host_keys,
-        })
+        };
+        Some(validate_ssh_draft(
+            draft,
+            &cx.global::<Config>().ssh_profiles,
+        ))
     }
 
     pub(crate) fn save_editing_profile(&mut self, cx: &mut Context<Self>) -> Option<Uuid> {
-        let profile = self.ssh_form_collect(cx)?;
+        let (profile, errors) = self.ssh_form_collect(cx)?;
+        // Save and Connect are both disabled while anything is wrong, but this
+        // is the door all of them go through, and what gets past it lands in
+        // the config file — where a host-less profile is a blank row nobody
+        // can identify or delete on sight.
+        if !errors.is_empty() {
+            return None;
+        }
         let id = profile.id;
         self.update_config(cx, |cfg| {
             if let Some(slot) = cfg.ssh_profiles.iter_mut().find(|p| p.id == id) {
@@ -3120,7 +3362,9 @@ impl Tty7App {
 
     /// Whether the SSH profile form on screen holds edits that were never
     /// saved. Save is enabled off exactly this, so closing on it is the same
-    /// question the button already answers.
+    /// question the button already answers — and it compares what the form
+    /// would save even when the form cannot be saved yet, so a half-typed new
+    /// host is still something Escape has to ask about.
     pub(crate) fn ssh_form_dirty(&self, cx: &App) -> bool {
         let Some(form) = self.active_settings().and_then(|s| s.ssh_form.as_ref()) else {
             return false;
@@ -3131,7 +3375,7 @@ impl Tty7App {
             .iter()
             .find(|p| p.id == form.editing)
             .cloned();
-        self.ssh_form_collect(cx) != saved
+        self.ssh_form_collect(cx).map(|(profile, _)| profile) != saved
     }
 
     /// Closing from Escape or the X is the user leaving; every other caller
@@ -3310,7 +3554,8 @@ impl Tty7App {
             .iter()
             .find(|p| p.id == editing)
             .cloned();
-        let collected = self.ssh_form_collect(cx);
+        let (collected, errors) = self.ssh_form_collect(cx).unzip();
+        let errors = errors.unwrap_or_default();
         let dirty = collected != saved;
         let address = collected
             .as_ref()
@@ -3379,20 +3624,38 @@ impl Tty7App {
                         Button::new("ssh-form-save")
                             .label(t(L10nKey::Save))
                             .small()
-                            .disabled(!dirty)
+                            .disabled(!dirty || !errors.is_empty())
                             .on_click(cx.listener(|this, _, _w, cx| this.save_ssh_form(cx))),
                     )
                     .child(
+                        // Connect saves first, so it answers to the same
+                        // rules. Before this it answered to none at all, and
+                        // an empty host reached the socket layer as a DNS
+                        // error about a name nobody typed.
                         Button::new("ssh-form-connect")
                             .label(t(L10nKey::Connect))
                             .primary()
                             .small()
+                            .disabled(!errors.is_empty())
                             .on_click(cx.listener(|this, _, window, cx| {
                                 this.save_and_connect_profile(window, cx)
                             })),
                     ),
             );
 
+        // Every field notifies on change, so this form re-renders on each
+        // keystroke: telling a brand-new host that it needs a host is
+        // something it would say before the user had typed a character. Hold
+        // that one line back until the group it belongs to has something in
+        // it. A malformed value has nothing to wait for and says so at once.
+        let core_blank = form.core_is_blank(cx);
+        let host_error = errors
+            .host
+            .as_ref()
+            .filter(|_| !core_blank)
+            .map(|e| field_error(e.message(), cx));
+        let port_error = errors.port.as_ref().map(|e| field_error(e.message(), cx));
+
         let core = v_flex()
             .gap_3()
             .child(
@@ -3411,21 +3674,28 @@ impl Tty7App {
                 self.settings_row(
                     t(L10nKey::SettingsHost),
                     t(L10nKey::SettingsHostDesc),
-                    h_flex()
-                        .gap_2()
+                    v_flex()
+                        .gap_1()
                         .max_w_full()
                         .child(
-                            div()
-                                .w(px(172.))
-                                .min_w_0()
-                                .child(Input::new(&form.host).small()),
-                        )
-                        .child(
-                            div()
-                                .w(px(80.))
-                                .flex_shrink_0()
-                                .child(Input::new(&form.port).small()),
+                            h_flex()
+                                .gap_2()
+                                .max_w_full()
+                                .child(
+                                    div()
+                                        .w(px(172.))
+                                        .min_w_0()
+                                        .child(Input::new(&form.host).small()),
+                                )
+                                .child(
+                                    div()
+                                        .w(px(80.))
+                                        .flex_shrink_0()
+                                        .child(Input::new(&form.port).small()),
+                                ),
                         )
+                        .when_some(host_error, |col, line| col.child(line))
+                        .when_some(port_error, |col, line| col.child(line))
                         .into_any_element(),
                     cx,
                 ),
@@ -3478,9 +3748,9 @@ impl Tty7App {
             .gap_4()
             .child(header)
             .child(core)
-            .child(self.render_ssh_profile_jump_section(form, cx))
+            .child(self.render_ssh_profile_jump_section(form, &errors, cx))
             .child(self.render_ssh_profile_forwards_section(form, cx))
-            .child(self.render_ssh_profile_advanced_section(form, cx))
+            .child(self.render_ssh_profile_advanced_section(form, &errors, cx))
             .into_any_element()
     }
 
@@ -3526,6 +3796,7 @@ impl Tty7App {
     fn render_ssh_profile_jump_section(
         &self,
         form: &SshProfileForm,
+        errors: &SshFormErrors,
         cx: &mut Context<Self>,
     ) -> AnyElement {
         let summary = {
@@ -3536,11 +3807,15 @@ impl Tty7App {
                 name
             }
         };
+        // A complaint nobody can see is a Save button that is greyed out for
+        // no reason the user can read, and the field keeps its text whether
+        // this section is folded or not — so an error holds it open.
+        let open = form.show_jump || errors.jump.is_some();
         let mut section = v_flex().child(self.disclosure_header(
             "ssh-sec-jump",
             t(L10nKey::SettingsJumpHost),
             &summary,
-            form.show_jump,
+            open,
             cx,
             |this, cx| {
                 if let Some(f) = this.ssh_form_mut() {
@@ -3549,15 +3824,18 @@ impl Tty7App {
                 }
             },
         ));
-        if form.show_jump {
+        if open {
+            let error = errors.jump.as_ref().map(|e| field_error(e.message(), cx));
             section = section.child(
                 self.settings_row(
                     t(L10nKey::SettingsJumpHost),
                     t(L10nKey::SettingsJumpHostDesc),
-                    div()
+                    v_flex()
+                        .gap_1()
                         .w(px(260.))
                         .max_w_full()
                         .child(Input::new(&form.jump).small())
+                        .when_some(error, |col, line| col.child(line))
                         .into_any_element(),
                     cx,
                 ),
@@ -3635,14 +3913,21 @@ impl Tty7App {
         cx: &mut Context<Self>,
     ) -> AnyElement {
         let muted = cx.theme().muted_foreground;
-        let danger = cx.theme().danger;
         let needs_target = row.kind != ForwardKind::Dynamic;
         let kind_idx = match row.kind {
             ForwardKind::Local => 0,
             ForwardKind::Remote => 1,
             ForwardKind::Dynamic => 2,
         };
-        let incomplete = row.collect(cx).is_none() && !row.is_blank(cx);
+        let incomplete = (row.collect(cx).is_none() && !row.is_blank(cx)).then(|| {
+            field_error(
+                match needs_target {
+                    true => t(L10nKey::SettingsFwdNeedsBoth),
+                    false => t(L10nKey::SettingsFwdNeedsListen),
+                },
+                cx,
+            )
+        });
 
         // Below `SPLIT_FORWARD_ROW_BELOW` the five controls stop fitting on one
         // line. The kind switch, the description and the remove button keep the
@@ -3746,13 +4031,7 @@ impl Tty7App {
             .gap_0p5()
             .py_1()
             .child(rule)
-            .when(incomplete, |col| {
-                col.child(div().text_xs().text_color(danger).child(if needs_target {
-                    t(L10nKey::SettingsFwdNeedsBoth)
-                } else {
-                    t(L10nKey::SettingsFwdNeedsListen)
-                }))
-            })
+            .when_some(incomplete, |col, line| col.child(line))
             .into_any_element()
     }
 
@@ -3788,13 +4067,18 @@ impl Tty7App {
     fn render_ssh_profile_advanced_section(
         &self,
         form: &SshProfileForm,
+        errors: &SshFormErrors,
         cx: &mut Context<Self>,
     ) -> AnyElement {
+        // This section opens folded, and a proxy address saved back when the
+        // form wrote port 0 is wrong the moment the profile is opened. Let the
+        // error unfold it, or Save is disabled over something out of sight.
+        let open = form.show_advanced || errors.socks.is_some() || errors.http.is_some();
         let mut section = v_flex().child(self.disclosure_header(
             "ssh-sec-adv",
             t(L10nKey::SettingsAdvanced),
             t(L10nKey::SettingsAdvancedSummary),
-            form.show_advanced,
+            open,
             cx,
             |this, cx| {
                 if let Some(f) = this.ssh_form_mut() {
@@ -3803,7 +4087,7 @@ impl Tty7App {
                 }
             },
         ));
-        if !form.show_advanced {
+        if !open {
             return section.into_any_element();
         }
 
@@ -3823,6 +4107,28 @@ impl Tty7App {
                 cx,
             )
         };
+        // The two proxy addresses are the only advanced fields with a rule of
+        // their own, so they carry room for the complaint under the control.
+        let proxy_row = |this: &Self,
+                         label: &str,
+                         desc: &str,
+                         input: &Entity<InputState>,
+                         error: Option<&SshFieldError>,
+                         cx: &mut Context<Self>| {
+            let line = error.map(|e| field_error(e.message(), cx));
+            this.settings_row(
+                label.to_string(),
+                desc.to_string(),
+                v_flex()
+                    .gap_1()
+                    .w(px(260.))
+                    .max_w_full()
+                    .child(Input::new(input).small())
+                    .when_some(line, |col, line| col.child(line))
+                    .into_any_element(),
+                cx,
+            )
+        };
 
         let on_off = |b: bool| {
             if b {
@@ -3877,18 +4183,20 @@ impl Tty7App {
                 &form.proxy_command,
                 cx,
             ))
-            .child(text_row(
+            .child(proxy_row(
                 self,
                 t(L10nKey::SettingsSocks5Proxy),
                 t(L10nKey::SettingsSocks5ProxyDesc),
                 &form.socks,
+                errors.socks.as_ref(),
                 cx,
             ))
-            .child(text_row(
+            .child(proxy_row(
                 self,
                 t(L10nKey::SettingsHttpProxy),
                 t(L10nKey::SettingsHttpProxyDesc),
                 &form.http,
+                errors.http.as_ref(),
                 cx,
             ))
             .child(self.subgroup_header(L10nKey::SettingsGroupAlgorithms, cx))
@@ -5774,19 +6082,14 @@ impl Tty7App {
         let http_proxy_value = http_proxy_input.read(cx).value().trim().to_string();
         let http_proxy_invalid = !http_proxy_value.is_empty()
             && !tty7_core::daemon::install::proxy::is_valid_manual(&http_proxy_value);
+        let http_proxy_error =
+            http_proxy_invalid.then(|| field_error(t(L10nKey::SettingsAppHttpProxyInvalid), cx));
         let http_proxy_control = v_flex()
             .gap_1()
             .w(px(260.))
             .max_w_full()
             .child(Input::new(&http_proxy_input).small())
-            .when(http_proxy_invalid, |this| {
-                this.child(
-                    div()
-                        .text_xs()
-                        .text_color(danger)
-                        .child(t(L10nKey::SettingsAppHttpProxyInvalid)),
-                )
-            })
+            .when_some(http_proxy_error, |this, line| this.child(line))
             .into_any_element();
 
         let logo = Arc::new(Image::from_bytes(
@@ -6572,11 +6875,182 @@ mod tests {
 
     #[test]
     fn parse_host_port_handles_blank_and_ports() {
-        assert!(parse_host_port("  ").is_none());
-        let hp = parse_host_port("example.com:2222").unwrap();
+        assert!(
+            parse_host_port_checked("  ", DEFAULT_SOCKS_PORT)
+                .unwrap()
+                .is_none()
+        );
+        let hp = parse_host_port_checked("example.com:2222", DEFAULT_SOCKS_PORT)
+            .unwrap()
+            .unwrap();
         assert_eq!(hp.host, "example.com");
         assert_eq!(hp.port, 2222);
-        assert_eq!(parse_host_port("host").unwrap().port, 0);
+        // Used to be port 0, which no proxy answers on.
+        assert_eq!(
+            parse_host_port_checked("host", DEFAULT_SOCKS_PORT)
+                .unwrap()
+                .unwrap()
+                .port,
+            DEFAULT_SOCKS_PORT
+        );
+    }
+
+    /// A form with the one field that is genuinely required, and nothing else.
+    fn draft_with_host() -> SshFormDraft {
+        SshFormDraft {
+            host: "example.com".to_string(),
+            ..Default::default()
+        }
+    }
+
+    #[test]
+    fn a_profile_with_no_host_is_not_saveable() {
+        let (_, errors) = validate_ssh_draft(SshFormDraft::default(), &[]);
+        assert_eq!(errors.host, Some(SshFieldError::HostMissing));
+        assert!(!errors.is_empty());
+    }
+
+    #[test]
+    fn spaces_are_not_a_host() {
+        let draft = SshFormDraft {
+            host: "   ".to_string(),
+            ..Default::default()
+        };
+        let (profile, errors) = validate_ssh_draft(draft, &[]);
+        assert_eq!(errors.host, Some(SshFieldError::HostMissing));
+        assert_eq!(profile.host, "");
+    }
+
+    #[test]
+    fn a_name_is_not_required() {
+        // Every host imported from ~/.ssh/config arrives without one, and the
+        // list falls back to the address.
+        let (profile, errors) = validate_ssh_draft(draft_with_host(), &[]);
+        assert_eq!(profile.name, "");
+        assert!(errors.is_empty());
+    }
+
+    #[test]
+    fn a_blank_port_still_means_22() {
+        let (profile, errors) = validate_ssh_draft(draft_with_host(), &[]);
+        assert_eq!(profile.port, 22);
+        assert_eq!(errors.port, None);
+    }
+
+    #[test]
+    fn a_port_that_is_not_a_port_is_refused() {
+        // "0" parses as a u16 and used to be saved as written; the other two
+        // failed to parse and were silently rewritten to 22.
+        for text in ["0", "abc", "70000", "-1", "22 "] {
+            let draft = SshFormDraft {
+                port: text.to_string(),
+                ..draft_with_host()
+            };
+            let (profile, errors) = validate_ssh_draft(draft, &[]);
+            match text {
+                "22 " => {
+                    assert_eq!(errors.port, None, "{text:?} is a port with spare space");
+                    assert_eq!(profile.port, 22);
+                }
+                _ => {
+                    assert_eq!(errors.port, Some(SshFieldError::PortRange), "{text:?}");
+                    assert!(!errors.is_empty());
+                }
+            }
+        }
+    }
+
+    #[test]
+    fn a_jump_host_that_exists_is_kept_by_id() {
+        let bastion = SshProfile::new("bastion");
+        let draft = SshFormDraft {
+            jump: "bastion".to_string(),
+            ..draft_with_host()
+        };
+        let (profile, errors) = validate_ssh_draft(draft, &[bastion.clone()]);
+        assert_eq!(profile.jump_host, Some(bastion.id));
+        assert!(errors.is_empty());
+    }
+
+    #[test]
+    fn a_mistyped_jump_host_says_which_name_it_could_not_find() {
+        let draft = SshFormDraft {
+            jump: "bastian".to_string(),
+            ..draft_with_host()
+        };
+        let (profile, errors) = validate_ssh_draft(draft, &[SshProfile::new("bastion")]);
+        assert_eq!(
+            errors.jump,
+            Some(SshFieldError::JumpUnknown("bastian".to_string()))
+        );
+        assert_eq!(
+            profile.jump_host, None,
+            "a typo never saves as a direct connection"
+        );
+    }
+
+    #[test]
+    fn a_host_cannot_jump_through_itself() {
+        let me = SshProfile::new("prod");
+        let draft = SshFormDraft {
+            id: me.id,
+            jump: "prod".to_string(),
+            ..draft_with_host()
+        };
+        let (profile, errors) = validate_ssh_draft(draft, &[me]);
+        assert_eq!(errors.jump, Some(SshFieldError::JumpIsSelf));
+        assert_eq!(profile.jump_host, None);
+    }
+
+    #[test]
+    fn a_bare_proxy_host_takes_the_scheme_default_port() {
+        let draft = SshFormDraft {
+            socks: "socks.example.com".to_string(),
+            http: "http.example.com".to_string(),
+            ..draft_with_host()
+        };
+        let (profile, errors) = validate_ssh_draft(draft, &[]);
+        assert_eq!(
+            profile.socks_proxy,
+            Some(HostPort::new("socks.example.com", 1080))
+        );
+        assert_eq!(
+            profile.http_proxy,
+            Some(HostPort::new("http.example.com", 8080))
+        );
+        assert!(errors.is_empty());
+    }
+
+    #[test]
+    fn a_proxy_address_with_a_colon_and_no_port_is_refused() {
+        for text in ["proxy.example.com:", "proxy.example.com:abc", "proxy:0"] {
+            let draft = SshFormDraft {
+                socks: text.to_string(),
+                ..draft_with_host()
+            };
+            let (profile, errors) = validate_ssh_draft(draft, &[]);
+            assert_eq!(
+                errors.socks,
+                Some(SshFieldError::ProxyPortRange),
+                "{text:?}"
+            );
+            assert_eq!(profile.socks_proxy, None, "{text:?}");
+        }
+    }
+
+    #[test]
+    fn a_form_that_cannot_be_saved_still_reports_what_it_would_save() {
+        // The Escape prompt asks whether the form differs from the config, so
+        // an invalid form has to hand back a profile to compare — otherwise a
+        // half-typed new host looks identical to the nothing on disk and
+        // Escape throws it away without asking.
+        let draft = SshFormDraft {
+            name: "half typed".to_string(),
+            ..Default::default()
+        };
+        let (profile, errors) = validate_ssh_draft(draft, &[]);
+        assert!(!errors.is_empty());
+        assert_eq!(profile.name, "half typed");
     }
 }
 

src/ui/ssh_connect.rs

@@ -265,8 +265,12 @@ fn map_proxy(profile: &SshProfile) -> SshProxy {
             return SshProxy::Command(cmd.clone());
         }
     }
+    // Port 0 is not somewhere a proxy listens. The settings form used to write
+    // it whenever the address had no port or an unparseable one, so configs
+    // carrying it are already on disk; connecting direct is the honest reading
+    // of an address that names nowhere.
     if let Some(HostPort { host, port }) = &profile.socks_proxy {
-        if !host.is_empty() {
+        if !host.is_empty() && *port != 0 {
             return SshProxy::Socks {
                 host: host.clone(),
                 port: *port,
@@ -274,7 +278,7 @@ fn map_proxy(profile: &SshProfile) -> SshProxy {
         }
     }
     if let Some(HostPort { host, port }) = &profile.http_proxy {
-        if !host.is_empty() {
+        if !host.is_empty() && *port != 0 {
             return SshProxy::Http {
                 host: host.clone(),
                 port: *port,
@@ -551,4 +555,22 @@ mod tests {
             SshProxy::Command(_)
         ));
     }
+
+    #[test]
+    fn a_proxy_on_port_zero_is_no_proxy() {
+        // What the settings form wrote for `proxy.example.com` before it
+        // checked the port, and what is still sitting in configs saved then.
+        let store = InMemoryCredentialStore::new();
+        let mut p = profile("web", "h", "u");
+        p.socks_proxy = Some(HostPort::new("socks", 0));
+        assert!(matches!(
+            build_native_ssh_spec(&p, &[], &store, true).proxy,
+            SshProxy::None
+        ));
+        p.http_proxy = Some(HostPort::new("http", 8080));
+        assert!(matches!(
+            build_native_ssh_spec(&p, &[], &store, true).proxy,
+            SshProxy::Http { .. }
+        ));
+    }
 }