Skip to content

Commit bfdb89d

Browse files
committed
Eagerly reject unsupported Git schemes
Initially, we were limiting Git schemes to HTTPS and SSH as only supported schemes. We lost this validation in #3429. This incidentally allow file schemes, which apparently work with Git out of the box. A caveat for this is that in `tool.uv.sources`, we parse the `git` field always as URL. This caused a problem with #11425: `repo = { git = 'c:\path\to\repo', rev = "xxxxx" }` was parsed as a URL where `c:` is the scheme, causing a bad error message down the line. This PR: * Puts Git URL validation back in place * Allows `file:` URL in Git: This seems to be supported by Git and we were supporting it albeit unintentionally, so it's reasonable to continue to support it. * It does _not_ allow relative paths in the `git` field in `tool.uv.sources`. Absolute file URLs are supported, whether we want relative file URLs for Git too should be discussed separately. Closes #3429: We reject the input with a proper error message, while hinting the user towards `file:`. If there's still desire for a relative path, we can reopen.
1 parent 248da23 commit bfdb89d

File tree

18 files changed

+207
-166
lines changed

18 files changed

+207
-166
lines changed

Cargo.lock

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/uv-distribution-types/src/lib.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -714,9 +714,7 @@ impl GitSourceDist {
714714
/// Return the [`ParsedUrl`] for the distribution.
715715
pub fn parsed_url(&self) -> ParsedUrl {
716716
ParsedUrl::Git(ParsedGitUrl::from_source(
717-
self.git.repository().clone(),
718-
self.git.reference().clone(),
719-
self.git.precise(),
717+
(*self.git).clone(),
720718
self.subdirectory.clone(),
721719
))
722720
}

crates/uv-distribution-types/src/resolution.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -266,10 +266,8 @@ impl From<&ResolvedDist> for RequirementSource {
266266
}
267267
}
268268
Dist::Source(SourceDist::Git(sdist)) => RequirementSource::Git {
269+
git: (*sdist.git).clone(),
269270
url: sdist.url.clone(),
270-
repository: sdist.git.repository().clone(),
271-
reference: sdist.git.reference().clone(),
272-
precise: sdist.git.precise(),
273271
subdirectory: sdist.subdirectory.clone(),
274272
},
275273
Dist::Source(SourceDist::Path(sdist)) => RequirementSource::Path {

crates/uv-distribution/src/metadata/lowering.rs

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use url::Url;
88

99
use uv_distribution_filename::DistExtension;
1010
use uv_distribution_types::{Index, IndexLocations, IndexName, Origin};
11-
use uv_git_types::GitReference;
11+
use uv_git_types::{GitReference, GitUrl, GitUrlParseError};
1212
use uv_normalize::{ExtraName, GroupName, PackageName};
1313
use uv_pep440::VersionSpecifiers;
1414
use uv_pep508::{looks_like_git_repository, MarkerTree, VerbatimUrl, VersionOrUrl};
@@ -291,9 +291,7 @@ impl LoweredRequirement {
291291
.expect("Workspace member must be relative");
292292
let subdirectory = uv_fs::normalize_path_buf(subdirectory);
293293
RequirementSource::Git {
294-
repository: git_member.git_source.git.repository().clone(),
295-
reference: git_member.git_source.git.reference().clone(),
296-
precise: git_member.git_source.git.precise(),
294+
git: git_member.git_source.git.clone(),
297295
subdirectory: if subdirectory == PathBuf::new() {
298296
None
299297
} else {
@@ -497,6 +495,8 @@ pub enum LoweringError {
497495
UndeclaredWorkspacePackage(PackageName),
498496
#[error("Can only specify one of: `rev`, `tag`, or `branch`")]
499497
MoreThanOneGitRef,
498+
#[error(transparent)]
499+
GitUrlParse(#[from] GitUrlParseError),
500500
#[error("Package `{0}` references an undeclared index: `{1}`")]
501501
MissingIndex(PackageName, IndexName),
502502
#[error("Workspace members are not allowed in non-workspace contexts")]
@@ -575,9 +575,7 @@ fn git_source(
575575

576576
Ok(RequirementSource::Git {
577577
url,
578-
repository,
579-
reference,
580-
precise: None,
578+
git: GitUrl::from_reference(repository, reference)?,
581579
subdirectory,
582580
})
583581
}
@@ -679,9 +677,7 @@ fn path_source(
679677
.expect("Workspace member must be relative");
680678
let subdirectory = uv_fs::normalize_path_buf(subdirectory);
681679
return Ok(RequirementSource::Git {
682-
repository: git_member.git_source.git.repository().clone(),
683-
reference: git_member.git_source.git.reference().clone(),
684-
precise: git_member.git_source.git.precise(),
680+
git: git_member.git_source.git.clone(),
685681
subdirectory: if subdirectory == PathBuf::new() {
686682
None
687683
} else {

crates/uv-git-types/src/lib.rs

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,20 @@
11
pub use crate::github::GitHubRepository;
22
pub use crate::oid::{GitOid, OidParseError};
33
pub use crate::reference::GitReference;
4+
5+
use thiserror::Error;
46
use url::Url;
57

68
mod github;
79
mod oid;
810
mod reference;
911

12+
#[derive(Debug, Error)]
13+
pub enum GitUrlParseError {
14+
#[error("Unsupported Git URL scheme `{0}:` in `{1}`, only `https:`, `ssh:` and `file:` are supported")]
15+
UnsupportedGitScheme(String, Url),
16+
}
17+
1018
/// A URL reference to a Git repository.
1119
#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Hash, Ord)]
1220
pub struct GitUrl {
@@ -21,21 +29,42 @@ pub struct GitUrl {
2129

2230
impl GitUrl {
2331
/// Create a new [`GitUrl`] from a repository URL and a reference.
24-
pub fn from_reference(repository: Url, reference: GitReference) -> Self {
25-
Self {
26-
repository,
27-
reference,
28-
precise: None,
29-
}
32+
pub fn from_reference(
33+
repository: Url,
34+
reference: GitReference,
35+
) -> Result<Self, GitUrlParseError> {
36+
Self::from_fields(repository, reference, None)
3037
}
3138

3239
/// Create a new [`GitUrl`] from a repository URL and a precise commit.
33-
pub fn from_commit(repository: Url, reference: GitReference, precise: GitOid) -> Self {
34-
Self {
40+
pub fn from_commit(
41+
repository: Url,
42+
reference: GitReference,
43+
precise: GitOid,
44+
) -> Result<Self, GitUrlParseError> {
45+
Self::from_fields(repository, reference, Some(precise))
46+
}
47+
48+
/// Create a new [`GitUrl`] from a repository URL and a precise commit, if known.
49+
pub fn from_fields(
50+
repository: Url,
51+
reference: GitReference,
52+
precise: Option<GitOid>,
53+
) -> Result<Self, GitUrlParseError> {
54+
match repository.scheme() {
55+
"https" | "ssh" | "file" => {}
56+
unsupported => {
57+
return Err(GitUrlParseError::UnsupportedGitScheme(
58+
unsupported.to_string(),
59+
repository,
60+
))
61+
}
62+
}
63+
Ok(Self {
3564
repository,
3665
reference,
37-
precise: Some(precise),
38-
}
66+
precise,
67+
})
3968
}
4069

4170
/// Set the precise [`GitOid`] to use for this Git URL.
@@ -69,7 +98,7 @@ impl GitUrl {
6998
}
7099

71100
impl TryFrom<Url> for GitUrl {
72-
type Error = OidParseError;
101+
type Error = GitUrlParseError;
73102

74103
/// Initialize a [`GitUrl`] source from a URL.
75104
fn try_from(mut url: Url) -> Result<Self, Self::Error> {
@@ -89,7 +118,7 @@ impl TryFrom<Url> for GitUrl {
89118
url.set_path(&prefix);
90119
}
91120

92-
Ok(Self::from_reference(url, reference))
121+
Self::from_reference(url, reference)
93122
}
94123
}
95124

crates/uv-installer/src/satisfies.rs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,7 @@ impl RequirementSatisfaction {
9797
}
9898
RequirementSource::Git {
9999
url: _,
100-
repository: requested_repository,
101-
reference: _,
102-
precise: requested_precise,
100+
git: requested_git,
103101
subdirectory: requested_subdirectory,
104102
} => {
105103
let InstalledDist::Url(InstalledDirectUrlDist { direct_url, .. }) = &distribution
@@ -129,21 +127,25 @@ impl RequirementSatisfaction {
129127
}
130128

131129
if !RepositoryUrl::parse(installed_url).is_ok_and(|installed_url| {
132-
installed_url == RepositoryUrl::new(requested_repository)
130+
installed_url == RepositoryUrl::new(requested_git.repository())
133131
}) {
134132
debug!(
135133
"Repository mismatch: {:?} vs. {:?}",
136-
installed_url, requested_repository
134+
installed_url,
135+
requested_git.repository()
137136
);
138137
return Ok(Self::Mismatch);
139138
}
140139

141140
// TODO(charlie): It would be more consistent for us to compare the requested
142141
// revisions here.
143-
if installed_precise.as_deref() != requested_precise.as_ref().map(GitOid::as_str) {
142+
if installed_precise.as_deref()
143+
!= requested_git.precise().as_ref().map(GitOid::as_str)
144+
{
144145
debug!(
145146
"Precise mismatch: {:?} vs. {:?}",
146-
installed_precise, requested_precise
147+
installed_precise,
148+
requested_git.precise()
147149
);
148150
return Ok(Self::OutOfDate);
149151
}

crates/uv-pypi-types/src/parsed_url.rs

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use thiserror::Error;
55
use url::{ParseError, Url};
66

77
use uv_distribution_filename::{DistExtension, ExtensionError};
8-
use uv_git_types::{GitOid, GitReference, GitUrl, OidParseError};
8+
use uv_git_types::{GitUrl, GitUrlParseError};
99
use uv_pep508::{
1010
looks_like_git_repository, Pep508Url, UnnamedRequirementUrl, VerbatimUrl, VerbatimUrlError,
1111
};
@@ -22,8 +22,8 @@ pub enum ParsedUrlError {
2222
},
2323
#[error("Invalid path in file URL: `{0}`")]
2424
InvalidFileUrl(String),
25-
#[error("Failed to parse Git reference from URL: `{0}`")]
26-
GitOidParse(String, #[source] OidParseError),
25+
#[error(transparent)]
26+
GitUrlParse(#[from] GitUrlParseError),
2727
#[error("Not a valid URL: `{0}`")]
2828
UrlParse(String, #[source] ParseError),
2929
#[error(transparent)]
@@ -244,17 +244,7 @@ pub struct ParsedGitUrl {
244244

245245
impl ParsedGitUrl {
246246
/// Construct a [`ParsedGitUrl`] from a Git requirement source.
247-
pub fn from_source(
248-
repository: Url,
249-
reference: GitReference,
250-
precise: Option<GitOid>,
251-
subdirectory: Option<PathBuf>,
252-
) -> Self {
253-
let url = if let Some(precise) = precise {
254-
GitUrl::from_commit(repository, reference, precise)
255-
} else {
256-
GitUrl::from_reference(repository, reference)
257-
};
247+
pub fn from_source(url: GitUrl, subdirectory: Option<PathBuf>) -> Self {
258248
Self { url, subdirectory }
259249
}
260250
}
@@ -274,8 +264,7 @@ impl TryFrom<Url> for ParsedGitUrl {
274264
.strip_prefix("git+")
275265
.unwrap_or(url_in.as_str());
276266
let url = Url::parse(url).map_err(|err| ParsedUrlError::UrlParse(url.to_string(), err))?;
277-
let url = GitUrl::try_from(url)
278-
.map_err(|err| ParsedUrlError::GitOidParse(url_in.to_string(), err))?;
267+
let url = GitUrl::try_from(url)?;
279268
Ok(Self { url, subdirectory })
280269
}
281270
}

0 commit comments

Comments
 (0)