From c8361d8eace818ec890ffabb00b8b2984d04ebeb Mon Sep 17 00:00:00 2001
From: Tobias Bieniek <tobias@bieniek.cloud>
Date: Tue, 4 Jun 2024 13:15:26 +0200
Subject: [PATCH] crates_io_tarball: Disallow paths differing only by case

---
 crates/crates_io_tarball/src/lib.rs | 51 +++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 13 deletions(-)

diff --git a/crates/crates_io_tarball/src/lib.rs b/crates/crates_io_tarball/src/lib.rs
index 732fc3eccfe..d89426536ba 100644
--- a/crates/crates_io_tarball/src/lib.rs
+++ b/crates/crates_io_tarball/src/lib.rs
@@ -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;
@@ -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")]
@@ -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),
 }
@@ -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)?;
@@ -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) {
@@ -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
@@ -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")
     }
 }