From 5c74d31eb8ebffaaa0de7fe02026eaf1ced77003 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Mon, 9 Feb 2026 12:53:15 -0500 Subject: [PATCH] Warn on unexpected ZIP compression methods (#17885) ## Summary This adds warnings to both our steam and sync ZIP handling on ZIP entries that aren't "well-known." For now, "well-known" means stored (i.e. no compression), DEFLATE, or zstd. In practice we have duplicated codepaths for this check: one for the "sync" (pre-downloaded) path, and one for the streaming path. See #16911 and #17467 for context. ## Test Plan Will update snapshots if/when they change. I'll also add a ZIP test for this. (Upd: added some "futzed" wheels for the ZIP tests.) --------- Signed-off-by: William Woodruff --- Cargo.lock | 1 + crates/uv-bin-install/src/lib.rs | 11 +++- crates/uv-dev/src/validate_zip.rs | 2 +- .../src/distribution_database.rs | 34 +++++++---- crates/uv-distribution/src/source/mod.rs | 7 ++- crates/uv-extract/Cargo.toml | 1 + crates/uv-extract/src/stream.rs | 60 ++++++++++++++----- crates/uv-extract/src/sync.rs | 30 ++++++++-- .../src/vendor/cloneable_seekable_reader.rs | 2 +- crates/uv-extract/src/vendor/mod.rs | 2 +- crates/uv-python/src/downloads.rs | 4 +- crates/uv-test/src/lib.rs | 15 +++++ crates/uv/src/commands/build_frontend.rs | 5 +- crates/uv/tests/it/extract.rs | 2 +- crates/uv/tests/it/pip_install.rs | 47 +++++++++++++++ 15 files changed, 180 insertions(+), 43 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index da4797e55..4c09fad8b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6340,6 +6340,7 @@ dependencies = [ "uv-distribution-filename", "uv-pypi-types", "uv-static", + "uv-warnings", "xz2", "zip", "zstd", diff --git a/crates/uv-bin-install/src/lib.rs b/crates/uv-bin-install/src/lib.rs index aae31d205..aae885c5f 100644 --- a/crates/uv-bin-install/src/lib.rs +++ b/crates/uv-bin-install/src/lib.rs @@ -339,9 +339,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( + DisplaySafeUrl::from_url(download_url.clone()), + &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 diff --git a/crates/uv-dev/src/validate_zip.rs b/crates/uv-dev/src/validate_zip.rs index 70ec215b0..ca7b4f1f1 100644 --- a/crates/uv-dev/src/validate_zip.rs +++ b/crates/uv-dev/src/validate_zip.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(()) } diff --git a/crates/uv-distribution/src/distribution_database.rs b/crates/uv-distribution/src/distribution_database.rs index b1d02c98a..dc370f85e 100644 --- a/crates/uv-distribution/src/distribution_database.rs +++ b/crates/uv-distribution/src/distribution_database.rs @@ -608,6 +608,7 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { let download = |response: reqwest::Response| { async { + let url = response.url().clone(); let size = size.or_else(|| content_length(&response)); let progress = self @@ -634,9 +635,13 @@ 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()) - .await - .map_err(|err| Error::Extract(filename.to_string(), err))?; + uv_extract::stream::unzip( + DisplaySafeUrl::from(url), + &mut reader, + temp_dir.path(), + ) + .await + .map_err(|err| Error::Extract(filename.to_string(), err))?; } WheelExtension::WhlZst => { uv_extract::stream::untar_zst(&mut reader, temp_dir.path()) @@ -647,9 +652,13 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { } None => match extension { WheelExtension::Whl => { - uv_extract::stream::unzip(&mut hasher, temp_dir.path()) - .await - .map_err(|err| Error::Extract(filename.to_string(), err))?; + uv_extract::stream::unzip( + DisplaySafeUrl::from(url), + &mut hasher, + temp_dir.path(), + ) + .await + .map_err(|err| Error::Extract(filename.to_string(), err))?; } WheelExtension::WhlZst => { uv_extract::stream::untar_zst(&mut hasher, temp_dir.path()) @@ -779,6 +788,7 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { let download = |response: reqwest::Response| { async { + let url = response.url().clone(); let size = size.or_else(|| content_length(&response)); let progress = self @@ -856,9 +866,13 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { match extension { WheelExtension::Whl => { - uv_extract::stream::unzip(&mut hasher, temp_dir.path()) - .await - .map_err(|err| Error::Extract(filename.to_string(), err))?; + uv_extract::stream::unzip( + DisplaySafeUrl::from(url), + &mut hasher, + temp_dir.path(), + ) + .await + .map_err(|err| Error::Extract(filename.to_string(), err))?; } WheelExtension::WhlZst => { uv_extract::stream::untar_zst(&mut hasher, temp_dir.path()) @@ -1046,7 +1060,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))?; } diff --git a/crates/uv-distribution/src/source/mod.rs b/crates/uv-distribution/src/source/mod.rs index a97f8dca4..74c88f968 100644 --- a/crates/uv-distribution/src/source/mod.rs +++ b/crates/uv-distribution/src/source/mod.rs @@ -2301,6 +2301,9 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { .bucket(CacheBucket::SourceDistributions), ) .map_err(Error::CacheWrite)?; + + let url = DisplaySafeUrl::from_url(response.url().clone()); + let reader = response .bytes_stream() .map_err(std::io::Error::other) @@ -2316,7 +2319,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(url, &mut hasher, ext, temp_dir.path()) .await .map_err(|err| Error::Extract(source.to_string(), err))?; drop(span); @@ -2385,7 +2388,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))?; diff --git a/crates/uv-extract/Cargo.toml b/crates/uv-extract/Cargo.toml index f77b6437d..21714859f 100644 --- a/crates/uv-extract/Cargo.toml +++ b/crates/uv-extract/Cargo.toml @@ -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"] } diff --git a/crates/uv-extract/src/stream.rs b/crates/uv-extract/src/stream.rs index fec33efac..20577a777 100644 --- a/crates/uv-extract/src/stream.rs +++ b/crates/uv-extract/src/stream.rs @@ -1,14 +1,17 @@ +use std::fmt::Display; use std::path::{Component, Path, PathBuf}; use std::pin::Pin; use async_zip::base::read::cd::Entry; use async_zip::error::ZipError; +use async_zip::{Compression, ZipEntry}; use futures::{AsyncReadExt, StreamExt}; use rustc_hash::{FxHashMap, FxHashSet}; 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}; @@ -43,10 +46,24 @@ 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( +/// +/// `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( + source_hint: D, reader: R, target: impl AsRef, ) -> Result<(), Error> { + /// Returns `true` if the entry uses a well-known compression method. + /// + /// This currently means just stored (no compression), DEFLATE, or zstd. + fn entry_has_well_known_compression(entry: &ZipEntry) -> bool { + matches!( + entry.compression(), + Compression::Stored | Compression::Deflate | Compression::Zstd + ) + } + /// Ensure the file path is safe to use as a [`Path`]. /// /// See: @@ -79,8 +96,19 @@ pub async fn unzip( 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. + if !entry_has_well_known_compression(zip_entry) { + warn_user_once!( + "One or more file entries in '{source_hint}' use the '{compression_method:?}' compression method, which is not widely supported. A future version of uv will reject ZIP archives containing entries compressed with this method.", + compression_method = zip_entry.compression() + ); + } + // 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 +135,14 @@ pub async fn unzip( 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 +151,23 @@ pub async fn unzip( } // 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 +192,7 @@ pub async fn unzip( { 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 +772,18 @@ pub async fn untar( /// Unpack a `.zip`, `.tar.gz`, `.tar.bz2`, `.tar.zst`, or `.tar.xz` archive into the target directory, /// without requiring `Seek`. -pub async fn archive( +/// +/// `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( + source_hint: D, reader: R, ext: SourceDistExtension, target: impl AsRef, ) -> Result<(), Error> { match ext { SourceDistExtension::Zip => { - unzip(reader, target).await?; + unzip(source_hint, reader, target).await?; } SourceDistExtension::Tar => { untar(reader, target).await?; diff --git a/crates/uv-extract/src/sync.rs b/crates/uv-extract/src/sync.rs index f60e416ae..0e574e668 100644 --- a/crates/uv-extract/src/sync.rs +++ b/crates/uv-extract/src/sync.rs @@ -1,19 +1,29 @@ use std::path::{Path, PathBuf}; use std::sync::{LazyLock, Mutex}; -use crate::vendor::{CloneableSeekableReader, HasLength}; +use crate::vendor::CloneableSeekableReader; use crate::{Error, insecure_no_validate, validate_archive_member_name}; use rayon::prelude::*; use rustc_hash::FxHashSet; use tracing::warn; use uv_configuration::RAYON_INITIALIZE; -use zip::ZipArchive; +use uv_warnings::warn_user_once; +use zip::{CompressionMethod, ZipArchive}; /// Unzip a `.zip` archive into the target directory. -pub fn unzip( - reader: R, - target: &Path, -) -> Result<(), Error> { +pub fn unzip(reader: fs_err::File, target: &Path) -> Result<(), Error> { + /// Returns `true` if the entry uses a well-known compression method. + /// + /// This currently means just stored (no compression), DEFLATE, or zstd. + fn entry_has_well_known_compression(method: CompressionMethod) -> bool { + matches!( + method, + CompressionMethod::Stored | CompressionMethod::Deflated | CompressionMethod::Zstd + ) + } + + let (reader, filename) = reader.into_parts(); + // Unzip in parallel. let reader = std::io::BufReader::new(reader); let archive = ZipArchive::new(CloneableSeekableReader::new(reader))?; @@ -27,6 +37,14 @@ pub fn unzip( let mut archive = archive.clone(); let mut file = archive.by_index(file_number)?; + if !entry_has_well_known_compression(file.compression()) { + warn_user_once!( + "One or more file entries in '{filename}' use the '{compression_method:?}' compression method, which is not widely supported. A future version of uv will reject ZIP archives containing entries compressed with this method.", + filename = filename.display(), + compression_method = file.compression() + ); + } + if let Err(e) = validate_archive_member_name(file.name()) { if !skip_validation { return Err(e); diff --git a/crates/uv-extract/src/vendor/cloneable_seekable_reader.rs b/crates/uv-extract/src/vendor/cloneable_seekable_reader.rs index 0de01e515..5bc0a849c 100644 --- a/crates/uv-extract/src/vendor/cloneable_seekable_reader.rs +++ b/crates/uv-extract/src/vendor/cloneable_seekable_reader.rs @@ -16,7 +16,7 @@ use std::{ /// A trait to represent some reader which has a total length known in /// advance. This is roughly equivalent to the nightly /// [`Seek::stream_len`] API. -pub trait HasLength { +pub(crate) trait HasLength { /// Return the current total length of this stream. fn len(&self) -> u64; } diff --git a/crates/uv-extract/src/vendor/mod.rs b/crates/uv-extract/src/vendor/mod.rs index 3148e2edd..d55ab7ed7 100644 --- a/crates/uv-extract/src/vendor/mod.rs +++ b/crates/uv-extract/src/vendor/mod.rs @@ -1,3 +1,3 @@ -pub(crate) use cloneable_seekable_reader::{CloneableSeekableReader, HasLength}; +pub(crate) use cloneable_seekable_reader::CloneableSeekableReader; mod cloneable_seekable_reader; diff --git a/crates/uv-python/src/downloads.rs b/crates/uv-python/src/downloads.rs index 4e2b867fd..4b4fdac69 100644 --- a/crates/uv-python/src/downloads.rs +++ b/crates/uv-python/src/downloads.rs @@ -1401,12 +1401,12 @@ impl ManagedPythonDownload { if let Some(reporter) = reporter { let progress_key = reporter.on_request_start(direction, &self.key, size); let mut reader = ProgressReader::new(&mut hasher, progress_key, reporter); - uv_extract::stream::archive(&mut reader, ext, target) + uv_extract::stream::archive(filename, &mut reader, ext, target) .await .map_err(|err| Error::ExtractError(filename.to_owned(), err))?; reporter.on_request_complete(direction, progress_key); } else { - uv_extract::stream::archive(&mut hasher, ext, target) + uv_extract::stream::archive(filename, &mut hasher, ext, target) .await .map_err(|err| Error::ExtractError(filename.to_owned(), err))?; } diff --git a/crates/uv-test/src/lib.rs b/crates/uv-test/src/lib.rs index 299044495..56d19aeb9 100755 --- a/crates/uv-test/src/lib.rs +++ b/crates/uv-test/src/lib.rs @@ -604,6 +604,21 @@ impl TestContext { self } + /// Add a filter for URLs from GitHub's release-assets CDN. + /// These URLs are non-deterministic and occur in logs when downloading distributions + /// from GitHub releases. + #[must_use] + pub fn with_filtered_github_release_asset_urls(mut self) -> Self { + // These look like 'https://release-assets.githubusercontent.com/github-production-release-asset/...', + // where ... is anything except whitespace or a quote. + let pattern = r#"https://release-assets\.githubusercontent\.com/[^\s\"']+"#; + self.filters.push(( + pattern.to_string(), + "[GITHUB_RELEASE_ASSET_URL]".to_string(), + )); + self + } + /// Use a shared global cache for Python downloads. #[must_use] pub fn with_python_download_cache(mut self) -> Self { diff --git a/crates/uv/src/commands/build_frontend.rs b/crates/uv/src/commands/build_frontend.rs index eabd58f89..07a0eab3f 100644 --- a/crates/uv/src/commands/build_frontend.rs +++ b/crates/uv/src/commands/build_frontend.rs @@ -720,7 +720,7 @@ async fn build_package( let ext = SourceDistExtension::from_path(path.as_path()) .map_err(|err| Error::InvalidSourceDistExt(path.user_display().to_string(), err))?; let temp_dir = tempfile::tempdir_in(cache.bucket(CacheBucket::SourceDistributions))?; - uv_extract::stream::archive(reader, ext, temp_dir.path()).await?; + uv_extract::stream::archive(path.display(), reader, ext, temp_dir.path()).await?; // Extract the top-level directory from the archive. let extracted = match uv_extract::strip_component(temp_dir.path()) { @@ -830,7 +830,8 @@ async fn build_package( Error::InvalidSourceDistExt(source.path().user_display().to_string(), err) })?; let temp_dir = tempfile::tempdir_in(&output_dir)?; - uv_extract::stream::archive(reader, ext, temp_dir.path()).await?; + uv_extract::stream::archive(source.path().display(), reader, ext, temp_dir.path()) + .await?; // If the source distribution has a version in its filename, check the version. let version = source diff --git a/crates/uv/tests/it/extract.rs b/crates/uv/tests/it/extract.rs index 2ebf06ec6..bfdd4b2ff 100644 --- a/crates/uv/tests/it/extract.rs +++ b/crates/uv/tests/it/extract.rs @@ -23,7 +23,7 @@ async fn unzip(url: &str) -> anyhow::Result<(), uv_extract::Error> { .into_async_read(); let target = tempfile::TempDir::new().map_err(uv_extract::Error::Io)?; - uv_extract::stream::unzip(reader.compat(), target.path()).await + uv_extract::stream::unzip(url, reader.compat(), target.path()).await } #[tokio::test] diff --git a/crates/uv/tests/it/pip_install.rs b/crates/uv/tests/it/pip_install.rs index 57be617ca..8691d7879 100644 --- a/crates/uv/tests/it/pip_install.rs +++ b/crates/uv/tests/it/pip_install.rs @@ -14135,3 +14135,50 @@ fn abi_compatibility_on_freethreaded_python() { + multi-abi-package==1.0.0 (from file://[WORKSPACE]/test/links/multi_abi_package-1.0.0-cp314-cp314t.abi3-manylinux_2_17_x86_64.whl) "); } + +#[test] +fn warn_on_bz2_wheel() { + let context = uv_test::test_context!("3.14").with_filtered_github_release_asset_urls(); + + uv_snapshot!( + context.filters(), + context.pip_install() + .arg("futzed_bz2 @ https://github.com/astral-sh/futzed-wheels/releases/download/v2026.02.09.2/futzed_bz2-0.1.0-py3-none-any.whl"), + @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + warning: One or more file entries in '[GITHUB_RELEASE_ASSET_URL]' use the 'Bz' compression method, which is not widely supported. A future version of uv will reject ZIP archives containing entries compressed with this method. + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + futzed-bz2==0.1.0 (from https://github.com/astral-sh/futzed-wheels/releases/download/v2026.02.09.2/futzed_bz2-0.1.0-py3-none-any.whl) + " + ); +} + +#[test] +fn warn_on_lzma_wheel() { + let context = uv_test::test_context!("3.14").with_filtered_github_release_asset_urls(); + + uv_snapshot!( + context.filters(), + context.pip_install() + .arg("futzed_lzma @ https://github.com/astral-sh/futzed-wheels/releases/download/v2026.02.09.2/futzed_lzma-0.1.0-py3-none-any.whl"), + @r" + success: false + exit_code: 1 + ----- stdout ----- + + ----- stderr ----- + × Failed to download `futzed-lzma @ https://github.com/astral-sh/futzed-wheels/releases/download/v2026.02.09.2/futzed_lzma-0.1.0-py3-none-any.whl` + ├─▶ Request failed after 3 retries + ├─▶ Failed to read metadata: `https://github.com/astral-sh/futzed-wheels/releases/download/v2026.02.09.2/futzed_lzma-0.1.0-py3-none-any.whl` + ├─▶ Failed to read from zip file + ├─▶ an upstream reader returned an error: stream/file format not recognized + ╰─▶ stream/file format not recognized + " + ); +}