Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce and backtrack on invalid versions in source metadata #2954

Merged
merged 1 commit into from
Apr 10, 2024
Merged

Enforce and backtrack on invalid versions in source metadata #2954

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions crates/distribution-types/src/buildable.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::borrow::Cow;
use std::path::Path;

use pep440_rs::Version;
use url::Url;

use uv_normalize::PackageName;
Expand Down Expand Up @@ -28,6 +29,15 @@ impl BuildableSource<'_> {
}
}

/// Return the [`Version`] of the source, if available.
pub fn version(&self) -> Option<&Version> {
match self {
Self::Dist(SourceDist::Registry(dist)) => Some(&dist.filename.version),
Self::Dist(_) => None,
Self::Url(_) => None,
}
}

/// Return the [`BuildableSource`] as a [`SourceDist`], if it is a distribution.
pub fn as_dist(&self) -> Option<&SourceDist> {
match self {
Expand Down
3 changes: 3 additions & 0 deletions crates/uv-distribution/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use tokio::task::JoinError;
use zip::result::ZipError;

use distribution_filename::WheelFilenameError;
use pep440_rs::Version;
use uv_client::BetterReqwestError;
use uv_normalize::PackageName;

Expand Down Expand Up @@ -47,6 +48,8 @@ pub enum Error {
given: PackageName,
metadata: PackageName,
},
#[error("Package metadata version `{metadata}` does not match given version `{given}`")]
VersionMismatch { given: Version, metadata: Version },
#[error("Failed to parse metadata from built wheel")]
Metadata(#[from] pypi_types::MetadataError),
#[error("Failed to read `dist-info` metadata from built wheel")]
Expand Down
59 changes: 27 additions & 32 deletions crates/uv-distribution/src/source/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1109,14 +1109,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
let metadata = read_wheel_metadata(&filename, cache_shard.join(&disk_filename))?;

// Validate the metadata.
if let Some(name) = source.name() {
if metadata.name != *name {
return Err(Error::NameMismatch {
metadata: metadata.name,
given: name.clone(),
});
}
}
validate(source, &metadata)?;

debug!("Finished building: {source}");
Ok((disk_filename, filename, metadata))
Expand All @@ -1138,14 +1131,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
debug!("Found static `PKG-INFO` for: {source}");

// Validate the metadata.
if let Some(name) = source.name() {
if metadata.name != *name {
return Err(Error::NameMismatch {
metadata: metadata.name,
given: name.clone(),
});
}
}
validate(source, &metadata)?;

return Ok(Some(metadata));
}
Expand All @@ -1161,14 +1147,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
debug!("Found static `pyproject.toml` for: {source}");

// Validate the metadata.
if let Some(name) = source.name() {
if metadata.name != *name {
return Err(Error::NameMismatch {
metadata: metadata.name,
given: name.clone(),
});
}
}
validate(source, &metadata)?;

return Ok(Some(metadata));
}
Expand Down Expand Up @@ -1208,14 +1187,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
let metadata = Metadata23::parse_metadata(&content)?;

// Validate the metadata.
if let Some(name) = source.name() {
if metadata.name != *name {
return Err(Error::NameMismatch {
metadata: metadata.name,
given: name.clone(),
});
}
}
validate(source, &metadata)?;

Ok(Some(metadata))
}
Expand Down Expand Up @@ -1278,6 +1250,29 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
}
}

/// Validate that the source distribution matches the built metadata.
fn validate(source: &BuildableSource<'_>, metadata: &Metadata23) -> Result<(), Error> {
if let Some(name) = source.name() {
if metadata.name != *name {
return Err(Error::NameMismatch {
metadata: metadata.name.clone(),
given: name.clone(),
});
}
}

if let Some(version) = source.version() {
if metadata.version != *version {
return Err(Error::VersionMismatch {
metadata: metadata.version.clone(),
given: version.clone(),
});
}
}

Ok(())
}

