refactor(registry): extract the general unpack logic

Author: weihanglo

src/cargo/sources/registry/mod.rs

@@ -630,60 +630,8 @@ impl<'gctx> RegistrySource<'gctx> {
             Err(e) => anyhow::bail!("unable to read .cargo-ok file at {path:?}: {e}"),
         }
         dst.create_dir()?;
-        let mut tar = {
-            let size_limit = max_unpack_size(self.gctx, tarball.metadata()?.len());
-            let gz = GzDecoder::new(tarball);
-            let gz = LimitErrorReader::new(gz, size_limit);
-            let mut tar = Archive::new(gz);
-            set_mask(&mut tar);
-            tar
-        };
-        let mut bytes_written = 0;
-        let prefix = unpack_dir.file_name().unwrap();
-        let parent = unpack_dir.parent().unwrap();
-        for entry in tar.entries()? {
-            let mut entry = entry.context("failed to iterate over archive")?;
-            let entry_path = entry
-                .path()
-                .context("failed to read entry path")?
-                .into_owned();
-
-            // We're going to unpack this tarball into the global source
-            // directory, but we want to make sure that it doesn't accidentally
-            // (or maliciously) overwrite source code from other crates. Cargo
-            // itself should never generate a tarball that hits this error, and
-            // crates.io should also block uploads with these sorts of tarballs,
-            // but be extra sure by adding a check here as well.
-            if !entry_path.starts_with(prefix) {
-                anyhow::bail!(
-                    "invalid tarball downloaded, contains \
-                     a file at {:?} which isn't under {:?}",
-                    entry_path,
-                    prefix
-                )
-            }
-            // Prevent unpacking the lockfile from the crate itself.
-            if entry_path
-                .file_name()
-                .map_or(false, |p| p == PACKAGE_SOURCE_LOCK)
-            {
-                continue;
-            }
-            // Unpacking failed
-            bytes_written += entry.size();
-            let mut result = entry.unpack_in(parent).map_err(anyhow::Error::from);
-            if cfg!(windows) && restricted_names::is_windows_reserved_path(&entry_path) {
-                result = result.with_context(|| {
-                    format!(
-                        "`{}` appears to contain a reserved Windows path, \
-                        it cannot be extracted on Windows",
-                        entry_path.display()
-                    )
-                });
-            }
-            result
-                .with_context(|| format!("failed to unpack entry at `{}`", entry_path.display()))?;
-        }
+
+        let bytes_written = unpack(self.gctx, tarball, unpack_dir)?;
 
         // Now that we've finished unpacking, create and write to the lock file to indicate that
         // unpacking was successful.
@@ -1046,3 +994,62 @@ fn set_mask<R: Read>(tar: &mut Archive<R>) {
     #[cfg(unix)]
     tar.set_mask(crate::util::get_umask());
 }
+
+/// Unpack a tarball with zip bomb and overwrite protections.
+fn unpack(gctx: &GlobalContext, tarball: &File, unpack_dir: &Path) -> CargoResult<u64> {
+    let mut tar = {
+        let size_limit = max_unpack_size(gctx, tarball.metadata()?.len());
+        let gz = GzDecoder::new(tarball);
+        let gz = LimitErrorReader::new(gz, size_limit);
+        let mut tar = Archive::new(gz);
+        set_mask(&mut tar);
+        tar
+    };
+    let mut bytes_written = 0;
+    let prefix = unpack_dir.file_name().unwrap();
+    let parent = unpack_dir.parent().unwrap();
+    for entry in tar.entries()? {
+        let mut entry = entry.context("failed to iterate over archive")?;
+        let entry_path = entry
+            .path()
+            .context("failed to read entry path")?
+            .into_owned();
+
+        // We're going to unpack this tarball into the global source
+        // directory, but we want to make sure that it doesn't accidentally
+        // (or maliciously) overwrite source code from other crates. Cargo
+        // itself should never generate a tarball that hits this error, and
+        // crates.io should also block uploads with these sorts of tarballs,
+        // but be extra sure by adding a check here as well.
+        if !entry_path.starts_with(prefix) {
+            anyhow::bail!(
+                "invalid tarball downloaded, contains \
+                     a file at {:?} which isn't under {:?}",
+                entry_path,
+                prefix
+            )
+        }
+        // Prevent unpacking the lockfile from the crate itself.
+        if entry_path
+            .file_name()
+            .map_or(false, |p| p == PACKAGE_SOURCE_LOCK)
+        {
+            continue;
+        }
+        // Unpacking failed
+        bytes_written += entry.size();
+        let mut result = entry.unpack_in(parent).map_err(anyhow::Error::from);
+        if cfg!(windows) && restricted_names::is_windows_reserved_path(&entry_path) {
+            result = result.with_context(|| {
+                format!(
+                    "`{}` appears to contain a reserved Windows path, \
+                        it cannot be extracted on Windows",
+                    entry_path.display()
+                )
+            });
+        }
+        result.with_context(|| format!("failed to unpack entry at `{}`", entry_path.display()))?;
+    }
+
+    Ok(bytes_written)
+}