From 726c5809c5996faabd6889b4cc9ef6229291ccda Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Thu, 7 May 2026 14:27:46 -0700 Subject: [PATCH 1/2] Require git refs in URLs to be percent-encoded Signed-off-by: William Woodruff --- Cargo.lock | 1 + .../uv-distribution-types/src/requirement.rs | 6 +- crates/uv-git-types/Cargo.toml | 1 + crates/uv-git-types/src/lib.rs | 60 ++++++++++ crates/uv-git-types/src/reference.rs | 23 ++++ crates/uv/tests/it/pip_install.rs | 109 +++++++++++++++++- 6 files changed, 196 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dd701dc5c59..5af736b354f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6593,6 +6593,7 @@ dependencies = [ name = "uv-git-types" version = "0.0.44" dependencies = [ + "percent-encoding", "serde", "thiserror 2.0.18", "tracing", diff --git a/crates/uv-distribution-types/src/requirement.rs b/crates/uv-distribution-types/src/requirement.rs index 92115042843..09e2e501729 100644 --- a/crates/uv-distribution-types/src/requirement.rs +++ b/crates/uv-distribution-types/src/requirement.rs @@ -344,7 +344,7 @@ impl Display for Requirement { subdirectory, } => { write!(f, " @ git+{}", git.url())?; - if let Some(reference) = git.reference().as_str() { + if let Some(reference) = git.reference().as_url_rev() { write!(f, "@{reference}")?; } if let Some(subdirectory) = subdirectory { @@ -771,7 +771,7 @@ impl Display for RequirementSource { subdirectory, } => { write!(f, " git+{}", git.url())?; - if let Some(reference) = git.reference().as_str() { + if let Some(reference) = git.reference().as_url_rev() { write!(f, "@{reference}")?; } if let Some(subdirectory) = subdirectory { @@ -981,7 +981,7 @@ impl TryFrom for RequirementSource { // Create a PEP 508-compatible URL. let mut url = DisplaySafeUrl::parse(&format!("git+{repository}"))?; - if let Some(rev) = reference.as_str() { + if let Some(rev) = reference.as_url_rev() { let path = format!("{}@{}", url.path(), rev); url.set_path(&path); } diff --git a/crates/uv-git-types/Cargo.toml b/crates/uv-git-types/Cargo.toml index 167ca855f7f..ee3c401b0c8 100644 --- a/crates/uv-git-types/Cargo.toml +++ b/crates/uv-git-types/Cargo.toml @@ -20,6 +20,7 @@ uv-cache-key = { workspace = true } uv-redacted = { workspace = true } uv-static = { workspace = true } +percent-encoding = { workspace = true } serde = { workspace = true } thiserror = { workspace = true } tracing = { workspace = true } diff --git a/crates/uv-git-types/src/lib.rs b/crates/uv-git-types/src/lib.rs index 818bddbaa3a..e59fb5bb8d6 100644 --- a/crates/uv-git-types/src/lib.rs +++ b/crates/uv-git-types/src/lib.rs @@ -4,6 +4,7 @@ pub use crate::reference::GitReference; use std::cmp::Ordering; use std::sync::LazyLock; +use percent_encoding::percent_decode_str; use thiserror::Error; use uv_cache_key::RepositoryUrl; use uv_redacted::DisplaySafeUrl; @@ -79,6 +80,10 @@ pub enum GitUrlParseError { "Unsupported Git URL scheme `{0}:` in `{1}` (expected one of `https:`, `ssh:`, or `file:`)" )] UnsupportedGitScheme(String, DisplaySafeUrl), + #[error( + "Ambiguous Git URL `{0}`: the path contains multiple `@` characters. If the Git revision contains `@`, percent-encode it as `%40`" + )] + AmbiguousRevision(DisplaySafeUrl), } /// A URL reference to a Git repository. @@ -234,6 +239,10 @@ impl TryFrom for GitUrl { url.set_fragment(None); url.set_query(None); + if url.path().matches('@').nth(1).is_some() { + return Err(GitUrlParseError::AmbiguousRevision(url)); + } + // If the URL ends with a reference, like `https://git.example.com/MyProject.git@v1.0`, // extract it. let mut reference = GitReference::DefaultBranch; @@ -242,6 +251,7 @@ impl TryFrom for GitUrl { .rsplit_once('@') .map(|(prefix, suffix)| (prefix.to_string(), suffix.to_string())) { + let suffix = percent_decode_str(&suffix).decode_utf8_lossy().into_owned(); reference = GitReference::from_rev(suffix); url.set_path(&prefix); } @@ -267,6 +277,7 @@ impl From for DisplaySafeUrl { | GitReference::BranchOrTag(rev) | GitReference::NamedRef(rev) | GitReference::BranchOrTagOrCommit(rev) => { + let rev = GitReference::encode_rev(&rev); let path = format!("{}@{}", url.path(), rev); url.set_path(&path); } @@ -283,3 +294,52 @@ impl std::fmt::Display for GitUrl { write!(f, "{}", &self.url) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_percent_encoded_reference() -> Result<(), Box> { + let url = DisplaySafeUrl::parse("https://example.com/pkg.git@dev%401%232")?; + let git = GitUrl::try_from(url)?; + + assert_eq!(git.url().as_str(), "https://example.com/pkg.git"); + assert_eq!(git.reference().as_str(), Some("dev@1#2")); + + Ok(()) + } + + #[test] + fn reject_ambiguous_reference() -> Result<(), Box> { + let url = DisplaySafeUrl::parse("https://example.com/pkg.git@dev@1.2.3")?; + let err = GitUrl::try_from(url).unwrap_err(); + + assert_eq!( + err.to_string(), + "Ambiguous Git URL `https://example.com/pkg.git@dev@1.2.3`: the path contains multiple `@` characters. If the Git revision contains `@`, percent-encode it as `%40`" + ); + + Ok(()) + } + + #[test] + fn display_percent_encodes_reference() -> Result<(), Box> { + let git = GitUrl::from_reference( + DisplaySafeUrl::parse("https://example.com/pkg.git")?, + GitReference::from_rev("refs/pull/493/head@1#2%".to_string()), + GitLfs::Disabled, + )?; + let url = DisplaySafeUrl::from(git); + + assert_eq!( + url.as_str(), + "https://example.com/pkg.git@refs/pull/493/head%401%232%25" + ); + + let git = GitUrl::try_from(url)?; + assert_eq!(git.reference().as_str(), Some("refs/pull/493/head@1#2%")); + + Ok(()) + } +} diff --git a/crates/uv-git-types/src/reference.rs b/crates/uv-git-types/src/reference.rs index cb329cd7af6..af54a7f8ea0 100644 --- a/crates/uv-git-types/src/reference.rs +++ b/crates/uv-git-types/src/reference.rs @@ -1,6 +1,19 @@ use std::fmt::Display; use std::str; +use percent_encoding::{AsciiSet, NON_ALPHANUMERIC, utf8_percent_encode}; + +/// Percent-encode Git revisions for use after the `@` in VCS URLs. +/// +/// This follows Python's `urllib.parse.quote(rev, safe="/")`. +/// See: +const GIT_REFERENCE_ENCODE_SET: &AsciiSet = &NON_ALPHANUMERIC + .remove(b'/') + .remove(b'-') + .remove(b'.') + .remove(b'_') + .remove(b'~'); + /// A reference to commit or commit-ish. #[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] pub enum GitReference { @@ -55,6 +68,16 @@ impl GitReference { } } + /// Converts the [`GitReference`] to a percent-encoded revision string for use in a URL. + pub fn as_url_rev(&self) -> Option { + self.as_str().map(Self::encode_rev) + } + + /// Percent-encode a revision string for use in a URL. + pub fn encode_rev(rev: &str) -> String { + utf8_percent_encode(rev, GIT_REFERENCE_ENCODE_SET).to_string() + } + /// Returns the kind of this reference. pub fn kind_str(&self) -> &str { match self { diff --git a/crates/uv/tests/it/pip_install.rs b/crates/uv/tests/it/pip_install.rs index 82710c16475..3e573f735e2 100644 --- a/crates/uv/tests/it/pip_install.rs +++ b/crates/uv/tests/it/pip_install.rs @@ -3,7 +3,7 @@ use std::io::Cursor; use std::path::PathBuf; use std::process::Command; -use anyhow::Result; +use anyhow::{Result, anyhow}; use assert_cmd::prelude::*; use assert_fs::prelude::*; use flate2::write::GzEncoder; @@ -2315,6 +2315,113 @@ fn install_implicit_git_public_https() { context.assert_installed("uv_public_pypackage", "0.1.0"); } +/// Install a package from a Git ref that contains a percent-encoded `@`. +#[test] +#[cfg(feature = "test-git")] +fn install_git_percent_encoded_ref() -> Result<()> { + let context = uv_test::test_context!(DEFAULT_PYTHON_VERSION); + + let repository = context.temp_dir.child("repository"); + repository + .child("packages/example/example") + .create_dir_all()?; + repository + .child("packages/example/example/__init__.py") + .write_str(r#"__version__ = "0.1.0""#)?; + repository + .child("packages/example/pyproject.toml") + .write_str(indoc! {r#" + [project] + name = "example" + version = "0.1.0" + requires-python = ">=3.12" + + [build-system] + requires = ["hatchling"] + build-backend = "hatchling.build" + "#})?; + + Command::new("git") + .arg("init") + .arg(repository.path()) + .assert() + .success(); + Command::new("git") + .arg("-C") + .arg(repository.path()) + .arg("add") + .arg(".") + .assert() + .success(); + Command::new("git") + .arg("-C") + .arg(repository.path()) + .arg("-c") + .arg("user.name=Example") + .arg("-c") + .arg("user.email=example@example.com") + .arg("commit") + .arg("-m") + .arg("Initial commit") + .env("GIT_AUTHOR_DATE", "2000-01-01T00:00:00Z") + .env("GIT_COMMITTER_DATE", "2000-01-01T00:00:00Z") + .assert() + .success(); + Command::new("git") + .arg("-C") + .arg(repository.path()) + .arg("tag") + .arg("pkg@1.2.3") + .assert() + .success(); + + let repository_url = Url::from_directory_path(repository.path()) + .map_err(|()| anyhow!("failed to convert repository path to file URL"))?; + let repository_url = repository_url.as_str().trim_end_matches('/'); + + let mut filters = context.filters(); + filters.push((r"@[0-9a-f]{40}", "@[COMMIT]")); + uv_snapshot!(filters, context + .pip_install() + .arg(format!( + "example @ git+{repository_url}@pkg%401.2.3#subdirectory=packages/example" + )), @" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + example==0.1.0 (from git+file://[TEMP_DIR]/repository@[COMMIT]#subdirectory=packages/example) + "); + + context.assert_installed("example", "0.1.0"); + + Ok(()) +} + +/// Reject an ambiguous Git URL when the ref contains an unescaped `@`. +#[test] +fn install_git_unescaped_ref() { + let context = uv_test::test_context!(DEFAULT_PYTHON_VERSION); + + uv_snapshot!(context.filters(), context + .pip_install() + .arg("example @ git+https://example.com/repository@pkg@1.2.3"), @" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: Failed to parse: `example @ git+https://example.com/repository@pkg@1.2.3` + Caused by: Ambiguous Git URL `https://example.com/repository@pkg@1.2.3`: the path contains multiple `@` characters. If the Git revision contains `@`, percent-encode it as `%40` + example @ git+https://example.com/repository@pkg@1.2.3 + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + "); +} + /// Install and update a package from a public GitHub repository #[test] #[cfg(feature = "test-git")] From 72a0d2a2c57c54d1cbe5f7ff70caf570c4de0c20 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Thu, 7 May 2026 15:10:42 -0700 Subject: [PATCH 2/2] Add a backstop test for SSH usernames Signed-off-by: William Woodruff --- crates/uv-git-types/src/lib.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/crates/uv-git-types/src/lib.rs b/crates/uv-git-types/src/lib.rs index e59fb5bb8d6..94f81b5f02c 100644 --- a/crates/uv-git-types/src/lib.rs +++ b/crates/uv-git-types/src/lib.rs @@ -310,6 +310,21 @@ mod tests { Ok(()) } + #[test] + fn parse_ssh_url_with_username_and_percent_encoded_reference() + -> Result<(), Box> { + let url = DisplaySafeUrl::parse("ssh://git@github.com/example/example.git@abc%401.2.3")?; + let git = GitUrl::try_from(url)?; + + assert_eq!( + git.url().as_str(), + "ssh://git@github.com/example/example.git" + ); + assert_eq!(git.reference().as_str(), Some("abc@1.2.3")); + + Ok(()) + } + #[test] fn reject_ambiguous_reference() -> Result<(), Box> { let url = DisplaySafeUrl::parse("https://example.com/pkg.git@dev@1.2.3")?;