diff --git a/crates_io_tarball/src/lib.rs b/crates_io_tarball/src/lib.rs index d73074f0d0..309ff5779a 100644 --- a/crates_io_tarball/src/lib.rs +++ b/crates_io_tarball/src/lib.rs @@ -5,8 +5,9 @@ extern crate claims; #[cfg(any(feature = "builder", test))] pub use crate::builder::TarballBuilder; use crate::limit_reader::LimitErrorReader; -pub use crate::manifest::Manifest; +use crate::manifest::validate_manifest; pub use crate::vcs_info::CargoVcsInfo; +pub use cargo_toml::Manifest; use flate2::read::GzDecoder; use std::io::Read; use std::path::Path; @@ -35,7 +36,7 @@ pub enum TarballError { #[error("Cargo.toml manifest is missing")] MissingManifest, #[error("Cargo.toml manifest is invalid: {0}")] - InvalidManifest(#[source] toml::de::Error), + InvalidManifest(#[from] cargo_toml::Error), #[error(transparent)] IO(#[from] std::io::Error), } @@ -97,7 +98,11 @@ pub fn process_tarball( // erroring if it cannot be read. let mut contents = String::new(); entry.read_to_string(&mut contents)?; - manifest = Some(toml::from_str(&contents).map_err(TarballError::InvalidManifest)?); + manifest = Some({ + let manifest = Manifest::from_str(&contents)?; + validate_manifest(&manifest)?; + manifest + }); } } @@ -178,10 +183,10 @@ repository = "https://github.com/foo/bar" let limit = 512 * 1024 * 1024; let tarball_info = assert_ok!(process_tarball("foo-0.0.1", &*tarball, limit)); - let package = tarball_info.manifest.package; - assert_some_eq!(package.readme.as_path(), Path::new("README.md")); - assert_some_eq!(package.repository, "https://github.com/foo/bar"); - assert_some_eq!(package.rust_version, "1.59"); + let package = assert_some!(tarball_info.manifest.package); + assert_some_eq!(package.readme().as_path(), Path::new("README.md")); + assert_some_eq!(package.repository(), "https://github.com/foo/bar"); + assert_some_eq!(package.rust_version(), "1.59"); } #[test] @@ -200,8 +205,8 @@ repository = "https://github.com/foo/bar" let limit = 512 * 1024 * 1024; let tarball_info = assert_ok!(process_tarball("foo-0.0.1", &*tarball, limit)); - let package = tarball_info.manifest.package; - assert_some_eq!(package.rust_version, "1.23"); + let package = assert_some!(tarball_info.manifest.package); + assert_some_eq!(package.rust_version(), "1.23"); } #[test] @@ -219,8 +224,8 @@ repository = "https://github.com/foo/bar" let limit = 512 * 1024 * 1024; let tarball_info = assert_ok!(process_tarball("foo-0.0.1", &*tarball, limit)); - let package = tarball_info.manifest.package; - assert_matches!(package.readme, OptionalFile::Flag(true)); + let package = assert_some!(tarball_info.manifest.package); + assert_matches!(package.readme(), OptionalFile::Flag(true)); } #[test] @@ -239,8 +244,8 @@ repository = "https://github.com/foo/bar" let limit = 512 * 1024 * 1024; let tarball_info = assert_ok!(process_tarball("foo-0.0.1", &*tarball, limit)); - let package = tarball_info.manifest.package; - assert_matches!(package.readme, OptionalFile::Flag(false)); + let package = assert_some!(tarball_info.manifest.package); + assert_matches!(package.readme(), OptionalFile::Flag(false)); } #[test] @@ -260,7 +265,7 @@ repository = "https://github.com/foo/bar" let limit = 512 * 1024 * 1024; let tarball_info = assert_ok!(process_tarball("foo-0.0.1", &*tarball, limit)); - let package = tarball_info.manifest.package; - assert_some_eq!(package.repository, "https://github.com/foo/bar"); + let package = assert_some!(tarball_info.manifest.package); + assert_some_eq!(package.repository(), "https://github.com/foo/bar"); } } diff --git a/crates_io_tarball/src/manifest.rs b/crates_io_tarball/src/manifest.rs index 5726f2eada..b84edbb44b 100644 --- a/crates_io_tarball/src/manifest.rs +++ b/crates_io_tarball/src/manifest.rs @@ -1,44 +1,89 @@ -use cargo_toml::OptionalFile; -use derive_deref::Deref; -use serde::{de, Deserialize, Deserializer}; - -#[derive(Debug, Deserialize)] -pub struct Manifest { - #[serde(alias = "project")] - pub package: Package, -} - -#[derive(Debug, Deserialize)] -#[serde(rename_all = "kebab-case")] -pub struct Package { - pub name: String, - pub version: String, - #[serde(default)] - pub readme: OptionalFile, - pub repository: Option, - pub rust_version: Option, -} - -#[derive(Debug, Deref)] -pub struct RustVersion(String); - -impl PartialEq<&str> for RustVersion { - fn eq(&self, other: &&str) -> bool { - self.0.eq(other) - } -} - -impl<'de> Deserialize<'de> for RustVersion { - fn deserialize>(d: D) -> Result { - let s = String::deserialize(d)?; - match semver::VersionReq::parse(&s) { - // Exclude semver operators like `^` and pre-release identifiers - Ok(_) if s.chars().all(|c| c.is_ascii_digit() || c == '.') => Ok(RustVersion(s)), - Ok(_) | Err(..) => { - let value = de::Unexpected::Str(&s); - let expected = "a valid rust_version"; - Err(de::Error::invalid_value(value, &expected)) - } - } +use cargo_toml::{Dependency, DepsSet, Error, Inheritable, Manifest, Package}; + +pub fn validate_manifest(manifest: &Manifest) -> Result<(), Error> { + let package = manifest.package.as_ref(); + + // Check that a `[package]` table exists in the manifest, since crates.io + // does not accept workspace manifests. + let package = package.ok_or(Error::Other("missing field `package`"))?; + + validate_package(package)?; + + // These checks ensure that dependency workspace inheritance has been + // normalized by cargo before publishing. + if manifest.dependencies.is_inherited() + || manifest.dev_dependencies.is_inherited() + || manifest.build_dependencies.is_inherited() + { + return Err(Error::InheritedUnknownValue); + } + + Ok(()) +} + +pub fn validate_package(package: &Package) -> Result<(), Error> { + // These checks ensure that package field workspace inheritance has been + // normalized by cargo before publishing. + if package.edition.is_inherited() + || package.rust_version.is_inherited() + || package.version.is_inherited() + || package.authors.is_inherited() + || package.description.is_inherited() + || package.homepage.is_inherited() + || package.documentation.is_inherited() + || package.readme.is_inherited() + || package.keywords.is_inherited() + || package.categories.is_inherited() + || package.exclude.is_inherited() + || package.include.is_inherited() + || package.license.is_inherited() + || package.license_file.is_inherited() + || package.repository.is_inherited() + || package.publish.is_inherited() + { + return Err(Error::InheritedUnknownValue); + } + + // Check that the `rust-version` field has a valid value, if it exists. + if let Some(rust_version) = package.rust_version() { + validate_rust_version(rust_version)?; + } + + Ok(()) +} + +trait IsInherited { + fn is_inherited(&self) -> bool; +} + +impl IsInherited for Inheritable { + fn is_inherited(&self) -> bool { + !self.is_set() + } +} + +impl IsInherited for Option { + fn is_inherited(&self) -> bool { + self.as_ref().map(|it| it.is_inherited()).unwrap_or(false) + } +} + +impl IsInherited for Dependency { + fn is_inherited(&self) -> bool { + matches!(self, Dependency::Inherited(_)) + } +} + +impl IsInherited for DepsSet { + fn is_inherited(&self) -> bool { + self.iter().any(|(_key, dep)| dep.is_inherited()) + } +} + +pub fn validate_rust_version(value: &str) -> Result<(), Error> { + match semver::VersionReq::parse(value) { + // Exclude semver operators like `^` and pre-release identifiers + Ok(_) if value.chars().all(|c| c.is_ascii_digit() || c == '.') => Ok(()), + Ok(_) | Err(..) => Err(Error::Other("invalid `rust-version` value")), } } diff --git a/src/admin/render_readmes.rs b/src/admin/render_readmes.rs index 97ad253fb6..9f21460939 100644 --- a/src/admin/render_readmes.rs +++ b/src/admin/render_readmes.rs @@ -177,11 +177,14 @@ fn render_pkg_readme(mut archive: Archive, pkg_name: &str) -> anyhow let contents = find_file_by_path(&mut entries, Path::new(&path)) .context("Failed to read Cargo.toml file")?; - toml::from_str(&contents).context("Failed to parse manifest file")? + Manifest::from_str(&contents).context("Failed to parse manifest file")? + + // We don't call `validate_manifest()` here since the additional validation is not needed + // and it would prevent us from reading a couple of legacy crate files. }; let rendered = { - let readme = manifest.package.readme; + let readme = manifest.package().readme(); if !readme.is_some() { return Ok("".to_string()); } @@ -198,7 +201,7 @@ fn render_pkg_readme(mut archive: Archive, pkg_name: &str) -> anyhow text_to_html( &contents, &readme_path, - manifest.package.repository.as_deref(), + manifest.package().repository(), pkg_path_in_vcs, ) }; diff --git a/src/controllers/krate/publish.rs b/src/controllers/krate/publish.rs index a0354ce69c..c6d0c1b5dc 100644 --- a/src/controllers/krate/publish.rs +++ b/src/controllers/krate/publish.rs @@ -197,8 +197,8 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult