Skip to content

Commit

Permalink
fix: assure passwords aren't returned in percent-encoded form.
Browse files Browse the repository at this point in the history
  • Loading branch information
Byron committed Dec 6, 2024
1 parent d281ba6 commit 530257f
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 10 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions gix-url/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand Down
2 changes: 1 addition & 1 deletion gix-url/src/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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("<redacted>".into());
storage.password = Some("redacted".into());
&storage
} else {
self
Expand Down
8 changes: 6 additions & 2 deletions gix-url/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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())?;
Expand Down
17 changes: 12 additions & 5 deletions gix-url/src/parse.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down Expand Up @@ -115,13 +115,20 @@ pub(crate) fn url(input: &BStr, protocol_end: usize) -> Result<crate::Url, Error
serialize_alternative_form: false,
scheme,
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: url.path().into(),
})
}

fn percent_decoded_utf8(s: &str) -> 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<crate::Url, Error> {
let input = input_to_utf8(input, UrlKind::Scp)?;

Expand Down Expand Up @@ -151,7 +158,7 @@ pub(crate) fn scp(input: &BStr, colon: usize) -> Result<crate::Url, Error> {
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(),
Expand All @@ -162,7 +169,7 @@ fn url_user(url: &url::Url) -> Option<String> {
if url.username().is_empty() && url.password().is_none() {
None
} else {
Some(url.username().into())
Some(percent_decoded_utf8(url.username()))
}
}

Expand Down
4 changes: 2 additions & 2 deletions gix-url/tests/access/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:<redacted>@host/path",
"it visibly redacts passwords though",
"https://user:redacted@host/path",
"it visibly redacts passwords though, and it's still a valid URL",
);
}
20 changes: 20 additions & 0 deletions gix-url/tests/parse/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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%[email protected]: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(
Expand Down

0 comments on commit 530257f

Please sign in to comment.