Skip to content

Commit

Permalink
crates_io_tarball: Disallow paths differing only by case
Browse files Browse the repository at this point in the history
  • Loading branch information
Turbo87 committed Jun 4, 2024
1 parent 3dc1848 commit c8361d8
Showing 1 changed file with 38 additions and 13 deletions.
51 changes: 38 additions & 13 deletions crates/crates_io_tarball/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::manifest::validate_manifest;
pub use crate::vcs_info::CargoVcsInfo;
pub use cargo_manifest::{Manifest, StringOrBool};
use flate2::read::GzDecoder;
use std::collections::BTreeMap;
use std::collections::{BTreeMap, HashSet};
use std::io::Read;
use std::path::{Path, PathBuf};
use std::str::FromStr;
Expand All @@ -33,6 +33,8 @@ pub enum TarballError {
Malformed(#[source] std::io::Error),
#[error("invalid path found: {0}")]
InvalidPath(String),
#[error("duplicate path found: {0}")]
DuplicatePath(String),
#[error("unexpected symlink or hard link found: {0}")]
UnexpectedSymlink(String),
#[error("Cargo.toml manifest is missing")]
Expand All @@ -41,8 +43,6 @@ pub enum TarballError {
InvalidManifest(#[from] cargo_manifest::Error),
#[error("Cargo.toml manifest is incorrectly cased: {0:?}")]
IncorrectlyCasedManifest(PathBuf),
#[error("more than one Cargo.toml manifest in tarball: {0:?}")]
TooManyManifests(Vec<PathBuf>),
#[error(transparent)]
IO(#[from] std::io::Error),
}
Expand All @@ -67,6 +67,7 @@ pub fn process_tarball<R: Read>(

let mut vcs_info = None;
let mut manifests = BTreeMap::new();
let mut paths = HashSet::new();

for entry in archive.entries()? {
let mut entry = entry.map_err(TarballError::Malformed)?;
Expand All @@ -93,6 +94,13 @@ pub fn process_tarball<R: Read>(
));
}

let lowercase_path = entry_path.as_os_str().to_ascii_lowercase();
if !paths.insert(lowercase_path) {
return Err(TarballError::DuplicatePath(
entry_path.display().to_string(),
));
}

// Let's go hunting for the VCS info and crate manifest. The only valid place for these is
// in the package root in the tarball.
if entry_path.parent() == Some(pkg_root) {
Expand All @@ -116,13 +124,6 @@ pub fn process_tarball<R: Read>(
}
}

if manifests.len() > 1 {
// There are no scenarios where we want to accept a crate file with multiple manifests.
return Err(TarballError::TooManyManifests(
manifests.into_keys().collect(),
));
}

// Although we're interested in all possible cases of `Cargo.toml` above to protect users
// on case-insensitive filesystems, to match the behaviour of cargo we should only actually
// accept `Cargo.toml` and (the now deprecated) `cargo.toml` as valid options for the
Expand Down Expand Up @@ -301,12 +302,36 @@ mod tests {
};

let err = assert_err!(process(vec!["cargo.toml", "Cargo.toml"]));
assert_snapshot!(err, @r###"more than one Cargo.toml manifest in tarball: ["foo-0.0.1/Cargo.toml", "foo-0.0.1/cargo.toml"]"###);
assert_snapshot!(err, @"duplicate path found: foo-0.0.1/Cargo.toml");

let err = assert_err!(process(vec!["Cargo.toml", "Cargo.Toml"]));
assert_snapshot!(err, @r###"more than one Cargo.toml manifest in tarball: ["foo-0.0.1/Cargo.Toml", "foo-0.0.1/Cargo.toml"]"###);
assert_snapshot!(err, @"duplicate path found: foo-0.0.1/Cargo.Toml");

let err = assert_err!(process(vec!["Cargo.toml", "cargo.toml", "CARGO.TOML"]));
assert_snapshot!(err, @r###"more than one Cargo.toml manifest in tarball: ["foo-0.0.1/CARGO.TOML", "foo-0.0.1/Cargo.toml", "foo-0.0.1/cargo.toml"]"###);
assert_snapshot!(err, @"duplicate path found: foo-0.0.1/cargo.toml");
}

#[test]
fn test_duplicate_paths() {
let tarball = TarballBuilder::new()
.add_file("foo-0.0.1/Cargo.toml", MANIFEST)
.add_file("foo-0.0.1/foo.rs", b"")
.add_file("foo-0.0.1/foo.rs", b"")
.build();

let err = assert_err!(process_tarball("foo-0.0.1", &*tarball, MAX_SIZE));
assert_snapshot!(err, @"duplicate path found: foo-0.0.1/foo.rs")
}

#[test]
fn test_case_insensitivity() {
let tarball = TarballBuilder::new()
.add_file("foo-0.0.1/Cargo.toml", MANIFEST)
.add_file("foo-0.0.1/foo.rs", b"")
.add_file("foo-0.0.1/FOO.rs", b"")
.build();

let err = assert_err!(process_tarball("foo-0.0.1", &*tarball, MAX_SIZE));
assert_snapshot!(err, @"duplicate path found: foo-0.0.1/FOO.rs")
}
}

0 comments on commit c8361d8

Please sign in to comment.