migrate static asset handler to tower_http::ServeDir

Author: syphar

Cargo.lock

@@ -90,9 +90,8 @@ axum-extra = "0.4.2"
 hyper = { version = "0.14.15", default-features = false }
 tower = "0.4.11"
 tower-service = "0.3.2"
-tower-http = { version = "0.3.4", features = ["trace"] }
+tower-http = { version = "0.3.4", features = ["fs", "trace"] }
 mime = "0.3.16"
-httpdate = "1.0.2"
 percent-encoding = "2.2.0"
 
 # NOTE: if you change this, also double-check that the comment in `queue_builder::remove_tempdirs` is still accurate.

Cargo.toml

@@ -1,4 +1,6 @@
-use super::{cache::CachePolicy, error::AxumNope, metrics::request_recorder};
+use super::{
+    cache::CachePolicy, error::AxumNope, metrics::request_recorder, statics::build_static_router,
+};
 use axum::{
     handler::Handler as AxumHandler,
     http::Request as AxumHttpRequest,
@@ -15,7 +17,7 @@ use tracing::{debug, instrument};
 const INTERNAL_PREFIXES: &[&str] = &["-", "about", "crate", "releases", "sitemap.xml"];
 
 #[instrument(skip_all)]
-fn get_static<H, T, S, B>(handler: H) -> MethodRouter<S, B, Infallible>
+pub(crate) fn get_static<H, T, S, B>(handler: H) -> MethodRouter<S, B, Infallible>
 where
     H: AxumHandler<T, S, B>,
     B: Send + 'static + hyper::body::HttpBody,
@@ -102,10 +104,7 @@ pub(super) fn build_axum_routes() -> AxumRouter {
             "/favicon.ico",
             get_static(|| async { Redirect::permanent("/-/static/favicon.ico") }),
         )
-        .route(
-            "/-/static/*path",
-            get_static(super::statics::static_handler),
-        )
+        .nest("/-/static/", build_static_router())
         .route(
             "/opensearch.xml",
             get_static(|| async { Redirect::permanent("/-/static/opensearch.xml") }),

src/web/routes.rs

@@ -1,125 +1,100 @@
-use super::{
-    cache::CachePolicy,
-    error::{AxumNope, AxumResult},
-};
-use crate::utils::report_error;
-use anyhow::Context;
+use super::{cache::CachePolicy, error::AxumNope, metrics::request_recorder, routes::get_static};
 use axum::{
-    extract::{Extension, Path},
-    http::{
-        header::{CONTENT_LENGTH, CONTENT_TYPE, LAST_MODIFIED},
-        StatusCode,
-    },
+    extract::Extension,
+    headers::HeaderValue,
+    http::{header::CONTENT_TYPE, Request},
+    middleware,
+    middleware::Next,
     response::{IntoResponse, Response},
+    routing::get_service,
+    Router as AxumRouter,
 };
-use chrono::prelude::*;
-use httpdate::fmt_http_date;
-use mime::Mime;
-use mime_guess::MimeGuess;
-use std::{ffi::OsStr, path, time::SystemTime};
-use tokio::fs;
+use std::io;
+use tower_http::services::ServeDir;
 
 const VENDORED_CSS: &str = include_str!(concat!(env!("OUT_DIR"), "/vendored.css"));
 const STYLE_CSS: &str = include_str!(concat!(env!("OUT_DIR"), "/style.css"));
 const RUSTDOC_CSS: &str = include_str!(concat!(env!("OUT_DIR"), "/rustdoc.css"));
 const RUSTDOC_2021_12_05_CSS: &str =
     include_str!(concat!(env!("OUT_DIR"), "/rustdoc-2021-12-05.css"));
-const STATIC_SEARCH_PATHS: &[&str] = &["static", "vendor"];
-
-pub(crate) async fn static_handler(Path(path): Path<String>) -> AxumResult<impl IntoResponse> {
-    let text_css: Mime = "text/css".parse().unwrap();
-
-    Ok(match path.as_str() {
-        "vendored.css" => build_response(VENDORED_CSS, text_css),
-        "style.css" => build_response(STYLE_CSS, text_css),
-        "rustdoc.css" => build_response(RUSTDOC_CSS, text_css),
-        "rustdoc-2021-12-05.css" => build_response(RUSTDOC_2021_12_05_CSS, text_css),
-        file => match serve_file(file).await {
-            Ok(response) => response.into_response(),
-            Err(err) => return Err(err),
-        },
-    })
-}
-
-async fn serve_file(file: &str) -> AxumResult<impl IntoResponse> {
-    // Find the first path that actually exists
-    let path = STATIC_SEARCH_PATHS
-        .iter()
-        .find_map(|root| {
-            let path = path::Path::new(root).join(file);
-            if !path.exists() {
-                return None;
-            }
 
-            // Prevent accessing static files outside the root. This could happen if the path
-            // contains `/` or `..`. The check doesn't outright prevent those strings to be present
-            // to allow accessing files in subdirectories.
-            let canonical_path = std::fs::canonicalize(path).ok()?;
-            let canonical_root = std::fs::canonicalize(root).ok()?;
-            if canonical_path.starts_with(canonical_root) {
-                Some(canonical_path)
-            } else {
-                None
-            }
-        })
-        .ok_or(AxumNope::ResourceNotFound)?;
-
-    let contents = fs::read(&path)
-        .await
-        .with_context(|| format!("failed to read static file {}", path.display()))
-        .map_err(|e| {
-            report_error(&e);
-            AxumNope::InternalServerError
-        })?;
-
-    // If we can detect the file's mime type, set it
-    // MimeGuess misses a lot of the file types we need, so there's a small wrapper
-    // around it
-    let content_type: Mime = if file == "opensearch.xml" {
-        "application/opensearchdescription+xml".parse().unwrap()
-    } else {
-        path.extension()
-            .and_then(OsStr::to_str)
-            .and_then(|ext| match ext {
-                "eot" => Some("application/vnd.ms-fontobject".parse().unwrap()),
-                "woff2" => Some("application/font-woff2".parse().unwrap()),
-                "ttf" => Some("application/x-font-ttf".parse().unwrap()),
-                _ => MimeGuess::from_path(&path).first(),
-            })
-            .unwrap_or(mime::APPLICATION_OCTET_STREAM)
-    };
-
-    Ok(build_response(contents, content_type))
+async fn handle_error(err: io::Error) -> impl IntoResponse {
+    AxumNope::InternalError(err.into()).into_response()
 }
 
-fn build_response<R>(resource: R, content_type: Mime) -> Response
-where
-    R: AsRef<[u8]>,
-{
+fn build_static_response(content: &'static str) -> impl IntoResponse {
     (
-        StatusCode::OK,
         Extension(CachePolicy::ForeverInCdnAndBrowser),
-        [
-            (CONTENT_LENGTH, resource.as_ref().len().to_string()),
-            (CONTENT_TYPE, content_type.to_string()),
-            (LAST_MODIFIED, fmt_http_date(SystemTime::from(Utc::now()))),
-        ],
-        resource.as_ref().to_vec(),
+        [(CONTENT_TYPE, mime::TEXT_CSS.as_ref())],
+        content,
     )
-        .into_response()
+}
+
+async fn set_needed_static_headers<B>(req: Request<B>, next: Next<B>) -> Response {
+    let is_opensearch_xml = req.uri().path().ends_with("/opensearch.xml");
+
+    let mut response = next.run(req).await;
+
+    if response.status().is_success() {
+        response
+            .extensions_mut()
+            .insert(CachePolicy::ForeverInCdnAndBrowser);
+    }
+
+    if is_opensearch_xml {
+        // overwrite the content type for opensearch.xml,
+        // otherwise mime-guess would return `text/xml`.
+        response.headers_mut().insert(
+            CONTENT_TYPE,
+            HeaderValue::from_static("application/opensearchdescription+xml"),
+        );
+    }
+
+    response
+}
+
+pub(crate) fn build_static_router() -> AxumRouter {
+    AxumRouter::new()
+        .route(
+            "/vendored.css",
+            get_static(|| async { build_static_response(VENDORED_CSS) }),
+        )
+        .route(
+            "/style.css",
+            get_static(|| async { build_static_response(STYLE_CSS) }),
+        )
+        .route(
+            "/rustdoc.css",
+            get_static(|| async { build_static_response(RUSTDOC_CSS) }),
+        )
+        .route(
+            "/rustdoc-2021-12-05.css",
+            get_static(|| async { build_static_response(RUSTDOC_2021_12_05_CSS) }),
+        )
+        .nest_service(
+            "/",
+            get_service(ServeDir::new("static").fallback(ServeDir::new("vendor")))
+                .handle_error(handle_error)
+                .layer(middleware::from_fn(set_needed_static_headers))
+                .layer(middleware::from_fn(|request, next| async {
+                    request_recorder(request, next, Some("static resource")).await
+                })),
+        )
 }
 
 #[cfg(test)]
 mod tests {
-    use super::{serve_file, STATIC_SEARCH_PATHS, STYLE_CSS, VENDORED_CSS};
+    use super::{STYLE_CSS, VENDORED_CSS};
     use crate::{
         test::{assert_cache_control, wrapper},
-        web::{cache::CachePolicy, error::AxumNope},
+        web::cache::CachePolicy,
     };
     use reqwest::StatusCode;
     use std::fs;
     use test_case::test_case;
 
+    const STATIC_SEARCH_PATHS: &[&str] = &["static", "vendor"];
+
     #[test]
     fn style_css() {
         wrapper(|env| {
@@ -243,28 +218,4 @@ mod tests {
             Ok(())
         });
     }
-
-    #[tokio::test]
-    async fn directory_traversal() {
-        const PATHS: &[&str] = &[
-            "../LICENSE",
-            "%2e%2e%2fLICENSE",
-            "%2e%2e/LICENSE",
-            "..%2fLICENSE",
-            "%2e%2e%5cLICENSE",
-        ];
-
-        for path in PATHS {
-            // This doesn't test an actual web request as the web framework used at the time of
-            // writing this test (iron 0.5) already resolves `..` before calling any handler.
-            //
-            // Still, the test ensures the underlying function called by the request handler to
-            // serve the file also includes protection for path traversal, in the event we switch
-            // to a framework that doesn't include builtin protection in the future.
-            assert!(
-                matches!(serve_file(path).await, Err(AxumNope::ResourceNotFound)),
-                "{path} did not return a 404"
-            );
-        }
-    }
 }