Warn on unexpected ZIP compression methods#17885
Conversation
|
Somewhat (?) surprisingly, nothing in the current tests triggers these warnings. So I'll contrive a ZIP that does. |
90150a6 to
ff11640
Compare
| /// `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, |
There was a problem hiding this comment.
This required updating a lot of callsites, as can be seen in the diff. But this is the core change here!
Signed-off-by: William Woodruff <william@astral.sh>
Signed-off-by: William Woodruff <william@astral.sh>
Signed-off-by: William Woodruff <william@astral.sh>
9cbf5bf to
2b9b4e1
Compare
Signed-off-by: William Woodruff <william@astral.sh>
Signed-off-by: William Woodruff <william@astral.sh>
| } | ||
|
|
||
| #[test] | ||
| fn warn_on_lzma_wheel() { |
There was a problem hiding this comment.
Flagging: this was kind of surprising -- we already didn't support lzma wheels, at least in the streaming pathway. I think this means I can probably just drop the lzma extra from async-zip.
Merging this PR will improve performance by 11.23%
Performance Changes
Comparing |
3 similar comments
Merging this PR will improve performance by 11.23%
Performance Changes
Comparing |
Merging this PR will improve performance by 11.23%
Performance Changes
Comparing |
Merging this PR will improve performance by 11.23%
Performance Changes
Comparing |
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.)