From 810a89fde08f4ea4dbe85e98b9f640df52cb6497 Mon Sep 17 00:00:00 2001 From: Leon Lux Date: Mon, 28 Oct 2024 13:32:26 +0100 Subject: [PATCH 01/16] add InternalServerError to quickly respond with an error that is not IntoResponse --- axum/src/response/error_response.rs | 49 +++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 axum/src/response/error_response.rs diff --git a/axum/src/response/error_response.rs b/axum/src/response/error_response.rs new file mode 100644 index 0000000000..72fd8edd53 --- /dev/null +++ b/axum/src/response/error_response.rs @@ -0,0 +1,49 @@ +use axum_core::response::{IntoResponse, Response}; +use http::StatusCode; +use std::error::Error; +use std::fmt::Display; +use std::io::Write; + +/// Convenience response to create an error response from a non-IntoResponse error +/// +/// This provides a method to quickly respond with an error that does not implement +/// the IntoResponse trait itself. When run in debug configuration, the error chain is +/// included in the response. In release builds, only a generic message will be shown, as errors +/// could contain sensitive data not meant to be shown to users. +/// +/// ```rust,no_run +/// use axum::response::{InternalServerError, IntoResponse, NoContent}; +/// # use std::io::{Error, ErrorKind}; +/// # fn try_thing() -> Result<(), Error> { +/// # Err(Error::new(ErrorKind::Other, "error")) +/// # } +/// +/// async fn maybe_error() -> Result> { +/// try_thing().map_err(InternalServerError)?; +/// // do something on success +/// # Ok(String::from("ok")) +/// } +/// ``` +#[derive(Debug)] +pub struct InternalServerError(pub T); + +impl IntoResponse for InternalServerError { + fn into_response(self) -> Response { + if cfg!(debug_assertions) { + let mut body = Vec::new(); + write!(body, "{}", self.0); + let mut e: &dyn Error = &self.0; + while let Some(new_e) = e.source() { + e = new_e; + write!(body, ": {e}").unwrap(); + } + (StatusCode::INTERNAL_SERVER_ERROR, body).into_response() + } else { + ( + StatusCode::INTERNAL_SERVER_ERROR, + "An error occurred while processing your request", + ) + .into_response() + } + } +} From f12eb4f813e540d9694f646a27cd0e1f6d6e5b2c Mon Sep 17 00:00:00 2001 From: Leon Lux Date: Mon, 28 Oct 2024 13:32:43 +0100 Subject: [PATCH 02/16] expose InternalServerError in mod.rs --- axum/src/response/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/axum/src/response/mod.rs b/axum/src/response/mod.rs index dd616dff57..0fccfdf621 100644 --- a/axum/src/response/mod.rs +++ b/axum/src/response/mod.rs @@ -4,6 +4,7 @@ use http::{header, HeaderValue, StatusCode}; mod redirect; +mod error_response; #[cfg(feature = "tokio")] pub mod sse; @@ -30,6 +31,9 @@ pub use self::redirect::Redirect; #[cfg(feature = "tokio")] pub use sse::Sse; +#[doc(inline)] +pub use error_response::InternalServerError; + /// An HTML response. /// /// Will automatically get `Content-Type: text/html`. From f6b29f01cfbd2cf1d08f061df9016d031e86cec8 Mon Sep 17 00:00:00 2001 From: Leon Lux Date: Tue, 29 Oct 2024 12:13:10 +0100 Subject: [PATCH 03/16] add test, update InternalServerError to behave the same in debug and release --- axum/src/response/error_response.rs | 46 ++++++++++++++++++----------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/axum/src/response/error_response.rs b/axum/src/response/error_response.rs index 72fd8edd53..6eef5a53fe 100644 --- a/axum/src/response/error_response.rs +++ b/axum/src/response/error_response.rs @@ -7,9 +7,9 @@ use std::io::Write; /// Convenience response to create an error response from a non-IntoResponse error /// /// This provides a method to quickly respond with an error that does not implement -/// the IntoResponse trait itself. When run in debug configuration, the error chain is -/// included in the response. In release builds, only a generic message will be shown, as errors -/// could contain sensitive data not meant to be shown to users. +/// the IntoResponse trait itself. This type should only be used for debugging purposes or internal +/// facing applications, as it includes the full error chain with descriptions, +/// thus leaking information that could possibly be sensitive. /// /// ```rust,no_run /// use axum::response::{InternalServerError, IntoResponse, NoContent}; @@ -29,21 +29,31 @@ pub struct InternalServerError(pub T); impl IntoResponse for InternalServerError { fn into_response(self) -> Response { - if cfg!(debug_assertions) { - let mut body = Vec::new(); - write!(body, "{}", self.0); - let mut e: &dyn Error = &self.0; - while let Some(new_e) = e.source() { - e = new_e; - write!(body, ": {e}").unwrap(); - } - (StatusCode::INTERNAL_SERVER_ERROR, body).into_response() - } else { - ( - StatusCode::INTERNAL_SERVER_ERROR, - "An error occurred while processing your request", - ) - .into_response() + let mut body = Vec::new(); + write!(body, "{}", self.0); + let mut e: &dyn Error = &self.0; + while let Some(new_e) = e.source() { + e = new_e; + write!(body, ": {e}").unwrap(); } + (StatusCode::INTERNAL_SERVER_ERROR, body).into_response() } } + +#[cfg(test)] +mod tests { + use super::*; + use std::io::{Error, ErrorKind}; + + #[test] + fn internal_server_error() { + let response = InternalServerError(Error::new(ErrorKind::Other, "Test")).into_response(); + assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR); + } + + #[test] + fn with_error_chain() { + todo!(); + } + +} From 4204e31f47bcb4b67f6c1d9986c3f7e66dc4663a Mon Sep 17 00:00:00 2001 From: Leon Lux Date: Wed, 30 Oct 2024 10:11:52 +0100 Subject: [PATCH 04/16] unwrap write call, remove no_run from docs --- axum/src/response/error_response.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/axum/src/response/error_response.rs b/axum/src/response/error_response.rs index 6eef5a53fe..23f815dbfa 100644 --- a/axum/src/response/error_response.rs +++ b/axum/src/response/error_response.rs @@ -1,7 +1,6 @@ use axum_core::response::{IntoResponse, Response}; use http::StatusCode; use std::error::Error; -use std::fmt::Display; use std::io::Write; /// Convenience response to create an error response from a non-IntoResponse error @@ -11,7 +10,7 @@ use std::io::Write; /// facing applications, as it includes the full error chain with descriptions, /// thus leaking information that could possibly be sensitive. /// -/// ```rust,no_run +/// ```rust /// use axum::response::{InternalServerError, IntoResponse, NoContent}; /// # use std::io::{Error, ErrorKind}; /// # fn try_thing() -> Result<(), Error> { @@ -30,7 +29,7 @@ pub struct InternalServerError(pub T); impl IntoResponse for InternalServerError { fn into_response(self) -> Response { let mut body = Vec::new(); - write!(body, "{}", self.0); + write!(body, "{}", self.0).unwrap(); let mut e: &dyn Error = &self.0; while let Some(new_e) = e.source() { e = new_e; @@ -51,9 +50,4 @@ mod tests { assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR); } - #[test] - fn with_error_chain() { - todo!(); - } - } From db723a392521d6d99ad5be57f5887a184d5e5d0f Mon Sep 17 00:00:00 2001 From: Leon Lux Date: Wed, 30 Oct 2024 10:22:55 +0100 Subject: [PATCH 05/16] reformat code --- axum/src/response/error_response.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/axum/src/response/error_response.rs b/axum/src/response/error_response.rs index 23f815dbfa..8268a01e42 100644 --- a/axum/src/response/error_response.rs +++ b/axum/src/response/error_response.rs @@ -49,5 +49,4 @@ mod tests { let response = InternalServerError(Error::new(ErrorKind::Other, "Test")).into_response(); assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR); } - } From 298efc289e229e5f4898b600355b2e0d9fcfe695 Mon Sep 17 00:00:00 2001 From: Leon Lux Date: Sat, 2 Nov 2024 15:51:03 +0100 Subject: [PATCH 06/16] move InternalServerError to axum-extra --- {axum => axum-extra}/src/response/error_response.rs | 3 ++- axum-extra/src/response/mod.rs | 4 ++++ axum/src/response/mod.rs | 4 ---- 3 files changed, 6 insertions(+), 5 deletions(-) rename {axum => axum-extra}/src/response/error_response.rs (94%) diff --git a/axum/src/response/error_response.rs b/axum-extra/src/response/error_response.rs similarity index 94% rename from axum/src/response/error_response.rs rename to axum-extra/src/response/error_response.rs index 8268a01e42..22fdaf1c08 100644 --- a/axum/src/response/error_response.rs +++ b/axum-extra/src/response/error_response.rs @@ -11,7 +11,8 @@ use std::io::Write; /// thus leaking information that could possibly be sensitive. /// /// ```rust -/// use axum::response::{InternalServerError, IntoResponse, NoContent}; +/// use axum_extra::response::InternalServerError; +/// use axum_core::response::IntoResponse; /// # use std::io::{Error, ErrorKind}; /// # fn try_thing() -> Result<(), Error> { /// # Err(Error::new(ErrorKind::Other, "error")) diff --git a/axum-extra/src/response/mod.rs b/axum-extra/src/response/mod.rs index 12e104d8f1..4935f2d798 100644 --- a/axum-extra/src/response/mod.rs +++ b/axum-extra/src/response/mod.rs @@ -9,6 +9,10 @@ mod attachment; #[cfg(feature = "multipart")] pub mod multiple; +mod error_response; + +pub use error_response::InternalServerError; + #[cfg(feature = "erased-json")] pub use erased_json::ErasedJson; diff --git a/axum/src/response/mod.rs b/axum/src/response/mod.rs index 0fccfdf621..dd616dff57 100644 --- a/axum/src/response/mod.rs +++ b/axum/src/response/mod.rs @@ -4,7 +4,6 @@ use http::{header, HeaderValue, StatusCode}; mod redirect; -mod error_response; #[cfg(feature = "tokio")] pub mod sse; @@ -31,9 +30,6 @@ pub use self::redirect::Redirect; #[cfg(feature = "tokio")] pub use sse::Sse; -#[doc(inline)] -pub use error_response::InternalServerError; - /// An HTML response. /// /// Will automatically get `Content-Type: text/html`. From 444d80065bcbe00b7ad686e8ff45fe00c38b9e64 Mon Sep 17 00:00:00 2001 From: Leon Lux Date: Sat, 2 Nov 2024 16:08:32 +0100 Subject: [PATCH 07/16] add feature flag for error_response --- axum-extra/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/axum-extra/Cargo.toml b/axum-extra/Cargo.toml index 52072240c6..9d13813070 100644 --- a/axum-extra/Cargo.toml +++ b/axum-extra/Cargo.toml @@ -16,6 +16,7 @@ default = ["tracing", "multipart"] async-read-body = ["dep:tokio-util", "tokio-util?/io", "dep:tokio"] attachment = ["dep:tracing"] +error_response = ["dep:tracing"] cookie = ["dep:cookie"] cookie-private = ["cookie", "cookie?/private"] cookie-signed = ["cookie", "cookie?/signed"] From d41f33abce90335fed3f59af162883cceec7c37c Mon Sep 17 00:00:00 2001 From: Leon Lux Date: Sat, 2 Nov 2024 16:08:50 +0100 Subject: [PATCH 08/16] change InternalServerError to log the error chain instead of responding with it --- axum-extra/src/response/error_response.rs | 8 +++++++- axum-extra/src/response/mod.rs | 2 ++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/axum-extra/src/response/error_response.rs b/axum-extra/src/response/error_response.rs index 22fdaf1c08..e92be46c26 100644 --- a/axum-extra/src/response/error_response.rs +++ b/axum-extra/src/response/error_response.rs @@ -2,6 +2,7 @@ use axum_core::response::{IntoResponse, Response}; use http::StatusCode; use std::error::Error; use std::io::Write; +use tracing::error; /// Convenience response to create an error response from a non-IntoResponse error /// @@ -36,7 +37,12 @@ impl IntoResponse for InternalServerError { e = new_e; write!(body, ": {e}").unwrap(); } - (StatusCode::INTERNAL_SERVER_ERROR, body).into_response() + error!("Internal server error: {}", body); + ( + StatusCode::INTERNAL_SERVER_ERROR, + "An error occurred while processing your request.", + ) + .into_response() } } diff --git a/axum-extra/src/response/mod.rs b/axum-extra/src/response/mod.rs index 4935f2d798..bac7d040fe 100644 --- a/axum-extra/src/response/mod.rs +++ b/axum-extra/src/response/mod.rs @@ -9,8 +9,10 @@ mod attachment; #[cfg(feature = "multipart")] pub mod multiple; +#[cfg(feature = "error_response")] mod error_response; +#[cfg(feature = "error_response")] pub use error_response::InternalServerError; #[cfg(feature = "erased-json")] From 3035dba2d81f79f060a0a77385b953e7d46f450c Mon Sep 17 00:00:00 2001 From: Leon Lux Date: Sat, 2 Nov 2024 16:15:36 +0100 Subject: [PATCH 09/16] change type of error vec --- axum-extra/src/response/error_response.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/axum-extra/src/response/error_response.rs b/axum-extra/src/response/error_response.rs index e92be46c26..5e309c555b 100644 --- a/axum-extra/src/response/error_response.rs +++ b/axum-extra/src/response/error_response.rs @@ -30,7 +30,7 @@ pub struct InternalServerError(pub T); impl IntoResponse for InternalServerError { fn into_response(self) -> Response { - let mut body = Vec::new(); + let mut body: Vec = Vec::new(); write!(body, "{}", self.0).unwrap(); let mut e: &dyn Error = &self.0; while let Some(new_e) = e.source() { From b1aa73c0d12dbe723331e68a5bd08c486bfa0070 Mon Sep 17 00:00:00 2001 From: Leon Lux Date: Sat, 2 Nov 2024 16:19:13 +0100 Subject: [PATCH 10/16] fix errors in InternalServerError --- axum-extra/src/response/error_response.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/axum-extra/src/response/error_response.rs b/axum-extra/src/response/error_response.rs index 5e309c555b..334e6d8c7d 100644 --- a/axum-extra/src/response/error_response.rs +++ b/axum-extra/src/response/error_response.rs @@ -30,14 +30,14 @@ pub struct InternalServerError(pub T); impl IntoResponse for InternalServerError { fn into_response(self) -> Response { - let mut body: Vec = Vec::new(); - write!(body, "{}", self.0).unwrap(); + let mut error: Vec = Vec::new(); + error.push(format!("{}", self.0)); let mut e: &dyn Error = &self.0; while let Some(new_e) = e.source() { e = new_e; - write!(body, ": {e}").unwrap(); + error.push(format!(": {e}")) } - error!("Internal server error: {}", body); + error!("Internal server error: {}", error); ( StatusCode::INTERNAL_SERVER_ERROR, "An error occurred while processing your request.", From c6a08169834665ef70d1fa637cbab4fc0a0077a9 Mon Sep 17 00:00:00 2001 From: Leon Lux Date: Sat, 2 Nov 2024 16:30:45 +0100 Subject: [PATCH 11/16] use tracings ability to directly log &dyn Error --- axum-extra/src/response/error_response.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/axum-extra/src/response/error_response.rs b/axum-extra/src/response/error_response.rs index 334e6d8c7d..f22c2e3e94 100644 --- a/axum-extra/src/response/error_response.rs +++ b/axum-extra/src/response/error_response.rs @@ -1,7 +1,6 @@ use axum_core::response::{IntoResponse, Response}; use http::StatusCode; use std::error::Error; -use std::io::Write; use tracing::error; /// Convenience response to create an error response from a non-IntoResponse error @@ -30,14 +29,8 @@ pub struct InternalServerError(pub T); impl IntoResponse for InternalServerError { fn into_response(self) -> Response { - let mut error: Vec = Vec::new(); - error.push(format!("{}", self.0)); let mut e: &dyn Error = &self.0; - while let Some(new_e) = e.source() { - e = new_e; - error.push(format!(": {e}")) - } - error!("Internal server error: {}", error); + error!(error = e); ( StatusCode::INTERNAL_SERVER_ERROR, "An error occurred while processing your request.", From 5d0e2c0c44ee6a95faa90381859697f9c3c42ccd Mon Sep 17 00:00:00 2001 From: Leon Lux Date: Sat, 2 Nov 2024 16:41:08 +0100 Subject: [PATCH 12/16] use tracing error correctly --- axum-extra/src/response/error_response.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/axum-extra/src/response/error_response.rs b/axum-extra/src/response/error_response.rs index f22c2e3e94..364b83f050 100644 --- a/axum-extra/src/response/error_response.rs +++ b/axum-extra/src/response/error_response.rs @@ -29,8 +29,8 @@ pub struct InternalServerError(pub T); impl IntoResponse for InternalServerError { fn into_response(self) -> Response { - let mut e: &dyn Error = &self.0; - error!(error = e); + let err = &self.0; + error!(%err); ( StatusCode::INTERNAL_SERVER_ERROR, "An error occurred while processing your request.", From fa781b470f5d5e0a3298b6b88717ba24102cc11f Mon Sep 17 00:00:00 2001 From: Leon Lux Date: Sun, 3 Nov 2024 18:37:02 +0100 Subject: [PATCH 13/16] properly use tracings dyn Error support --- axum-extra/src/response/error_response.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/axum-extra/src/response/error_response.rs b/axum-extra/src/response/error_response.rs index 364b83f050..a0d467501e 100644 --- a/axum-extra/src/response/error_response.rs +++ b/axum-extra/src/response/error_response.rs @@ -27,10 +27,10 @@ use tracing::error; #[derive(Debug)] pub struct InternalServerError(pub T); -impl IntoResponse for InternalServerError { +impl IntoResponse for InternalServerError { fn into_response(self) -> Response { let err = &self.0; - error!(%err); + error!(error = err as &dyn Error); ( StatusCode::INTERNAL_SERVER_ERROR, "An error occurred while processing your request.", From 5a91719b6dd215829fdd2f905dda4c86bfc59a87 Mon Sep 17 00:00:00 2001 From: Leon Lux Date: Sun, 3 Nov 2024 18:42:35 +0100 Subject: [PATCH 14/16] fix usage of inner error field --- axum-extra/src/response/error_response.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/axum-extra/src/response/error_response.rs b/axum-extra/src/response/error_response.rs index a0d467501e..7e4c9aea85 100644 --- a/axum-extra/src/response/error_response.rs +++ b/axum-extra/src/response/error_response.rs @@ -29,8 +29,7 @@ pub struct InternalServerError(pub T); impl IntoResponse for InternalServerError { fn into_response(self) -> Response { - let err = &self.0; - error!(error = err as &dyn Error); + error!(error = &self.0 as &dyn Error); ( StatusCode::INTERNAL_SERVER_ERROR, "An error occurred while processing your request.", From 3a54fd3435c257bfefd6fab688f497e10fbfa2c1 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Sun, 17 Nov 2024 21:58:08 +0100 Subject: [PATCH 15/16] Enable tracing's std feature for error_response --- axum-extra/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/axum-extra/Cargo.toml b/axum-extra/Cargo.toml index 9d13813070..1e403ef581 100644 --- a/axum-extra/Cargo.toml +++ b/axum-extra/Cargo.toml @@ -16,7 +16,7 @@ default = ["tracing", "multipart"] async-read-body = ["dep:tokio-util", "tokio-util?/io", "dep:tokio"] attachment = ["dep:tracing"] -error_response = ["dep:tracing"] +error_response = ["dep:tracing", "tracing/std"] cookie = ["dep:cookie"] cookie-private = ["cookie", "cookie?/private"] cookie-signed = ["cookie", "cookie?/signed"] From ed072023a05c036646a9d5fd5f420abf77bd1ca1 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Sun, 17 Nov 2024 22:03:41 +0100 Subject: [PATCH 16/16] Improve docs --- axum-extra/src/response/error_response.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/axum-extra/src/response/error_response.rs b/axum-extra/src/response/error_response.rs index 7e4c9aea85..0706950555 100644 --- a/axum-extra/src/response/error_response.rs +++ b/axum-extra/src/response/error_response.rs @@ -3,10 +3,10 @@ use http::StatusCode; use std::error::Error; use tracing::error; -/// Convenience response to create an error response from a non-IntoResponse error +/// Convenience response to create an error response from a non-[`IntoResponse`] error /// /// This provides a method to quickly respond with an error that does not implement -/// the IntoResponse trait itself. This type should only be used for debugging purposes or internal +/// the `IntoResponse` trait itself. This type should only be used for debugging purposes or internal /// facing applications, as it includes the full error chain with descriptions, /// thus leaking information that could possibly be sensitive. ///