Skip to content

Commit

Permalink
Limit zstd window size to web-safe value (#490)
Browse files Browse the repository at this point in the history
* Limit zstd window size to web-safe value

zstd's memory buffer is limited to 8MB by Chromium and Firefox (at least), but the encoder can exceed that at level 20 and above. Ensure it's limited to 8MB.

* Add test for zstd window size

Also bump the version of the zstd dev dependency.
  • Loading branch information
AlyoshaVasilieva authored May 27, 2024
1 parent 52f1020 commit a1e3f8a
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 2 deletions.
2 changes: 1 addition & 1 deletion tower-http/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ tokio = { version = "1", features = ["full"] }
tower = { version = "0.4.10", features = ["buffer", "util", "retry", "make", "timeout"] }
tracing-subscriber = "0.3"
uuid = { version = "1.0", features = ["v4"] }
zstd = "0.12"
zstd = "0.13"

[features]
default = []
Expand Down
24 changes: 23 additions & 1 deletion tower-http/src/compression/body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,29 @@ where
type Output = ZstdEncoder<Self::Input>;

fn apply(input: Self::Input, quality: CompressionLevel) -> Self::Output {
ZstdEncoder::with_quality(input, quality.into_async_compression())
// See https://issues.chromium.org/issues/41493659:
// "For memory usage reasons, Chromium limits the window size to 8MB"
// See https://datatracker.ietf.org/doc/html/rfc8878#name-window-descriptor
// "For improved interoperability, it's recommended for decoders to support values
// of Window_Size up to 8 MB and for encoders not to generate frames requiring a
// Window_Size larger than 8 MB."
// Level 17 in zstd (as of v1.5.6) is the first level with a window size of 8 MB (2^23):
// https://github.com/facebook/zstd/blob/v1.5.6/lib/compress/clevels.h#L25-L51
// Set the parameter for all levels >= 17. This will either have no effect (but reduce
// the risk of future changes in zstd) or limit the window log to 8MB.
let needs_window_limit = match quality {
CompressionLevel::Best => true, // level 20
CompressionLevel::Precise(level) => level >= 17,
_ => false,
};
// The parameter is not set for levels below 17 as it will increase the window size
// for those levels.
if needs_window_limit {
let params = [async_compression::zstd::CParameter::window_log(23)];
ZstdEncoder::with_quality_and_params(input, quality.into_async_compression(), &params)
} else {
ZstdEncoder::with_quality(input, quality.into_async_compression())
}
}

fn get_pin_mut(pinned: Pin<&mut Self::Output>) -> Pin<&mut Self::Input> {
Expand Down
37 changes: 37 additions & 0 deletions tower-http/src/compression/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,4 +200,41 @@ mod tests {

Ok(())
}

/// Test ensuring that zstd compression will not exceed an 8MiB window size; browsers do not
/// accept responses using 16MiB+ window sizes.
#[tokio::test]
async fn zstd_is_web_safe() -> Result<(), crate::BoxError> {
async fn zeroes(_req: Request<Body>) -> Result<Response<Body>, Infallible> {
Ok(Response::new(Body::from(vec![0u8; 18_874_368])))
}
// zstd will (I believe) lower its window size if a larger one isn't beneficial and
// it knows the size of the input; use an 18MiB body to ensure it would want a
// >=16MiB window (though it might not be able to see the input size here).

let zstd_layer = CompressionLayer::new()
.quality(CompressionLevel::Best)
.no_br()
.no_deflate()
.no_gzip();

let mut service = ServiceBuilder::new().layer(zstd_layer).service_fn(zeroes);

let request = Request::builder()
.header(ACCEPT_ENCODING, "zstd")
.body(Body::empty())?;

let response = service.ready().await?.call(request).await?;

assert_eq!(response.headers()["content-encoding"], "zstd");

let body = response.into_body();
let bytes = body.collect().await?.to_bytes();
let mut dec = zstd::Decoder::new(&*bytes)?;
dec.window_log_max(23)?; // Limit window size accepted by decoder to 2 ^ 23 bytes (8MiB)

std::io::copy(&mut dec, &mut std::io::sink())?;

Ok(())
}
}

0 comments on commit a1e3f8a

Please sign in to comment.