-
-
Notifications
You must be signed in to change notification settings - Fork 314
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
Feature: Add config setting to disable gzip compression #1627 #1628
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -158,7 +158,13 @@ | |
#[cfg(feature = "gzip")] | ||
let stack = ServiceBuilder::new() | ||
.layer(stack) | ||
.layer(tower_http::decompression::DecompressionLayer::new()) | ||
.layer( | ||
tower_http::decompression::DecompressionLayer::new() | ||
.no_br() | ||
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. intentionally disable anything but GZIP compression - reading kubernetes/kubernetes#112296 show there isn't like to be any other option in Kube for a while, but these flags could easily be refitted when/if Kube gains alternatives. 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. agreed. think this is sensible also. |
||
.no_deflate() | ||
.no_zstd() | ||
.gzip(!config.disable_compression), | ||
) | ||
.into_inner(); | ||
|
||
let service = ServiceBuilder::new() | ||
|
@@ -226,3 +232,70 @@ | |
default_ns, | ||
)) | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
#[cfg(feature = "gzip")] use super::*; | ||
|
||
#[cfg(feature = "gzip")] | ||
#[tokio::test] | ||
async fn test_no_accept_encoding_header_sent_when_compression_disabled( | ||
) -> Result<(), Box<dyn std::error::Error>> { | ||
use http::Uri; | ||
use std::net::SocketAddr; | ||
use tokio::net::{TcpListener, TcpStream}; | ||
|
||
// setup a server that echoes back any encoding header value | ||
let addr: SocketAddr = ([127, 0, 0, 1], 0).into(); | ||
let listener = TcpListener::bind(addr).await?; | ||
let local_addr = listener.local_addr()?; | ||
let uri: Uri = format!("http://{}", local_addr).parse()?; | ||
|
||
tokio::spawn(async move { | ||
use http_body_util::Full; | ||
use hyper::{server::conn::http1, service::service_fn}; | ||
use hyper_util::rt::{TokioIo, TokioTimer}; | ||
use std::convert::Infallible; | ||
|
||
loop { | ||
let (tcp, _) = listener.accept().await.unwrap(); | ||
let io: TokioIo<TcpStream> = TokioIo::new(tcp); | ||
|
||
tokio::spawn(async move { | ||
let _ = http1::Builder::new() | ||
.timer(TokioTimer::new()) | ||
.serve_connection( | ||
io, | ||
service_fn(|req| async move { | ||
let response = req | ||
.headers() | ||
.get(http::header::ACCEPT_ENCODING) | ||
.map(|b| Bytes::copy_from_slice(b.as_bytes())) | ||
.unwrap_or_default(); | ||
Ok::<_, Infallible>(Response::new(Full::new(response))) | ||
}), | ||
) | ||
.await | ||
.unwrap(); | ||
}); | ||
} | ||
}); | ||
|
||
// confirm gzip echoed back with default config | ||
let config = Config { ..Config::new(uri) }; | ||
let client = make_generic_builder(HttpConnector::new(), config.clone())?.build(); | ||
let response = client.request_text(http::Request::default()).await?; | ||
assert_eq!(&response, "gzip"); | ||
|
||
// now disable and check empty string echoed back | ||
let config = Config { | ||
disable_compression: true, | ||
..config | ||
}; | ||
let client = make_generic_builder(HttpConnector::new(), config)?.build(); | ||
let response = client.request_text(http::Request::default()).await?; | ||
assert_eq!(&response, ""); | ||
Comment on lines
+284
to
+297
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. wow great test. 👍 |
||
|
||
Ok(()) | ||
} | ||
} |
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.
trying to wrap the whole layer in an "option_layer" block ran head first into a load of compile issues - fortunately the Compression Layer supports enabling/disabling through configuration hence that path was chosen