From 8060181b9f53854a86516fcf28dcd6cdb8f7c864 Mon Sep 17 00:00:00 2001 From: cloudhead Date: Wed, 9 Aug 2023 16:59:19 +0200 Subject: [PATCH] httpd: Fix more routes that return the wrong code These routes should return 404 if the entity was not found. --- radicle-httpd/src/api/error.rs | 8 +++++ radicle-httpd/src/api/v1/projects.rs | 50 ++++++++++++++++++++++++++++ radicle-httpd/src/lib.rs | 25 ++++++++++++++ radicle-httpd/src/main.rs | 27 ++------------- radicle-httpd/src/test.rs | 2 ++ 5 files changed, 87 insertions(+), 25 deletions(-) diff --git a/radicle-httpd/src/api/error.rs b/radicle-httpd/src/api/error.rs index d956090d..864daa40 100644 --- a/radicle-httpd/src/api/error.rs +++ b/radicle-httpd/src/api/error.rs @@ -93,11 +93,19 @@ impl IntoResponse for Error { } Error::Auth(msg) => (StatusCode::BAD_REQUEST, Some(msg.to_string())), Error::Crypto(msg) => (StatusCode::BAD_REQUEST, Some(msg.to_string())), + Error::Surf(radicle_surf::Error::Git(e)) if radicle::git::is_not_found_err(&e) => { + (StatusCode::NOT_FOUND, None) + } + Error::Surf(radicle_surf::Error::Directory( + radicle_surf::fs::error::Directory::PathNotFound(_), + )) => (StatusCode::NOT_FOUND, None), + Error::Git2(e) if radicle::git::is_not_found_err(&e) => (StatusCode::NOT_FOUND, None), Error::Git2(e) => ( StatusCode::INTERNAL_SERVER_ERROR, Some(e.message().to_owned()), ), Error::Storage(err) if err.is_not_found() => (StatusCode::NOT_FOUND, None), + Error::StorageRef(err) if err.is_not_found() => (StatusCode::NOT_FOUND, None), Error::BadRequest(msg) => (StatusCode::BAD_REQUEST, Some(msg)), other => { tracing::error!("Error: {message}"); diff --git a/radicle-httpd/src/api/v1/projects.rs b/radicle-httpd/src/api/v1/projects.rs index f317a8af..6addd4d1 100644 --- a/radicle-httpd/src/api/v1/projects.rs +++ b/radicle-httpd/src/api/v1/projects.rs @@ -1298,6 +1298,19 @@ mod routes { ); } + #[tokio::test] + async fn test_projects_commits_not_found() { + let tmp = tempfile::tempdir().unwrap(); + let app = super::router(seed(tmp.path())); + let response = get( + &app, + format!("/projects/{RID}/commits/ffffffffffffffffffffffffffffffffffffffff"), + ) + .await; + + assert_eq!(response.status(), StatusCode::NOT_FOUND); + } + #[tokio::test] async fn test_projects_tree() { let tmp = tempfile::tempdir().unwrap(); @@ -1389,6 +1402,21 @@ mod routes { ); } + #[tokio::test] + async fn test_projects_tree_not_found() { + let tmp = tempfile::tempdir().unwrap(); + let app = super::router(seed(tmp.path())); + let response = get( + &app, + format!("/projects/{RID}/tree/ffffffffffffffffffffffffffffffffffffffff"), + ) + .await; + assert_eq!(response.status(), StatusCode::NOT_FOUND); + + let response = get(&app, format!("/projects/{RID}/tree/{HEAD}/unknown")).await; + assert_eq!(response.status(), StatusCode::NOT_FOUND); + } + #[tokio::test] async fn test_projects_remotes_root() { let tmp = tempfile::tempdir().unwrap(); @@ -1433,6 +1461,19 @@ mod routes { ); } + #[tokio::test] + async fn test_projects_remotes_not_found() { + let tmp = tempfile::tempdir().unwrap(); + let app = super::router(seed(tmp.path())); + let response = get( + &app, + format!("/projects/{RID}/remotes/z6MksFqXN3Yhqk8pTJdUGLwATkRfQvwZXPqR2qMEhbS9wzpT"), + ) + .await; + + assert_eq!(response.status(), StatusCode::NOT_FOUND); + } + #[tokio::test] async fn test_projects_blob() { let tmp = tempfile::tempdir().unwrap(); @@ -1468,6 +1509,15 @@ mod routes { ); } + #[tokio::test] + async fn test_projects_blob_not_found() { + let tmp = tempfile::tempdir().unwrap(); + let app = super::router(seed(tmp.path())); + let response = get(&app, format!("/projects/{RID}/blob/{HEAD}/unknown")).await; + + assert_eq!(response.status(), StatusCode::NOT_FOUND); + } + #[tokio::test] async fn test_projects_readme() { let tmp = tempfile::tempdir().unwrap(); diff --git a/radicle-httpd/src/lib.rs b/radicle-httpd/src/lib.rs index 1271912f..0fc159c2 100644 --- a/radicle-httpd/src/lib.rs +++ b/radicle-httpd/src/lib.rs @@ -121,6 +121,31 @@ fn router(options: Options, profile: Profile) -> anyhow::Result { Ok(app) } +pub mod logger { + use tracing::dispatcher::Dispatch; + + pub fn init() -> Result<(), tracing::subscriber::SetGlobalDefaultError> { + tracing::dispatcher::set_global_default(Dispatch::new(subscriber())) + } + + #[cfg(feature = "logfmt")] + pub fn subscriber() -> impl tracing::Subscriber { + use tracing_subscriber::layer::SubscriberExt as _; + use tracing_subscriber::EnvFilter; + + tracing_subscriber::Registry::default() + .with(EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new("info"))) + .with(tracing_logfmt::layer()) + } + + #[cfg(not(feature = "logfmt"))] + pub fn subscriber() -> impl tracing::Subscriber { + tracing_subscriber::FmtSubscriber::builder() + .with_target(false) + .finish() + } +} + #[cfg(test)] mod routes { use std::collections::HashMap; diff --git a/radicle-httpd/src/main.rs b/radicle-httpd/src/main.rs index 374fd088..47be4e47 100644 --- a/radicle-httpd/src/main.rs +++ b/radicle-httpd/src/main.rs @@ -3,36 +3,13 @@ use std::{collections::HashMap, process}; use radicle::prelude::Id; use radicle_httpd as httpd; -use tracing::dispatcher::Dispatch; - -#[cfg(feature = "logfmt")] -mod logger { - use tracing_subscriber::layer::SubscriberExt as _; - use tracing_subscriber::EnvFilter; - - pub fn subscriber() -> impl tracing::Subscriber { - tracing_subscriber::Registry::default() - .with(EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new("info"))) - .with(tracing_logfmt::layer()) - } -} - -#[cfg(not(feature = "logfmt"))] -mod logger { - pub fn subscriber() -> impl tracing::Subscriber { - tracing_subscriber::FmtSubscriber::builder() - .with_target(false) - .finish() - } -} #[tokio::main] async fn main() -> anyhow::Result<()> { let options = parse_options()?; - tracing::dispatcher::set_global_default(Dispatch::new(logger::subscriber())) - .expect("Global logger hasn't already been set"); - + // SAFETY: The logger is only initialized once. + httpd::logger::init().unwrap(); tracing::info!("version {}-{}", env!("CARGO_PKG_VERSION"), env!("GIT_HEAD")); match httpd::run(options).await { diff --git a/radicle-httpd/src/test.rs b/radicle-httpd/src/test.rs index f3c42d31..641096e2 100644 --- a/radicle-httpd/src/test.rs +++ b/radicle-httpd/src/test.rs @@ -72,6 +72,8 @@ pub fn seed(dir: &Path) -> Context { let profile = profile(home.as_path(), [0xff; 32]); let signer = Box::new(MockSigner::from_seed([0xff; 32])); + crate::logger::init().ok(); + seed_with_signer(dir, profile, &signer) }