Skip to content

Commit

Permalink
Start on using {user,host}_as_argument in prepare_invocation
Browse files Browse the repository at this point in the history
Right now this is more locked down than necessary, serving to check
that ArgumentSafety can be easily matched on in scenarios where one
may wish to reject both usernames and hosts that are ambiguous.

This is verified by all tests passing except these that fail:

- ambiguous_host_is_allowed_with_user_explicit_ssh
- ambiguous_host_is_allowed_with_user_implicit_ssh

Prohibiting ambiguous hosts even when the username is present and
cannot itself be confused with a command-line option isn't needed
in gix-transport, which passes URLs of the form `user@host` anytime
the username is present. However, if an application ever passes
the username as a separate argument rather than as part of the
netloc with the host, then being able to express that in a natural
and non-error-prone way is important.
  • Loading branch information
EliahKagan committed Apr 12, 2024
1 parent 1b0af07 commit 4dda375
Showing 1 changed file with 9 additions and 17 deletions.
26 changes: 9 additions & 17 deletions gix-transport/src/client/blocking_io/ssh/program_kind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ use std::{ffi::OsStr, io::ErrorKind};

use bstr::{BString, ByteSlice, ByteVec};

use gix_url::ArgumentSafety::*;

use crate::{
client::{ssh, ssh::ProgramKind},
Protocol,
Expand Down Expand Up @@ -60,23 +62,13 @@ impl ProgramKind {
}
}
};
let host_maybe_with_user_as_ssh_arg = match url.user() {
Some(user) => {
// FIXME: See the fixme comment on Url::user_argument_safe() about its return type.
if url.user_argument_safe() != Some(user) {
return Err(ssh::invocation::Error::AmbiguousUserName { user: user.into() });
}
let host = url.host().expect("present in ssh urls");
format!("{user}@{host}")
}
None => {
let host = url
.host_argument_safe()
.ok_or_else(|| ssh::invocation::Error::AmbiguousHostName {
host: url.host().expect("ssh host always set").into(),
})?;
host.into()
}

let host_maybe_with_user_as_ssh_arg = match (url.user_as_argument(), url.host_as_argument()) {
(Usable(user), Usable(host)) => format!("{user}@{host}"),
(Absent, Usable(host)) => host.into(),
(Dangerous(user), _) => Err(ssh::invocation::Error::AmbiguousUserName { user: user.into() })?,
(_, Dangerous(host)) => Err(ssh::invocation::Error::AmbiguousHostName { host: host.into() })?,
(_, Absent) => panic!("BUG: host should always be present in SSH URLs"),
};

// Try to force ssh to yield English messages (for parsing later).
Expand Down

0 comments on commit 4dda375

Please sign in to comment.