Skip to content

Commit

Permalink
fix clippy, only send gzip if accept-encoding header says to
Browse files Browse the repository at this point in the history
  • Loading branch information
david-crespo committed Jul 2, 2022
1 parent 053b425 commit 9f48b8f
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 8 deletions.
19 changes: 13 additions & 6 deletions nexus/src/external_api/console_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ fn with_gz_ext(path: Vec<String>) -> Vec<String> {
return path;
}

let mut new_path = path.clone();
let mut new_path = path;
let last = new_path.pop().unwrap(); // we know there's one there
new_path.push(format!("{last}.gz"));
new_path
Expand All @@ -645,7 +645,7 @@ pub async fn asset(
let path = path_params.into_inner().path;

// bail unless the extension is allowed
let filename = path.last().ok_or(not_found("no path"))?;
let filename = path.last().ok_or_else(|| not_found("no path"))?;
let ext = PathBuf::from(filename)
.extension()
.map(|ext| ext.to_os_string())
Expand All @@ -659,14 +659,21 @@ pub async fn asset(
.console_config
.static_dir
.as_ref()
.ok_or(not_found("static_dir undefined"))?
.ok_or_else(|| not_found("static_dir undefined"))?
.join("assets");

let request = &rqctx.request.lock().await;
let accept_encoding = request.headers().get(http::header::ACCEPT_ENCODING);

let accept_gz = accept_encoding.map_or(false, |val| {
val.to_str().map_or(false, |s| s.contains("gzip"))
});

let path_gz = with_gz_ext(path.clone());

// try to find the gzipped version, fall back to non-gz
let (file, gzipped) = match find_file(path_gz, &assets_dir) {
Ok(file) => (file, true),
// try to find the gzipped version (if accepted), fall back to non-gz
let (file, gzipped) = match (accept_gz, find_file(path_gz, &assets_dir)) {
(true, Ok(gzipped_file)) => (gzipped_file, true),
_ => (find_file(path, &assets_dir)?, false),
};

Expand Down
38 changes: 36 additions & 2 deletions nexus/tests/integration_tests/console_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,17 +195,51 @@ async fn test_assets(cptestctx: &ControlPlaneTestContext) {
.expect("failed to get existing file");

assert_eq!(resp.body, "hello there".as_bytes());
// make sure we're not including the gzip header on non-gzipped files
assert_eq!(resp.headers.get(http::header::CONTENT_ENCODING), None);

// file with existing gzipped version is returned
// file with only gzipped version 404s if request doesn't have accept-encoding: gzip
let _ = RequestBuilder::new(&testctx, Method::GET, "/assets/gzip-only.txt")
.expect_status(Some(StatusCode::NOT_FOUND))
.execute()
.await
.expect("failed to 404 on gzip file without accept-encoding: gzip");

// file with only gzipped version is returned if request accepts gzip
let resp =
RequestBuilder::new(&testctx, Method::GET, "/assets/gzipped.txt")
RequestBuilder::new(&testctx, Method::GET, "/assets/gzip-only.txt")
.header(http::header::ACCEPT_ENCODING, "gzip")
.expect_status(Some(StatusCode::OK))
.expect_response_header(http::header::CONTENT_ENCODING, "gzip")
.execute()
.await
.expect("failed to get existing file");

assert_eq!(resp.body, "nothing but gzip".as_bytes());

// file with both gzip and not returns gzipped if request accepts gzip
let resp =
RequestBuilder::new(&testctx, Method::GET, "/assets/gzip-and-not.txt")
.header(http::header::ACCEPT_ENCODING, "gzip")
.expect_status(Some(StatusCode::OK))
.expect_response_header(http::header::CONTENT_ENCODING, "gzip")
.execute()
.await
.expect("failed to get existing file");

assert_eq!(resp.body, "pretend this is gzipped beep boop".as_bytes());

// returns non-gzipped if request doesn't accept gzip
let resp =
RequestBuilder::new(&testctx, Method::GET, "/assets/gzip-and-not.txt")
.expect_status(Some(StatusCode::OK))
.execute()
.await
.expect("failed to get existing file");

assert_eq!(resp.body, "not gzipped but I know a guy".as_bytes());
// make sure we're not including the gzip header on non-gzipped files
assert_eq!(resp.headers.get(http::header::CONTENT_ENCODING), None);
}

#[tokio::test]
Expand Down
1 change: 1 addition & 0 deletions nexus/tests/static/assets/gzip-and-not.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
not gzipped but I know a guy
File renamed without changes.
1 change: 1 addition & 0 deletions nexus/tests/static/assets/gzip-only.txt.gz
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
nothing but gzip

0 comments on commit 9f48b8f

Please sign in to comment.