Skip to content

Commit

Permalink
fix: send "404 Not Found" body on error response
Browse files Browse the repository at this point in the history
We were previously leaking opaque Git error messages, which were
confusing.
  • Loading branch information
n-dusan committed Aug 22, 2024
1 parent 851aa0d commit 28cacd6
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 5 deletions.
7 changes: 5 additions & 2 deletions src/server/api/serve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
#![allow(clippy::infinite_loop)]
use actix_web::{web, HttpRequest, HttpResponse, Responder};

use crate::utils::{git::Repo, http::get_contenttype, paths::clean_path};
use crate::{
server::errors::HTTPError,
utils::{git::Repo, http::get_contenttype, paths::clean_path},
};

use super::state::{RepoData as RepoState, Shared as SharedState};
/// Most-recent git commit
Expand All @@ -29,7 +32,7 @@ pub async fn serve(
Ok(content) => HttpResponse::Ok().insert_header(contenttype).body(content),
Err(error) => {
tracing::debug!("{path}: {error}",);
HttpResponse::BadRequest().into()
HttpResponse::NotFound().body(HTTPError::NotFound.to_string())
}
}
}
Expand Down
11 changes: 11 additions & 0 deletions src/server/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,17 @@ use actix_web::{error, http::StatusCode, HttpResponse};
use derive_more::{Display, Error};
use std::io;

/// Collection of possible HTTP errors
#[derive(Debug, Display, Error)]
pub enum HTTPError {
#[display(fmt = "404 Not Found")]
/// 404
NotFound,
#[display(fmt = "Unexpected server error")]
/// 500
InternalServerError,
}

/// Collection of possible CLI errors
#[derive(Debug, Display, Error)]
pub enum CliError {
Expand Down
6 changes: 3 additions & 3 deletions src/server/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use git2::{self, ErrorCode};
use std::path::PathBuf;
use tracing_actix_web::TracingLogger;

use super::errors::{CliError, StelaeError};
use super::errors::{CliError, HTTPError, StelaeError};
use crate::utils::git::{Repo, GIT_REQUEST_NOT_FOUND};
use crate::utils::http::get_contenttype;
use crate::{server::tracing::StelaeRootSpanBuilder, utils::paths::clean_path};
Expand Down Expand Up @@ -77,9 +77,9 @@ fn blob_error_response(error: &anyhow::Error, namespace: &str, name: &str) -> Ht
match error {
// TODO: Obviously it's better to use custom `Error` types
_ if error.to_string() == GIT_REQUEST_NOT_FOUND => {
HttpResponse::NotFound().body(GIT_REQUEST_NOT_FOUND)
HttpResponse::NotFound().body(HTTPError::NotFound.to_string())
}
_ => HttpResponse::InternalServerError().body("Unexpected server error"),
_ => HttpResponse::InternalServerError().body(HTTPError::InternalServerError.to_string()),
}
}

Expand Down

0 comments on commit 28cacd6

Please sign in to comment.