Merge branch 'password-in-urls'

Author: Byron

gix-transport/src/client/blocking_io/connect.rs

@@ -23,7 +23,7 @@ pub(crate) mod function {
         Ok(match url.scheme {
             gix_url::Scheme::Ext(_) => return Err(Error::UnsupportedScheme(url.scheme)),
             gix_url::Scheme::File => {
-                if url.user().is_some() || url.host().is_some() || url.port.is_some() {
+                if url.user().is_some() || url.password().is_some() || url.host().is_some() || url.port.is_some() {
                     return Err(Error::UnsupportedUrlTokens {
                         url: url.to_bstring(),
                         scheme: url.scheme,
@@ -59,10 +59,9 @@ pub(crate) mod function {
             #[cfg(not(any(feature = "http-client-curl", feature = "http-client-reqwest")))]
             gix_url::Scheme::Https | gix_url::Scheme::Http => return Err(Error::CompiledWithoutHttp(url.scheme)),
             #[cfg(any(feature = "http-client-curl", feature = "http-client-reqwest"))]
-            gix_url::Scheme::Https | gix_url::Scheme::Http => Box::new(crate::client::http::connect(
-                &url.to_bstring().to_string(),
-                options.version,
-            )),
+            gix_url::Scheme::Https | gix_url::Scheme::Http => {
+                Box::new(crate::client::http::connect(url, options.version))
+            }
         })
     }
 }

gix-transport/src/client/blocking_io/file.rs

@@ -71,7 +71,7 @@ impl SpawnProcessOnDemand {
     }
     fn new_local(path: BString, version: Protocol) -> SpawnProcessOnDemand {
         SpawnProcessOnDemand {
-            url: gix_url::Url::from_parts_as_alternative_form(gix_url::Scheme::File, None, None, None, path.clone())
+            url: gix_url::Url::from_parts(gix_url::Scheme::File, None, None, None, None, path.clone(), true)
                 .expect("valid url"),
             path,
             ssh_cmd: None,

gix-transport/src/client/blocking_io/http/mod.rs

@@ -217,26 +217,40 @@ pub struct Transport<H: Http> {
 impl<H: Http> Transport<H> {
     /// Create a new instance with `http` as implementation to communicate to `url` using the given `desired_version`.
     /// Note that we will always fallback to other versions as supported by the server.
-    pub fn new_http(http: H, url: &str, desired_version: Protocol) -> Self {
+    pub fn new_http(http: H, url: gix_url::Url, desired_version: Protocol) -> Self {
+        let identity = url
+            .user()
+            .zip(url.password())
+            .map(|(user, pass)| gix_sec::identity::Account {
+                username: user.to_string(),
+                password: pass.to_string(),
+            });
         Transport {
-            url: url.to_owned(),
+            url: url.to_bstring().to_string(),
             user_agent_header: concat!("User-Agent: git/oxide-", env!("CARGO_PKG_VERSION")),
             desired_version,
             actual_version: Default::default(),
             service: None,
             http,
             line_provider: None,
-            identity: None,
+            identity,
         }
     }
 }
 
+impl<H: Http> Transport<H> {
+    /// Returns the identity that the transport uses when connecting to the remote.
+    pub fn identity(&self) -> Option<&gix_sec::identity::Account> {
+        self.identity.as_ref()
+    }
+}
+
 #[cfg(any(feature = "http-client-curl", feature = "http-client-reqwest"))]
 impl Transport<Impl> {
     /// Create a new instance to communicate to `url` using the given `desired_version` of the `git` protocol.
     ///
     /// Note that the actual implementation depends on feature toggles.
-    pub fn new(url: &str, desired_version: Protocol) -> Self {
+    pub fn new(url: gix_url::Url, desired_version: Protocol) -> Self {
         Self::new_http(Impl::default(), url, desired_version)
     }
 }
@@ -497,13 +511,13 @@ impl<H: Http, B: ExtendedBufRead + Unpin> ExtendedBufRead for HeadersThenBody<H,
 
 /// Connect to the given `url` via HTTP/S using the `desired_version` of the `git` protocol, with `http` as implementation.
 #[cfg(all(feature = "http-client", not(feature = "http-client-curl")))]
-pub fn connect_http<H: Http>(http: H, url: &str, desired_version: Protocol) -> Transport<H> {
+pub fn connect_http<H: Http>(http: H, url: gix_url::Url, desired_version: Protocol) -> Transport<H> {
     Transport::new_http(http, url, desired_version)
 }
 
 /// Connect to the given `url` via HTTP/S using the `desired_version` of the `git` protocol.
 #[cfg(any(feature = "http-client-curl", feature = "http-client-reqwest"))]
-pub fn connect(url: &str, desired_version: Protocol) -> Transport<Impl> {
+pub fn connect(url: gix_url::Url, desired_version: Protocol) -> Transport<Impl> {
     Transport::new(url, desired_version)
 }
 

gix-transport/src/client/blocking_io/ssh/mod.rs

@@ -86,6 +86,7 @@ pub mod connect {
 ///
 /// The `desired_version` is the preferred protocol version when establishing the connection, but note that it can be
 /// downgraded by servers not supporting it.
+#[allow(clippy::result_large_err)]
 pub fn connect(
     url: gix_url::Url,
     desired_version: Protocol,

gix-transport/src/client/blocking_io/ssh/tests.rs

@@ -192,7 +192,7 @@ mod program_kind {
         }
 
         fn joined(input: &[&str]) -> String {
-            input.iter().copied().collect::<Vec<_>>().join(" ")
+            input.to_vec().join(" ")
         }
         fn try_call(
             kind: ProgramKind,

gix-transport/tests/client/blocking_io/http/mock.rs

@@ -99,13 +99,13 @@ pub fn serve_and_connect(
     version: Protocol,
 ) -> Result<(Server, http::Transport<http::Impl>), crate::Error> {
     let server = serve_once(name);
-    let url = format!(
+    let url_str = format!(
         "http://{}:{}/{}",
         &server.addr.ip().to_string(),
         &server.addr.port(),
         path
     );
-    let client = gix_transport::client::http::connect(&url, version);
-    assert_eq!(url, client.to_url().as_ref());
+    let client = gix_transport::client::http::connect(url_str.as_str().try_into()?, version);
+    assert_eq!(url_str, client.to_url().as_ref());
     Ok((server, client))
 }

gix-transport/tests/client/blocking_io/http/mod.rs

@@ -46,6 +46,21 @@ fn http_status_500_is_communicated_via_special_io_error() -> crate::Result {
     Ok(())
 }
 
+#[test]
+fn http_identity_is_picked_up_from_url() -> crate::Result {
+    let transport =
+        gix_transport::client::http::connect("https://user:pass@example.com/repo".try_into()?, Protocol::V2);
+    assert_eq!(transport.to_url().as_ref(), "https://user:pass@example.com/repo");
+    assert_eq!(
+        transport.identity(),
+        Some(&gix_sec::identity::Account {
+            username: "user".into(),
+            password: "pass".into()
+        })
+    );
+    Ok(())
+}
+
 // based on a test in cargo
 #[test]
 fn http_will_use_pipelining() {
@@ -108,7 +123,8 @@ fn http_will_use_pipelining() {
     });
 
     let url = format!("http://{}:{}/reponame", &addr.ip().to_string(), &addr.port(),);
-    let mut client = gix_transport::client::http::connect(&url, gix_transport::Protocol::V2);
+    let mut client =
+        gix_transport::client::http::connect(url.try_into().expect("valid url"), gix_transport::Protocol::V2);
     match client.handshake(gix_transport::Service::UploadPack, &[]) {
         Ok(_) => unreachable!("expecting permission denied to be detected"),
         Err(gix_transport::client::Error::Io(err)) if err.kind() == std::io::ErrorKind::PermissionDenied => {}

gix-url/src/impls.rs

@@ -13,6 +13,7 @@ impl Default for Url {
             serialize_alternative_form: false,
             scheme: Scheme::Ssh,
             user: None,
+            password: None,
             host: None,
             port: None,
             path: bstr::BString::default(),

gix-url/src/lib.rs

@@ -38,6 +38,8 @@ pub struct Url {
     pub scheme: Scheme,
     /// The user to impersonate on the remote.
     user: Option<String>,
+    /// The password associated with a user.
+    password: Option<String>,
     /// The host to which to connect. Localhost is implied if `None`.
     host: Option<String>,
     /// When serializing, use the alternative forms as it was parsed as such.
@@ -50,44 +52,25 @@ pub struct Url {
 
 /// Instantiation
 impl Url {
-    /// Create a new instance from the given parts, which will be validated by parsing them back.
+    /// Create a new instance from the given parts, including a password, which will be validated by parsing them back.
     pub fn from_parts(
         scheme: Scheme,
         user: Option<String>,
+        password: Option<String>,
         host: Option<String>,
         port: Option<u16>,
         path: BString,
+        serialize_alternative_form: bool,
     ) -> Result<Self, parse::Error> {
         parse(
             Url {
                 scheme,
                 user,
+                password,
                 host,
                 port,
                 path,
-                serialize_alternative_form: false,
-            }
-            .to_bstring()
-            .as_ref(),
-        )
-    }
-
-    /// Create a new instance from the given parts, which will be validated by parsing them back from its alternative form.
-    pub fn from_parts_as_alternative_form(
-        scheme: Scheme,
-        user: Option<String>,
-        host: Option<String>,
-        port: Option<u16>,
-        path: BString,
-    ) -> Result<Self, parse::Error> {
-        parse(
-            Url {
-                scheme,
-                user,
-                host,
-                port,
-                path,
-                serialize_alternative_form: true,
+                serialize_alternative_form,
             }
             .to_bstring()
             .as_ref(),
@@ -133,6 +116,10 @@ impl Url {
     pub fn user(&self) -> Option<&str> {
         self.user.as_deref()
     }
+    /// Returns the password mentioned in the url, if present.
+    pub fn password(&self) -> Option<&str> {
+        self.password.as_deref()
+    }
     /// Returns the host mentioned in the url, if present.
     pub fn host(&self) -> Option<&str> {
         self.host.as_deref()
@@ -178,6 +165,10 @@ impl Url {
         match (&self.user, &self.host) {
             (Some(user), Some(host)) => {
                 out.write_all(user.as_bytes())?;
+                if let Some(password) = &self.password {
+                    out.write_all(&[b':'])?;
+                    out.write_all(password.as_bytes())?;
+                }
                 out.write_all(&[b'@'])?;
                 out.write_all(host.as_bytes())?;
             }

gix-url/src/parse.rs

@@ -66,10 +66,12 @@ fn has_no_explicit_protocol(url: &[u8]) -> bool {
 }
 
 fn to_owned_url(url: url::Url) -> Result<crate::Url, Error> {
+    let password = url.password();
     Ok(crate::Url {
         serialize_alternative_form: false,
         scheme: str_to_protocol(url.scheme()),
-        user: if url.username().is_empty() {
+        password: password.map(ToOwned::to_owned),
+        user: if url.username().is_empty() && password.is_none() {
             None
         } else {
             Some(url.username().into())

gix-url/tests/parse/http.rs

@@ -0,0 +1,76 @@
+use gix_url::Scheme;
+
+use crate::parse::{assert_url, assert_url_roundtrip, url, url_with_pass};
+
+#[test]
+fn username_expansion_is_unsupported() -> crate::Result {
+    assert_url_roundtrip(
+        "http://example.com/~byron/hello",
+        url(Scheme::Http, None, "example.com", None, b"/~byron/hello"),
+    )
+}
+
+#[test]
+fn empty_user_cannot_roundtrip() -> crate::Result {
+    let actual = gix_url::parse("http://@example.com/~byron/hello".into())?;
+    let expected = url(Scheme::Http, "", "example.com", None, b"/~byron/hello");
+    assert_eq!(actual, expected);
+    assert_eq!(
+        actual.to_bstring(),
+        "http://example.com/~byron/hello",
+        "we cannot differentiate between empty user and no user"
+    );
+    Ok(())
+}
+
+#[test]
+fn username_and_password() -> crate::Result {
+    assert_url_roundtrip(
+        "http://user:password@example.com/~byron/hello",
+        url_with_pass(Scheme::Http, "user", "password", "example.com", None, b"/~byron/hello"),
+    )
+}
+
+#[test]
+fn username_and_password_and_port() -> crate::Result {
+    assert_url_roundtrip(
+        "http://user:password@example.com:8080/~byron/hello",
+        url_with_pass(Scheme::Http, "user", "password", "example.com", 8080, b"/~byron/hello"),
+    )
+}
+
+#[test]
+fn only_password() -> crate::Result {
+    assert_url_roundtrip(
+        "http://:password@example.com/~byron/hello",
+        url_with_pass(Scheme::Http, "", "password", "example.com", None, b"/~byron/hello"),
+    )
+}
+
+#[test]
+fn username_and_empty_password() -> crate::Result {
+    let actual = gix_url::parse("http://user:@example.com/~byron/hello".into())?;
+    let expected = url_with_pass(Scheme::Http, "user", "", "example.com", None, b"/~byron/hello");
+    assert_eq!(actual, expected);
+    assert_eq!(
+        actual.to_bstring(),
+        "http://user@example.com/~byron/hello",
+        "an empty password appears like no password to us - fair enough"
+    );
+    Ok(())
+}
+
+#[test]
+fn secure() -> crate::Result {
+    assert_url_roundtrip(
+        "https://github.com/byron/gitoxide",
+        url(Scheme::Https, None, "github.com", None, b"/byron/gitoxide"),
+    )
+}
+
+#[test]
+fn http_missing_path() -> crate::Result {
+    assert_url_roundtrip("http://host.xz/", url(Scheme::Http, None, "host.xz", None, b"/"))?;
+    assert_url("http://host.xz", url(Scheme::Http, None, "host.xz", None, b"/"))?;
+    Ok(())
+}