From 33df529d257ea9ed5daaec0bfbc7c21462e8d62b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 28 Aug 2025 03:33:16 +0000 Subject: [PATCH 1/7] Initial plan From f82eb7338608fe7042c22c9b45a70537e22425f8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 28 Aug 2025 03:39:12 +0000 Subject: [PATCH 2/7] Make all Url fields public and add mutability warning to docs Co-authored-by: Byron <63622+Byron@users.noreply.github.com> --- gix-url/src/lib.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/gix-url/src/lib.rs b/gix-url/src/lib.rs index 7f3494f85c1..3e46c250c3b 100644 --- a/gix-url/src/lib.rs +++ b/gix-url/src/lib.rs @@ -78,6 +78,12 @@ pub enum ArgumentSafety<'a> { /// /// Additionally there is support for [deserialization](Url::from_bytes()) and [serialization](Url::to_bstring()). /// +/// # Mutability Warning +/// +/// Due to the mutability of this type, it's possible that the URL serializes to something invalid +/// when fields are modified directly. The URL should be accepted as a parameter directly rather +/// than being reconstructed, to maintain validity guarantees. +/// /// # Security Warning /// /// URLs may contain passwords and using standard [formatting](std::fmt::Display) will redact @@ -93,13 +99,13 @@ pub struct Url { /// The URL scheme. pub scheme: Scheme, /// The user to impersonate on the remote. - user: Option, + pub user: Option, /// The password associated with a user. - password: Option, + pub password: Option, /// The host to which to connect. Localhost is implied if `None`. - host: Option, + pub host: Option, /// When serializing, use the alternative forms as it was parsed as such. - serialize_alternative_form: bool, + pub serialize_alternative_form: bool, /// The port to use when connecting to a host. If `None`, standard ports depending on `scheme` will be used. pub port: Option, /// The path portion of the URL, usually the location of the git repository. From b2afc1c6fa0ce62693c345cc582d3342e1c58d1d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 28 Aug 2025 03:48:53 +0000 Subject: [PATCH 3/7] Clarify mutability warning to specify parsing vs reconstruction Co-authored-by: Byron <63622+Byron@users.noreply.github.com> --- gix-url/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/gix-url/src/lib.rs b/gix-url/src/lib.rs index 3e46c250c3b..74e77b31b8f 100644 --- a/gix-url/src/lib.rs +++ b/gix-url/src/lib.rs @@ -81,8 +81,9 @@ pub enum ArgumentSafety<'a> { /// # Mutability Warning /// /// Due to the mutability of this type, it's possible that the URL serializes to something invalid -/// when fields are modified directly. The URL should be accepted as a parameter directly rather -/// than being reconstructed, to maintain validity guarantees. +/// when fields are modified directly. URLs should always be parsed to this type from string or byte +/// parameters, but never be accepted as an instance of this type and then reconstructed, to maintain +/// validity guarantees. /// /// # Security Warning /// From fbe454eab21cdbaff140bf65a0bd8fa25f53fedb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 28 Aug 2025 04:01:10 +0000 Subject: [PATCH 4/7] Replace all unreachable! with proper error handling in gix-url Co-authored-by: Byron <63622+Byron@users.noreply.github.com> --- gix-url/src/lib.rs | 14 ++++++++++++-- gix-url/src/parse.rs | 6 +++++- gix-url/tests/url/expand_path.rs | 5 ++++- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/gix-url/src/lib.rs b/gix-url/src/lib.rs index 74e77b31b8f..34d59bb4263 100644 --- a/gix-url/src/lib.rs +++ b/gix-url/src/lib.rs @@ -353,7 +353,12 @@ impl Url { out.write_all(host.as_bytes())?; } (None, None) => {} - (Some(_user), None) => unreachable!("BUG: should not be possible to have a user but no host"), + (Some(_user), None) => { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + "Invalid URL structure: user specified without host" + )); + } } if let Some(port) = &self.port { write!(out, ":{port}")?; @@ -377,7 +382,12 @@ impl Url { out.write_all(host.as_bytes())?; } (None, None) => {} - (Some(_user), None) => unreachable!("BUG: should not be possible to have a user but no host"), + (Some(_user), None) => { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + "Invalid URL structure: user specified without host" + )); + } } assert!(self.port.is_none(), "BUG: cannot serialize port in alternative form"); if self.scheme == Scheme::Ssh { diff --git a/gix-url/src/parse.rs b/gix-url/src/parse.rs index d35c0c11493..dc16edadb80 100644 --- a/gix-url/src/parse.rs +++ b/gix-url/src/parse.rs @@ -28,11 +28,15 @@ pub enum Error { MissingRepositoryPath { url: BString, kind: UrlKind }, #[error("URL {url:?} is relative which is not allowed in this context")] RelativeUrl { url: String }, + #[error("URL has invalid structure: user specified without host")] + InvalidUrlStructure, } impl From for Error { fn from(_: Infallible) -> Self { - unreachable!("Cannot actually happen, but it seems there can't be a blanket impl for this") + // This should theoretically never happen since Infallible can never be constructed, + // but with public URL fields, we need to handle it gracefully + Error::InvalidUrlStructure } } diff --git a/gix-url/tests/url/expand_path.rs b/gix-url/tests/url/expand_path.rs index 88ae4cbed35..9a2233f65fa 100644 --- a/gix-url/tests/url/expand_path.rs +++ b/gix-url/tests/url/expand_path.rs @@ -39,7 +39,10 @@ fn without_username() -> crate::Result { fn with_username() -> crate::Result { let (user, resolved_path) = expand_path::parse(b"/~byron/hello/git".as_bstr())?; let resolved_path = expand_path::with(user.as_ref(), resolved_path.as_ref(), |user: &ForUser| match user { - ForUser::Current => unreachable!("we have a name"), + ForUser::Current => { + // This test expects a specific user name, not the current user + None // Return None to trigger the MissingHome error + }, ForUser::Name(name) => Some(user_home(name.to_str_lossy().as_ref())), })?; assert_eq!(resolved_path, expected_path()); From c29bd0abd47eca59aa8c3d682001c8b5f6a1111d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 28 Aug 2025 06:35:57 +0000 Subject: [PATCH 5/7] Address PR feedback: use Error::other() and revert unnecessary changes Co-authored-by: Byron <63622+Byron@users.noreply.github.com> --- gix-url/src/lib.rs | 6 ++---- gix-url/src/parse.rs | 6 +----- gix-url/tests/url/expand_path.rs | 5 +---- 3 files changed, 4 insertions(+), 13 deletions(-) diff --git a/gix-url/src/lib.rs b/gix-url/src/lib.rs index 34d59bb4263..81e5d4dfb36 100644 --- a/gix-url/src/lib.rs +++ b/gix-url/src/lib.rs @@ -354,8 +354,7 @@ impl Url { } (None, None) => {} (Some(_user), None) => { - return Err(std::io::Error::new( - std::io::ErrorKind::InvalidData, + return Err(std::io::Error::other( "Invalid URL structure: user specified without host" )); } @@ -383,8 +382,7 @@ impl Url { } (None, None) => {} (Some(_user), None) => { - return Err(std::io::Error::new( - std::io::ErrorKind::InvalidData, + return Err(std::io::Error::other( "Invalid URL structure: user specified without host" )); } diff --git a/gix-url/src/parse.rs b/gix-url/src/parse.rs index dc16edadb80..d35c0c11493 100644 --- a/gix-url/src/parse.rs +++ b/gix-url/src/parse.rs @@ -28,15 +28,11 @@ pub enum Error { MissingRepositoryPath { url: BString, kind: UrlKind }, #[error("URL {url:?} is relative which is not allowed in this context")] RelativeUrl { url: String }, - #[error("URL has invalid structure: user specified without host")] - InvalidUrlStructure, } impl From for Error { fn from(_: Infallible) -> Self { - // This should theoretically never happen since Infallible can never be constructed, - // but with public URL fields, we need to handle it gracefully - Error::InvalidUrlStructure + unreachable!("Cannot actually happen, but it seems there can't be a blanket impl for this") } } diff --git a/gix-url/tests/url/expand_path.rs b/gix-url/tests/url/expand_path.rs index 9a2233f65fa..88ae4cbed35 100644 --- a/gix-url/tests/url/expand_path.rs +++ b/gix-url/tests/url/expand_path.rs @@ -39,10 +39,7 @@ fn without_username() -> crate::Result { fn with_username() -> crate::Result { let (user, resolved_path) = expand_path::parse(b"/~byron/hello/git".as_bstr())?; let resolved_path = expand_path::with(user.as_ref(), resolved_path.as_ref(), |user: &ForUser| match user { - ForUser::Current => { - // This test expects a specific user name, not the current user - None // Return None to trigger the MissingHome error - }, + ForUser::Current => unreachable!("we have a name"), ForUser::Name(name) => Some(user_home(name.to_str_lossy().as_ref())), })?; assert_eq!(resolved_path, expected_path()); From 519e76988bf5f00cdbe719fffbc8361cded31b8d Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Thu, 28 Aug 2025 10:00:37 +0200 Subject: [PATCH 6/7] fix formatting, clippy --- gix-url/Cargo.toml | 2 +- gix-url/src/lib.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gix-url/Cargo.toml b/gix-url/Cargo.toml index dd73934ae6d..bf0c84b936b 100644 --- a/gix-url/Cargo.toml +++ b/gix-url/Cargo.toml @@ -9,7 +9,7 @@ description = "A crate of the gitoxide project implementing parsing and serializ authors = ["Sebastian Thiel "] edition = "2021" include = ["src/**/*", "LICENSE-*", "tests/baseline/**/*"] -rust-version = "1.70" +rust-version = "1.74" [lib] doctest = false diff --git a/gix-url/src/lib.rs b/gix-url/src/lib.rs index 81e5d4dfb36..dfbfb6b045b 100644 --- a/gix-url/src/lib.rs +++ b/gix-url/src/lib.rs @@ -355,7 +355,7 @@ impl Url { (None, None) => {} (Some(_user), None) => { return Err(std::io::Error::other( - "Invalid URL structure: user specified without host" + "Invalid URL structure: user specified without host", )); } } @@ -383,7 +383,7 @@ impl Url { (None, None) => {} (Some(_user), None) => { return Err(std::io::Error::other( - "Invalid URL structure: user specified without host" + "Invalid URL structure: user specified without host", )); } } From 52c8edbea179585b92d1cdba792866c06e160596 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Thu, 28 Aug 2025 10:04:01 +0200 Subject: [PATCH 7/7] fixup cargo deny - ignore async-std --- deny.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/deny.toml b/deny.toml index 6dbb3089723..c8f73c52dbf 100644 --- a/deny.toml +++ b/deny.toml @@ -10,6 +10,7 @@ [advisories] ignore = [ { id = "RUSTSEC-2024-0436", reason = "`paste` - macro crate without replacement" }, + { id = "RUSTSEC-2025-0052", reason = "`async-std` - unmaintained without replacement - needs some time to replace, but async version isn't too important right now" }, ]