migrate homepage, release-lists & search to axum

Author: syphar

src/web/error.rs

@@ -4,6 +4,7 @@ use crate::{
     db::PoolError,
     web::{page::WebPage, releases::Search, AxumErrorPage, ErrorPage},
 };
+use anyhow::Context as _;
 use axum::{
     http::StatusCode,
     response::{IntoResponse, Response as AxumResponse},
@@ -22,8 +23,6 @@ pub enum Nope {
     OwnerNotFound,
     #[error("Requested crate does not have specified version")]
     VersionNotFound,
-    #[error("Search yielded no results")]
-    NoResults,
     #[error("Internal server error")]
     InternalServerError,
 }
@@ -37,8 +36,7 @@ impl From<Nope> for IronError {
             | Nope::BuildNotFound
             | Nope::CrateNotFound
             | Nope::OwnerNotFound
-            | Nope::VersionNotFound
-            | Nope::NoResults => status::NotFound,
+            | Nope::VersionNotFound => status::NotFound,
             Nope::InternalServerError => status::InternalServerError,
         };
 
@@ -96,29 +94,6 @@ impl Handler for Nope {
                 .into_response(req)
             }
 
-            Nope::NoResults => {
-                let mut params = req.url.as_ref().query_pairs();
-
-                if let Some((_, query)) = params.find(|(key, _)| key == "query") {
-                    // this used to be a search
-                    Search {
-                        title: format!("No crates found matching '{}'", query),
-                        search_query: Some(query.into_owned()),
-                        status: Status::NotFound,
-                        ..Default::default()
-                    }
-                    .into_response(req)
-                } else {
-                    // user did a search with no search terms
-                    Search {
-                        title: "No results given for empty search query".to_owned(),
-                        status: Status::NotFound,
-                        ..Default::default()
-                    }
-                    .into_response(req)
-                }
-            }
-
             Nope::InternalServerError => {
                 // something went wrong, details should have been logged
                 ErrorPage {
@@ -151,8 +126,8 @@ pub enum AxumNope {
     OwnerNotFound,
     #[error("Requested crate does not have specified version")]
     VersionNotFound,
-    // #[error("Search yielded no results")]
-    // NoResults,
+    #[error("Search yielded no results")]
+    NoResults,
     #[error("Internal server error")]
     InternalServerError,
     #[error("internal error")]
@@ -207,9 +182,15 @@ impl IntoResponse for AxumNope {
                 }
                 .into_response()
             }
-            // AxumNope::NoResults => {
-            //     todo!("to be implemented when search-handler is migrated to axum")
-            // }
+            AxumNope::NoResults => {
+                // user did a search with no search terms
+                Search {
+                    title: "No results given for empty search query".to_owned(),
+                    status: StatusCode::NOT_FOUND,
+                    ..Default::default()
+                }
+                .into_response()
+            }
             AxumNope::InternalServerError => {
                 // something went wrong, details should have been logged
                 AxumErrorPage {
@@ -254,14 +235,51 @@ impl From<Nope> for AxumNope {
             Nope::CrateNotFound => AxumNope::CrateNotFound,
             Nope::OwnerNotFound => AxumNope::OwnerNotFound,
             Nope::VersionNotFound => AxumNope::VersionNotFound,
-            Nope::NoResults => todo!(),
             Nope::InternalServerError => AxumNope::InternalServerError,
         }
     }
 }
 
 pub(crate) type AxumResult<T> = Result<T, AxumNope>;
 
+/// spawn-blocking helper for usage in axum requests.
+///
+/// Should be used when spawning tasks from inside web-handlers
+/// that return AxumNope as error. The join-error will also be
+/// converted into an `anyhow::Error`. Any standard AxumNope
+/// will be left intact so the error-handling can generate the
+/// correct status-page with the correct status code.
+///
+/// Examples:
+///
+/// with standard `tokio::task::spawn_blocking`:
+/// ```ignore
+/// let data = spawn_blocking(move || -> Result<_, AxumNope> {
+///     let data = get_the_data()?;
+///     Ok(data)
+/// })
+/// .await
+/// .context("failed to join thread")??;
+/// ```
+///
+/// with this helper function:
+/// ```ignore
+/// let data = spawn_blocking(move || {
+///     let data = get_the_data()?;
+///     Ok(data)
+/// })
+/// .await?
+/// ```
+pub(crate) async fn spawn_blocking_web<F, R>(f: F) -> AxumResult<R>
+where
+    F: FnOnce() -> AxumResult<R> + Send + 'static,
+    R: Send + 'static,
+{
+    tokio::task::spawn_blocking(f)
+        .await
+        .context("failed to join thread")?
+}
+
 #[cfg(test)]
 mod tests {
     use crate::test::wrapper;

src/web/mod.rs

@@ -4,7 +4,7 @@ pub mod page;
 
 use crate::utils::get_correct_docsrs_style_file;
 use crate::utils::report_error;
-use anyhow::anyhow;
+use anyhow::{anyhow, bail, Context as _};
 use serde_json::Value;
 use tracing::{info, instrument};
 
@@ -92,7 +92,7 @@ mod source;
 mod statics;
 mod strangler;
 
-use crate::{impl_axum_webpage, impl_webpage, Context};
+use crate::{db::Pool, impl_axum_webpage, impl_webpage, Context};
 use anyhow::Error;
 use axum::{
     extract::Extension,
@@ -123,6 +123,7 @@ use std::{borrow::Cow, net::SocketAddr, sync::Arc};
 use strangler::StranglerService;
 use tower::ServiceBuilder;
 use tower_http::trace::TraceLayer;
+use url::form_urlencoded;
 
 /// Duration of static files for staticfile and DatabaseFileHandler (in seconds)
 const STATIC_FILE_CACHE_DURATION: u64 = 60 * 60 * 24 * 30 * 12; // 12 months
@@ -428,6 +429,27 @@ fn match_version(
     Err(Nope::VersionNotFound)
 }
 
+// temporary wrapper around `match_version` for axum handlers.
+//
+// FIXME: this can go when we fully migrated to axum / async in web
+async fn match_version_axum(
+    pool: &Pool,
+    name: &str,
+    input_version: Option<&str>,
+) -> Result<MatchVersion, error::AxumNope> {
+    error::spawn_blocking_web({
+        let name = name.to_owned();
+        let input_version = input_version.map(str::to_owned);
+        let pool = pool.clone();
+        move || {
+            let mut conn = pool.get().context("could not get connection from pool")?;
+            match_version(&mut conn, &name, input_version.as_deref())
+                .map_err(Into::<error::AxumNope>::into)
+        }
+    })
+    .await
+}
+
 #[instrument(skip_all)]
 pub(crate) fn build_axum_app(
     context: &dyn Context,
@@ -539,15 +561,29 @@ fn redirect(url: Url) -> Response {
     resp
 }
 
-fn axum_redirect(url: &str) -> Result<impl IntoResponse, Error> {
-    if !url.starts_with('/') || url.starts_with("//") {
-        return Err(anyhow!("invalid redirect URL: {}", url));
+fn axum_redirect<U>(uri: U) -> Result<impl IntoResponse, Error>
+where
+    U: TryInto<http::Uri>,
+    <U as TryInto<http::Uri>>::Error: std::fmt::Debug,
+{
+    let uri: http::Uri = uri
+        .try_into()
+        .map_err(|err| anyhow!("invalid URI: {:?}", err))?;
+
+    if let Some(path_and_query) = uri.path_and_query() {
+        if path_and_query.as_str().starts_with("//") {
+            bail!("protocol relative redirects are forbidden");
+        }
+    } else {
+        // we always want a path to redirect to, even when it's just `/`
+        bail!("missing path in URI");
     }
+
     Ok((
         StatusCode::FOUND,
         [(
             http::header::LOCATION,
-            http::HeaderValue::try_from(url).expect("invalid url for redirect"),
+            http::HeaderValue::try_from(uri.to_string()).context("invalid uri for redirect")?,
         )],
     ))
 }
@@ -605,6 +641,29 @@ where
     }
 }
 
+/// Parse an URI into a http::Uri struct.
+/// When `queries` are given these are added to the URL,
+/// with empty `queries` the `?` will be omitted.
+pub(crate) fn axum_parse_uri_with_params<I, K, V>(uri: &str, queries: I) -> Result<http::Uri, Error>
+where
+    I: IntoIterator,
+    I::Item: Borrow<(K, V)>,
+    K: AsRef<str>,
+    V: AsRef<str>,
+{
+    let mut queries = queries.into_iter().peekable();
+    if queries.peek().is_some() {
+        let query_params: String = form_urlencoded::Serializer::new(String::new())
+            .extend_pairs(queries)
+            .finish();
+        format!("{uri}?{}", query_params)
+            .parse::<http::Uri>()
+            .context("error parsing URL")
+    } else {
+        uri.parse::<http::Uri>().context("error parsing URL")
+    }
+}
+
 /// MetaData used in header
 #[derive(Debug, Clone, PartialEq, Eq, Serialize)]
 pub(crate) struct MetaData {