/// Read an existing HTTP-cached [`Revision`], if it exists.
pub(crate) fn read_http_revision(cache_entry: &CacheEntry) -> Result<Option<Revision>, Error> {
match fs_err::File::open(cache_entry.path()) {
Expand Down
33 changes: 33 additions & 0 deletions crates/uv-resolver/src/pubgrub/report.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,15 @@ impl PubGrubReportFormatter<'_> {
reason: reason.clone(),
});
}
IncompletePackage::InconsistentMetadata(reason) => {
hints.insert(
PubGrubHint::InconsistentVersionMetadata {
package: package.clone(),
version: version.clone(),
reason: reason.clone(),
},
);
}
IncompletePackage::InvalidStructure(reason) => {
hints.insert(
PubGrubHint::InvalidVersionStructure {
Expand Down Expand Up @@ -530,6 +539,15 @@ pub(crate) enum PubGrubHint {
#[derivative(PartialEq = "ignore", Hash = "ignore")]
reason: String,
},
/// Metadata for a package version was inconsistent (e.g., the package name did not match that
/// of the file).
InconsistentVersionMetadata {
package: PubGrubPackage,
#[derivative(PartialEq = "ignore", Hash = "ignore")]
version: Version,
#[derivative(PartialEq = "ignore", Hash = "ignore")]
reason: String,
},
/// The structure of a package version was invalid (e.g., multiple `.dist-info` directories).
InvalidVersionStructure {
package: PubGrubPackage,
Expand Down Expand Up @@ -629,6 +647,21 @@ impl std::fmt::Display for PubGrubHint {
textwrap::indent(reason, " ")
)
}
PubGrubHint::InconsistentVersionMetadata {
package,
version,
reason,
} => {
write!(
f,
"{}{} Metadata for {}=={} was inconsistent:\n{}",
"hint".bold().cyan(),
":".bold(),
package.bold(),
version.bold(),
textwrap::indent(reason, " ")
)
}
}
}
}
Expand Down
24 changes: 24 additions & 0 deletions crates/uv-resolver/src/resolver/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ pub(crate) enum IncompletePackage {
Offline,
/// The wheel metadata was found, but could not be parsed.
InvalidMetadata(String),
/// The wheel metadata was found, but the metadata was inconsistent.
InconsistentMetadata(String),
/// The wheel has an invalid structure.
InvalidStructure(String),
}
Expand Down Expand Up @@ -646,6 +648,13 @@ impl<
);
return Ok(None);
}
MetadataResponse::InconsistentMetadata(err) => {
self.unavailable_packages.insert(
package_name.clone(),
UnavailablePackage::InvalidMetadata(err.to_string()),
);
return Ok(None);
}
MetadataResponse::InvalidStructure(err) => {
self.unavailable_packages.insert(
package_name.clone(),
Expand Down Expand Up @@ -945,6 +954,7 @@ impl<
));
}
MetadataResponse::InvalidMetadata(err) => {
warn!("Unable to extract metadata for {package_name}: {err}");
self.incomplete_packages
.entry(package_name.clone())
.or_default()
Expand All @@ -956,7 +966,21 @@ impl<
"the package metadata could not be parsed".to_string(),
));
}
MetadataResponse::InconsistentMetadata(err) => {
warn!("Unable to extract metadata for {package_name}: {err}");
self.incomplete_packages
.entry(package_name.clone())
.or_default()
.insert(
version.clone(),
IncompletePackage::InconsistentMetadata(err.to_string()),
);
return Ok(Dependencies::Unavailable(
"the package metadata was inconsistent".to_string(),
));
}
MetadataResponse::InvalidStructure(err) => {
warn!("Unable to extract metadata for {package_name}: {err}");
self.incomplete_packages
.entry(package_name.clone())
.or_default()
Expand Down
8 changes: 8 additions & 0 deletions crates/uv-resolver/src/resolver/provider.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ pub enum MetadataResponse {
Found(Metadata23),
/// The wheel metadata was found, but could not be parsed.
InvalidMetadata(Box<pypi_types::MetadataError>),
/// The wheel metadata was found, but the metadata was inconsistent.
InconsistentMetadata(Box<uv_distribution::Error>),
/// The wheel has an invalid structure.
InvalidStructure(Box<install_wheel_rs::Error>),
/// The wheel metadata was not found in the cache and the network is not available.
Expand Down Expand Up @@ -185,6 +187,12 @@ impl<'a, Context: BuildContext + Send + Sync> ResolverProvider
}
kind => Err(uv_client::Error::from(kind).into()),
},
uv_distribution::Error::VersionMismatch { .. } => {
Ok(MetadataResponse::InconsistentMetadata(Box::new(err)))
}
uv_distribution::Error::NameMismatch { .. } => {
Ok(MetadataResponse::InconsistentMetadata(Box::new(err)))
}
uv_distribution::Error::Metadata(err) => {
Ok(MetadataResponse::InvalidMetadata(Box::new(err)))
}
Expand Down
Loading