-
Notifications
You must be signed in to change notification settings - Fork 923
Support compressed package downloads #6069
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,12 @@ | ||
| use std::{ | ||
| collections::HashMap, | ||
| fmt::Write as _, | ||
| io::{ErrorKind, Write as _}, | ||
| io::{ErrorKind, Read, Write as _}, | ||
| path::PathBuf, | ||
| sync::{Arc, RwLock}, | ||
| }; | ||
|
|
||
| use anyhow::{Context, Error}; | ||
| use anyhow::{Context, Error, bail}; | ||
| use bytes::Bytes; | ||
| use http::{HeaderMap, Method}; | ||
| use tempfile::NamedTempFile; | ||
|
|
@@ -284,6 +284,8 @@ impl BuiltinPackageLoader { | |
| } | ||
|
|
||
| let body = response.body.context("package download failed")?; | ||
| let body = Self::decode_response_body(&response.headers, body) | ||
| .context("package download failed: could not decode response body")?; | ||
| tracing::debug!(%url, "package_download_succeeded"); | ||
|
|
||
| let body = bytes::Bytes::from(body); | ||
|
|
@@ -298,6 +300,12 @@ impl BuiltinPackageLoader { | |
| headers.insert("Accept", "application/webc".parse().unwrap()); | ||
| headers.insert("User-Agent", USER_AGENT.parse().unwrap()); | ||
|
|
||
| // Accept compressed responses. | ||
| // NOTE: gzip and zstd decoding is available on native platforms. | ||
| // In browser platforms, the fetch implementation should automatically | ||
| // handle decoding of gzip/zstd responses transparently. | ||
theduke marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| headers.insert(http::header::ACCEPT_ENCODING, "gzip, zstd".parse().unwrap()); | ||
theduke marked this conversation as resolved.
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we use the qvalue weights to prefer ZSTD if possible? |
||
|
|
||
| if url.has_authority() | ||
| && let Some(token) = self.tokens.get(url.authority()) | ||
| { | ||
|
|
@@ -317,6 +325,53 @@ impl BuiltinPackageLoader { | |
|
|
||
| headers | ||
| } | ||
|
|
||
| fn decode_response_body(headers: &HeaderMap, body: Vec<u8>) -> Result<Vec<u8>, anyhow::Error> { | ||
| let encodings = match headers.get(http::header::CONTENT_ENCODING) { | ||
| Some(header) => header | ||
| .to_str() | ||
| .context("invalid content-encoding header")? | ||
| .split(',') | ||
| .map(|encoding| encoding.trim().to_ascii_lowercase()) | ||
marxin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| .filter(|encoding| !encoding.is_empty()) | ||
| .collect::<Vec<_>>(), | ||
| None => Vec::new(), | ||
| }; | ||
|
|
||
| if encodings.is_empty() || (encodings.len() == 1 && encodings[0] == "identity") { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. at_most_one might work well. |
||
| return Ok(body); | ||
| } | ||
|
|
||
| let mut reader: Box<dyn Read> = Box::new(std::io::Cursor::new(body)); | ||
| for encoding in encodings.iter().rev() { | ||
| match encoding.as_str() { | ||
| "gzip" => { | ||
| reader = Box::new(flate2::read::GzDecoder::new(reader)); | ||
| } | ||
| "zstd" => { | ||
| #[cfg(not(target_arch = "wasm32"))] | ||
| { | ||
| reader = Box::new( | ||
| zstd::stream::read::Decoder::new(reader) | ||
| .context("failed to initialize zstd decoder")?, | ||
| ); | ||
| } | ||
| #[cfg(target_arch = "wasm32")] | ||
| { | ||
| bail!("zstd content-encoding is not supported on wasm32"); | ||
marxin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
| "identity" => {} | ||
| other => bail!("unsupported content-encoding: {other}"), | ||
| } | ||
| } | ||
|
|
||
| let mut decoded = Vec::new(); | ||
| reader | ||
| .read_to_end(&mut decoded) | ||
| .context("failed to decode response body")?; | ||
| Ok(decoded) | ||
| } | ||
|
Comment on lines
+329
to
+374
|
||
| } | ||
|
|
||
| impl Default for BuiltinPackageLoader { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spelling errors in comment: "respone" should be "response", and there's an extra space between "decoding" and "the".