diff --git a/src/filesystem.rs b/src/filesystem.rs index bc655b0c..dcca3226 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -95,7 +95,9 @@ impl FileSystem { for (i, component) in path.components().enumerate() { if let Component::Normal(c) = component { if !priviledged && i == 0 && c.eq_ignore_ascii_case("sqlpage") { - anyhow::bail!("Access to the sqlpage config directory is not allowed."); + anyhow::bail!(ErrorWithStatus { + status: actix_web::http::StatusCode::FORBIDDEN, + }); } } else { anyhow::bail!( diff --git a/src/webserver/error_with_status.rs b/src/webserver/error_with_status.rs index 23f1a8da..1b3c09c7 100644 --- a/src/webserver/error_with_status.rs +++ b/src/webserver/error_with_status.rs @@ -1,4 +1,4 @@ -use actix_web::http::StatusCode; +use actix_web::{http::StatusCode, ResponseError}; #[derive(Debug, PartialEq)] pub struct ErrorWithStatus { @@ -10,3 +10,12 @@ impl std::fmt::Display for ErrorWithStatus { } } impl std::error::Error for ErrorWithStatus {} + +impl ResponseError for ErrorWithStatus { + fn status_code(&self) -> StatusCode { + self.status + } + fn error_response(&self) -> actix_web::HttpResponse { + actix_web::HttpResponse::build(self.status).body(self.status.to_string()) + } +} diff --git a/src/webserver/http.rs b/src/webserver/http.rs index bfa8d77d..276b8e94 100644 --- a/src/webserver/http.rs +++ b/src/webserver/http.rs @@ -3,7 +3,7 @@ use crate::webserver::database::{stream_query_results, DbItem}; use crate::webserver::ErrorWithStatus; use crate::{AppState, Config, ParsedSqlFile}; use actix_web::dev::{fn_service, ServiceFactory, ServiceRequest}; -use actix_web::error::{ErrorInternalServerError, ErrorNotFound}; +use actix_web::error::ErrorInternalServerError; use actix_web::http::header::{ContentType, Header, HttpDate, IfModifiedSince, LastModified}; use actix_web::http::{header, StatusCode, Uri}; use actix_web::web::Form; @@ -411,24 +411,21 @@ async fn process_sql_request( .sql_file_cache .get(app_state, &sql_path) .await - .map_err(|e| { - log::error!("Error while trying to get SQL file: {:#}", e); - if e.downcast_ref() - == Some(&ErrorWithStatus { - status: StatusCode::NOT_FOUND, - }) - { - ErrorNotFound("The requested file was not found.") - } else { - ErrorInternalServerError(format!( - "An error occurred while trying to handle your request: {e:#}" - )) - } - })?; + .map_err(anyhow_err_to_actix)?; let response = render_sql(&mut req, sql_file).await?; Ok(req.into_response(response)) } +fn anyhow_err_to_actix(e: anyhow::Error) -> actix_web::Error { + log::error!("Error while trying to get SQL file: {:#}", e); + match e.downcast::() { + Ok(err) => actix_web::Error::from(err), + Err(e) => ErrorInternalServerError(format!( + "An error occurred while trying to handle your request: {e:#}" + )), + } +} + async fn serve_file( path: &str, state: &AppState, @@ -441,7 +438,7 @@ async fn serve_file( .file_system .modified_since(state, path.as_ref(), since, false) .await - .map_err(actix_web::error::ErrorBadRequest)?; + .map_err(anyhow_err_to_actix)?; if !modified { return Ok(HttpResponse::NotModified().finish()); } @@ -450,7 +447,7 @@ async fn serve_file( .file_system .read_file(state, path.as_ref(), false) .await - .map_err(actix_web::error::ErrorBadRequest) + .map_err(anyhow_err_to_actix) .map(|b| { HttpResponse::Ok() .insert_header( diff --git a/tests/index.rs b/tests/index.rs index b27e111d..17f4695a 100644 --- a/tests/index.rs +++ b/tests/index.rs @@ -1,4 +1,5 @@ use actix_web::{ + body::MessageBody, http::{self, header::ContentType}, test, }; @@ -6,7 +7,7 @@ use sqlpage::{app_config::AppConfig, webserver::http::main_handler, AppState}; #[actix_web::test] async fn test_index_ok() { - let resp = req_path("/").await; + let resp = req_path("/").await.unwrap(); assert_eq!(resp.status(), http::StatusCode::OK); let body = test::read_body(resp).await; assert!(body.starts_with(b"")); @@ -16,7 +17,20 @@ async fn test_index_ok() { assert!(!body.contains("error")); } -async fn req_path(path: &str) -> actix_web::dev::ServiceResponse { +#[actix_web::test] +async fn test_access_config_forbidden() { + let resp_result = req_path("/sqlpage/sqlpage.json").await; + assert!(resp_result.is_err(), "Accessing the config file should be forbidden, but we received a response: {resp_result:?}"); + let resp = resp_result.unwrap_err().error_response(); + assert_eq!(resp.status(), http::StatusCode::FORBIDDEN); + assert!( + String::from_utf8_lossy(&resp.into_body().try_into_bytes().unwrap()) + .to_lowercase() + .contains("forbidden"), + ); +} + +async fn req_path(path: &str) -> Result { init_log(); let config = test_config(); let state = AppState::init(&config).await.unwrap(); @@ -26,7 +40,7 @@ async fn req_path(path: &str) -> actix_web::dev::ServiceResponse { .app_data(data) .insert_header(ContentType::plaintext()) .to_srv_request(); - main_handler(req).await.unwrap() + main_handler(req).await } pub fn test_config() -> AppConfig { @@ -44,5 +58,5 @@ pub fn test_config() -> AppConfig { } fn init_log() { - env_logger::builder().is_test(true).try_init().unwrap(); + let _ = env_logger::builder().is_test(true).try_init(); }