From 530257f718d8abe0668eeeed072cc4fee9707cbb Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 6 Dec 2024 20:42:56 +0100 Subject: [PATCH] fix: assure passwords aren't returned in percent-encoded form. --- Cargo.lock | 1 + gix-url/Cargo.toml | 1 + gix-url/src/impls.rs | 2 +- gix-url/src/lib.rs | 8 ++++++-- gix-url/src/parse.rs | 17 ++++++++++++----- gix-url/tests/access/mod.rs | 4 ++-- gix-url/tests/parse/http.rs | 20 ++++++++++++++++++++ 7 files changed, 43 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7f226e80914..c755882bd88 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2792,6 +2792,7 @@ dependencies = [ "gix-features 0.39.1", "gix-path 0.10.13", "gix-testtools", + "percent-encoding", "serde", "thiserror 2.0.3", "url", diff --git a/gix-url/Cargo.toml b/gix-url/Cargo.toml index 5e8c01dc0d6..b2b2ce6d789 100644 --- a/gix-url/Cargo.toml +++ b/gix-url/Cargo.toml @@ -26,6 +26,7 @@ serde = { version = "1.0.114", optional = true, default-features = false, featur thiserror = "2.0.0" url = "2.5.2" bstr = { version = "1.3.0", default-features = false, features = ["std"] } +percent-encoding = "2.3.1" document-features = { version = "0.2.0", optional = true } diff --git a/gix-url/src/impls.rs b/gix-url/src/impls.rs index d153d707fde..8c4be38a80a 100644 --- a/gix-url/src/impls.rs +++ b/gix-url/src/impls.rs @@ -81,7 +81,7 @@ impl std::fmt::Display for Url { let mut storage; let to_print = if self.password.is_some() { storage = self.clone(); - storage.password = Some("".into()); + storage.password = Some("redacted".into()); &storage } else { self diff --git a/gix-url/src/lib.rs b/gix-url/src/lib.rs index d4bca2dd36e..67ab8388731 100644 --- a/gix-url/src/lib.rs +++ b/gix-url/src/lib.rs @@ -308,6 +308,10 @@ impl Url { } } +fn percent_encode(s: &str) -> Cow<'_, str> { + percent_encoding::utf8_percent_encode(s, percent_encoding::NON_ALPHANUMERIC).into() +} + /// Serialization impl Url { /// Write this URL losslessly to `out`, ready to be parsed again. @@ -318,10 +322,10 @@ impl Url { } match (&self.user, &self.host) { (Some(user), Some(host)) => { - out.write_all(user.as_bytes())?; + out.write_all(percent_encode(user).as_bytes())?; if let Some(password) = &self.password { out.write_all(b":")?; - out.write_all(password.as_bytes())?; + out.write_all(percent_encode(password).as_bytes())?; } out.write_all(b"@")?; out.write_all(host.as_bytes())?; diff --git a/gix-url/src/parse.rs b/gix-url/src/parse.rs index 3a2b2bfe7c2..e6496ccf48a 100644 --- a/gix-url/src/parse.rs +++ b/gix-url/src/parse.rs @@ -1,8 +1,8 @@ use std::convert::Infallible; -use bstr::{BStr, BString, ByteSlice}; - use crate::Scheme; +use bstr::{BStr, BString, ByteSlice}; +use percent_encoding::percent_decode_str; /// The error returned by [parse()](crate::parse()). #[derive(Debug, thiserror::Error)] @@ -115,13 +115,20 @@ pub(crate) fn url(input: &BStr, protocol_end: usize) -> Result String { + percent_decode_str(s) + .decode_utf8() + .expect("it's not possible to sneak illegal UTF8 into a URL") + .into_owned() +} + pub(crate) fn scp(input: &BStr, colon: usize) -> Result { let input = input_to_utf8(input, UrlKind::Scp)?; @@ -151,7 +158,7 @@ pub(crate) fn scp(input: &BStr, colon: usize) -> Result { serialize_alternative_form: true, scheme: url.scheme().into(), user: url_user(&url), - password: url.password().map(Into::into), + password: url.password().map(percent_decoded_utf8), host: url.host_str().map(Into::into), port: url.port(), path: path.into(), @@ -162,7 +169,7 @@ fn url_user(url: &url::Url) -> Option { if url.username().is_empty() && url.password().is_none() { None } else { - Some(url.username().into()) + Some(percent_decoded_utf8(url.username())) } } diff --git a/gix-url/tests/access/mod.rs b/gix-url/tests/access/mod.rs index ae2269ea2fc..de18a138ea8 100644 --- a/gix-url/tests/access/mod.rs +++ b/gix-url/tests/access/mod.rs @@ -174,7 +174,7 @@ fn display() { compare("/path/to/repo", "/path/to/repo", "same goes for simple paths"); compare( "https://user:password@host/path", - "https://user:@host/path", - "it visibly redacts passwords though", + "https://user:redacted@host/path", + "it visibly redacts passwords though, and it's still a valid URL", ); } diff --git a/gix-url/tests/parse/http.rs b/gix-url/tests/parse/http.rs index dd122a48004..9c017225d4d 100644 --- a/gix-url/tests/parse/http.rs +++ b/gix-url/tests/parse/http.rs @@ -39,6 +39,26 @@ fn username_and_password_and_port() -> crate::Result { ) } +#[test] +fn username_and_password_with_spaces_and_port() -> crate::Result { + let expected = gix_url::Url::from_parts( + Scheme::Http, + Some("user name".into()), + Some("password secret".into()), + Some("example.com".into()), + Some(8080), + b"/~byron/hello".into(), + false, + )?; + assert_url_roundtrip( + "http://user%20name:password%20secret@example.com:8080/~byron/hello", + expected.clone(), + )?; + assert_eq!(expected.user(), Some("user name")); + assert_eq!(expected.password(), Some("password secret")); + Ok(()) +} + #[test] fn only_password() -> crate::Result { assert_url_roundtrip(