fix clippy, only send gzip if accept-encoding header says to

nexus/src/external_api/console_api.rs

@@ -624,14 +624,16 @@ 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
 }
 
 /// Fetch a static asset from `<static_dir>/assets`. 404 on virtually all
-/// errors. No auth. NO SENSITIVE FILES.
+/// errors. No auth. NO SENSITIVE FILES. Will serve a gzipped version if the
+/// `.gz` file is present in the directory and `Accept-Encoding: gzip` is
+/// present on the request.
 #[endpoint {
    method = GET,
    path = "/assets/{path:.*}",
@@ -644,38 +646,50 @@ pub async fn asset(
     let apictx = rqctx.context();
     let path = path_params.into_inner().path;
 
-    // bail unless the extension is allowed
-    let filename = path.last().ok_or(not_found("no path"))?;
-    let ext = PathBuf::from(filename)
+    // Bail unless the extension is allowed
+    let filename =
+        PathBuf::from(path.last().ok_or_else(|| not_found("no path"))?);
+    let ext = filename
         .extension()
         .map(|ext| ext.to_os_string())
         .unwrap_or_else(|| OsString::from("disallowed"));
     if !ALLOWED_EXTENSIONS.contains(&ext) {
         return Err(not_found("file extension not allowed"));
     }
 
-    // important: we only serve assets from assets/ within static_dir
+    // We only serve assets from assets/ within static_dir
     let assets_dir = &apictx
         .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),
-        _ => (find_file(path, &assets_dir)?, false),
-    };
+    // If req accepts gzip and we have a gzipped version, serve that. Otherwise
+    // fall back to non-gz. If neither file found, bubble up 404.
+    let (file_path, set_content_encoding_gzip) =
+        match (accept_gz, find_file(path_gz, &assets_dir)) {
+            (true, Ok(gzipped_file)) => (gzipped_file, true),
+            _ => (find_file(path, &assets_dir)?, false),
+        };
 
-    let file_contents = tokio::fs::read(&file)
-        .await
-        .map_err(|e| not_found(&format!("accessing {:?}: {:#}", file, e)))?;
+    // File read is the same regardless of gzip
+    let file_contents = tokio::fs::read(&file_path).await.map_err(|e| {
+        not_found(&format!("accessing {:?}: {:#}", file_path, e))
+    })?;
 
-    // Derive the MIME type from the file name
-    let content_type = mime_guess::from_path(&file)
+    // Derive the MIME type from the file name. Have to use filename and not
+    // file_path because the latter might have a .gz on the end.
+    let content_type = mime_guess::from_path(filename)
         .first()
         .map_or_else(|| "text/plain".to_string(), |m| m.to_string());
 
@@ -687,7 +701,7 @@ pub async fn asset(
             cache_control_header_value(apictx),
         );
 
-    if gzipped {
+    if set_content_encoding_gzip {
         resp = resp.header(http::header::CONTENT_ENCODING, "gzip");
     }
 

nexus/tests/integration_tests/console_api.rs

@@ -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]

nexus/tests/static/assets/gzip-and-not.txt

@@ -0,0 +1 @@
+not gzipped but I know a guy

nexus/tests/static/assets/gzip-only.txt.gz

@@ -0,0 +1 @@
+nothing but gzip