Deprecate unexpected ZIP compression methods (#17946)

Author: woodruffw

Cargo.lock

@@ -340,9 +340,14 @@ async fn download_and_unpack(
 
     let id = reporter.on_download_start(binary.name(), version, size);
     let mut progress_reader = ProgressReader::new(reader, id, reporter);
-    stream::archive(&mut progress_reader, format.into(), temp_dir.path())
-        .await
-        .map_err(|e| Error::Extract { source: e })?;
+    stream::archive(
+        download_url,
+        &mut progress_reader,
+        format.into(),
+        temp_dir.path(),
+    )
+    .await
+    .map_err(|e| Error::Extract { source: e })?;
     reporter.on_download_complete(id);
 
     // Find the binary in the extracted files

crates/uv-bin-install/src/lib.rs

@@ -47,7 +47,7 @@ pub(crate) async fn validate_zip(
 
     let target = tempfile::TempDir::new()?;
 
-    uv_extract::stream::unzip(reader.compat(), target.path()).await?;
+    uv_extract::stream::unzip(args.url.to_url(), reader.compat(), target.path()).await?;
 
     Ok(())
 }

crates/uv-dev/src/validate_zip.rs

@@ -606,6 +606,8 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
         // Create an entry for the HTTP cache.
         let http_entry = wheel_entry.with_file(format!("{}.http", filename.cache_key()));
 
+        let query_url = &url.clone();
+
         let download = |response: reqwest::Response| {
             async {
                 let size = size.or_else(|| content_length(&response));
@@ -634,7 +636,7 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
                         let mut reader = ProgressReader::new(&mut hasher, progress, &**reporter);
                         match extension {
                             WheelExtension::Whl => {
-                                uv_extract::stream::unzip(&mut reader, temp_dir.path())
+                                uv_extract::stream::unzip(query_url, &mut reader, temp_dir.path())
                                     .await
                                     .map_err(|err| Error::Extract(filename.to_string(), err))?;
                             }
@@ -647,7 +649,7 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
                     }
                     None => match extension {
                         WheelExtension::Whl => {
-                            uv_extract::stream::unzip(&mut hasher, temp_dir.path())
+                            uv_extract::stream::unzip(query_url, &mut hasher, temp_dir.path())
                                 .await
                                 .map_err(|err| Error::Extract(filename.to_string(), err))?;
                         }
@@ -777,6 +779,8 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
         // Create an entry for the HTTP cache.
         let http_entry = wheel_entry.with_file(format!("{}.http", filename.cache_key()));
 
+        let query_url = &url.clone();
+
         let download = |response: reqwest::Response| {
             async {
                 let size = size.or_else(|| content_length(&response));
@@ -856,7 +860,7 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
 
                     match extension {
                         WheelExtension::Whl => {
-                            uv_extract::stream::unzip(&mut hasher, temp_dir.path())
+                            uv_extract::stream::unzip(query_url, &mut hasher, temp_dir.path())
                                 .await
                                 .map_err(|err| Error::Extract(filename.to_string(), err))?;
                         }
@@ -1046,7 +1050,7 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
             // Unzip the wheel to a temporary directory.
             match extension {
                 WheelExtension::Whl => {
-                    uv_extract::stream::unzip(&mut hasher, temp_dir.path())
+                    uv_extract::stream::unzip(path.display(), &mut hasher, temp_dir.path())
                         .await
                         .map_err(|err| Error::Extract(filename.to_string(), err))?;
                 }

crates/uv-distribution/src/distribution_database.rs

@@ -792,6 +792,8 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
         };
 
         let download = |response| {
+            let query_url = url.clone();
+
             async {
                 // At this point, we're seeing a new or updated source distribution. Initialize a
                 // new revision, to collect the source and built artifacts.
@@ -802,7 +804,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
                 let entry = cache_shard.shard(revision.id()).entry(SOURCE);
                 let algorithms = hashes.algorithms();
                 let hashes = self
-                    .download_archive(response, source, ext, entry.path(), &algorithms)
+                    .download_archive(query_url, response, source, ext, entry.path(), &algorithms)
                     .await?;
 
                 Ok(revision.with_hashes(HashDigests::from(hashes)))
@@ -2242,6 +2244,8 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
         };
 
         let download = |response| {
+            let query_url = url.clone();
+
             async {
                 // Take the union of the requested and existing hash algorithms.
                 let algorithms = {
@@ -2255,7 +2259,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
                 };
 
                 let hashes = self
-                    .download_archive(response, source, ext, entry.path(), &algorithms)
+                    .download_archive(query_url, response, source, ext, entry.path(), &algorithms)
                     .await?;
                 for existing in revision.hashes() {
                     if !hashes.contains(existing) {
@@ -2289,6 +2293,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
     /// Download and unzip a source distribution into the cache from an HTTP response.
     async fn download_archive(
         &self,
+        query_url: DisplaySafeUrl,
         response: Response,
         source: &BuildableSource<'_>,
         ext: SourceDistExtension,
@@ -2301,6 +2306,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
                 .bucket(CacheBucket::SourceDistributions),
         )
         .map_err(Error::CacheWrite)?;
+
         let reader = response
             .bytes_stream()
             .map_err(std::io::Error::other)
@@ -2316,7 +2322,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
 
         // Download and unzip the source distribution into a temporary directory.
         let span = info_span!("download_source_dist", source_dist = %source);
-        uv_extract::stream::archive(&mut hasher, ext, temp_dir.path())
+        uv_extract::stream::archive(query_url, &mut hasher, ext, temp_dir.path())
             .await
             .map_err(|err| Error::Extract(source.to_string(), err))?;
         drop(span);
@@ -2385,7 +2391,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
         let mut hasher = uv_extract::hash::HashReader::new(reader, &mut hashers);
 
         // Unzip the archive into a temporary directory.
-        uv_extract::stream::archive(&mut hasher, ext, &temp_dir.path())
+        uv_extract::stream::archive(path.display(), &mut hasher, ext, &temp_dir.path())
             .await
             .map_err(|err| Error::Extract(temp_dir.path().to_string_lossy().into_owned(), err))?;
 

crates/uv-distribution/src/source/mod.rs

@@ -20,6 +20,7 @@ uv-configuration = { workspace = true }
 uv-distribution-filename = { workspace = true }
 uv-pypi-types = { workspace = true }
 uv-static = { workspace = true }
+uv-warnings = { workspace = true }
 
 astral-tokio-tar = { workspace = true }
 async-compression = { workspace = true, features = ["bzip2", "gzip", "zstd", "xz"] }

crates/uv-extract/Cargo.toml

@@ -1,4 +1,4 @@
-use std::sync::LazyLock;
+use std::{fmt::Display, sync::LazyLock};
 
 pub use error::Error;
 use regex::Regex;
@@ -14,6 +14,64 @@ mod vendor;
 static CONTROL_CHARACTERS_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"\p{C}").unwrap());
 static REPLACEMENT_CHARACTER: &str = "\u{FFFD}";
 
+/// Compression methods that we consider supported.
+///
+/// Our underlying ZIP dependencies may support more.
+pub(crate) enum CompressionMethod {
+    Stored,
+    Deflated,
+    Zstd,
+    // NOTE: This will become `Unsupported(...)` in the future.
+    Deprecated(&'static str),
+}
+
+impl CompressionMethod {
+    /// Returns `true` if this is a well-known compression method that we
+    /// expect other ZIP implementations to support.
+    pub(crate) fn is_well_known(&self) -> bool {
+        matches!(self, Self::Stored | Self::Deflated | Self::Zstd)
+    }
+}
+
+impl Display for CompressionMethod {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::Stored => write!(f, "stored"),
+            Self::Deflated => write!(f, "DEFLATE"),
+            Self::Zstd => write!(f, "zstd"),
+            Self::Deprecated(name) => write!(f, "{name}"),
+        }
+    }
+}
+
+impl From<async_zip::Compression> for CompressionMethod {
+    fn from(value: async_zip::Compression) -> Self {
+        match value {
+            async_zip::Compression::Stored => Self::Stored,
+            async_zip::Compression::Deflate => Self::Deflated,
+            async_zip::Compression::Zstd => Self::Zstd,
+            async_zip::Compression::Bz => Self::Deprecated("bzip2"),
+            async_zip::Compression::Lzma => Self::Deprecated("lzma"),
+            async_zip::Compression::Xz => Self::Deprecated("xz"),
+            _ => Self::Deprecated("unknown"),
+        }
+    }
+}
+
+impl From<zip::CompressionMethod> for CompressionMethod {
+    fn from(value: zip::CompressionMethod) -> Self {
+        match value {
+            zip::CompressionMethod::Stored => Self::Stored,
+            zip::CompressionMethod::Deflated => Self::Deflated,
+            zip::CompressionMethod::Zstd => Self::Zstd,
+            zip::CompressionMethod::Bzip2 => Self::Deprecated("bzip2"),
+            zip::CompressionMethod::Lzma => Self::Deprecated("lzma"),
+            zip::CompressionMethod::Xz => Self::Deprecated("xz"),
+            _ => Self::Deprecated("unknown"),
+        }
+    }
+}
+
 /// Validate that a given filename (e.g. reported by a ZIP archive's
 /// local file entries or central directory entries) is "safe" to use.
 ///

crates/uv-extract/src/lib.rs

@@ -1,3 +1,4 @@
+use std::fmt::Display;
 use std::path::{Component, Path, PathBuf};
 use std::pin::Pin;
 
@@ -9,8 +10,9 @@ use tokio_util::compat::{FuturesAsyncReadCompatExt, TokioAsyncReadCompatExt};
 use tracing::{debug, warn};
 
 use uv_distribution_filename::SourceDistExtension;
+use uv_warnings::warn_user_once;
 
-use crate::{Error, insecure_no_validate, validate_archive_member_name};
+use crate::{CompressionMethod, Error, insecure_no_validate, validate_archive_member_name};
 
 const DEFAULT_BUF_SIZE: usize = 128 * 1024;
 
@@ -43,7 +45,11 @@ struct ComputedEntry {
 /// This is useful for unzipping files as they're being downloaded. If the archive
 /// is already fully on disk, consider using `unzip_archive`, which can use multiple
 /// threads to work faster in that case.
-pub async fn unzip<R: tokio::io::AsyncRead + Unpin>(
+///
+/// `source_hint` is used for warning messages, to identify the source of the ZIP archive
+/// beneath the reader. It might be a URL, a file path, or something else.
+pub async fn unzip<D: Display, R: tokio::io::AsyncRead + Unpin>(
+    source_hint: D,
     reader: R,
     target: impl AsRef<Path>,
 ) -> Result<(), Error> {
@@ -79,8 +85,22 @@ pub async fn unzip<R: tokio::io::AsyncRead + Unpin>(
     let mut offset = 0;
 
     while let Some(mut entry) = zip.next_with_entry().await? {
+        let zip_entry = entry.reader().entry();
+
+        // Check for unexpected compression methods.
+        // A future version of uv will reject instead of warning about these.
+        let compression = CompressionMethod::from(zip_entry.compression());
+        if !compression.is_well_known() {
+            warn_user_once!(
+                "One or more file entries in '{source_hint}' use the '{compression}' compression method, which is not widely supported. A future version of uv will reject ZIP archives containing entries compressed with this method. Entries must be compressed with the '{stored}', '{deflate}', or '{zstd}' compression methods.",
+                stored = CompressionMethod::Stored,
+                deflate = CompressionMethod::Deflated,
+                zstd = CompressionMethod::Zstd,
+            );
+        }
+
         // Construct the (expected) path to the file on-disk.
-        let path = match entry.reader().entry().filename().as_str() {
+        let path = match zip_entry.filename().as_str() {
             Ok(path) => path,
             Err(ZipError::StringNotUtf8) => return Err(Error::LocalHeaderNotUtf8 { offset }),
             Err(err) => return Err(err.into()),
@@ -107,14 +127,14 @@ pub async fn unzip<R: tokio::io::AsyncRead + Unpin>(
             continue;
         };
 
-        let file_offset = entry.reader().entry().file_offset();
-        let expected_compressed_size = entry.reader().entry().compressed_size();
-        let expected_uncompressed_size = entry.reader().entry().uncompressed_size();
-        let expected_data_descriptor = entry.reader().entry().data_descriptor();
+        let file_offset = zip_entry.file_offset();
+        let expected_compressed_size = zip_entry.compressed_size();
+        let expected_uncompressed_size = zip_entry.uncompressed_size();
+        let expected_data_descriptor = zip_entry.data_descriptor();
 
         // Either create the directory or write the file to disk.
         let path = target.join(&relpath);
-        let is_dir = entry.reader().entry().dir()?;
+        let is_dir = zip_entry.dir()?;
         let computed = if is_dir {
             if directories.insert(path.clone()) {
                 fs_err::tokio::create_dir_all(path)
@@ -123,23 +143,23 @@ pub async fn unzip<R: tokio::io::AsyncRead + Unpin>(
             }
 
             // If this is a directory, we expect the CRC32 to be 0.
-            if entry.reader().entry().crc32() != 0 {
+            if zip_entry.crc32() != 0 {
                 if !skip_validation {
                     return Err(Error::BadCrc32 {
                         path: relpath.clone(),
                         computed: 0,
-                        expected: entry.reader().entry().crc32(),
+                        expected: zip_entry.crc32(),
                     });
                 }
             }
 
             // If this is a directory, we expect the uncompressed size to be 0.
-            if entry.reader().entry().uncompressed_size() != 0 {
+            if zip_entry.uncompressed_size() != 0 {
                 if !skip_validation {
                     return Err(Error::BadUncompressedSize {
                         path: relpath.clone(),
                         computed: 0,
-                        expected: entry.reader().entry().uncompressed_size(),
+                        expected: zip_entry.uncompressed_size(),
                     });
                 }
             }
@@ -164,7 +184,7 @@ pub async fn unzip<R: tokio::io::AsyncRead + Unpin>(
             {
                 Ok(file) => {
                     // Write the file to disk.
-                    let size = entry.reader().entry().uncompressed_size();
+                    let size = zip_entry.uncompressed_size();
                     let mut writer = if let Ok(size) = usize::try_from(size) {
                         tokio::io::BufWriter::with_capacity(std::cmp::min(size, 1024 * 1024), file)
                     } else {
@@ -744,14 +764,18 @@ pub async fn untar<R: tokio::io::AsyncRead + Unpin>(
 
 /// Unpack a `.zip`, `.tar.gz`, `.tar.bz2`, `.tar.zst`, or `.tar.xz` archive into the target directory,
 /// without requiring `Seek`.
-pub async fn archive<R: tokio::io::AsyncRead + Unpin>(
+///
+/// `source_hint` is used for warning messages, to identify the source of the archive
+/// beneath the reader. It might be a URL, a file path, or something else.
+pub async fn archive<D: Display, R: tokio::io::AsyncRead + Unpin>(
+    source_hint: D,
     reader: R,
     ext: SourceDistExtension,
     target: impl AsRef<Path>,
 ) -> Result<(), Error> {
     match ext {
         SourceDistExtension::Zip => {
-            unzip(reader, target).await?;
+            unzip(source_hint, reader, target).await?;
         }
         SourceDistExtension::Tar => {
             untar(reader, target).await?;