From 59a2960e42ed2568a16bb3798c9b74cfaa28c765 Mon Sep 17 00:00:00 2001 From: Vegard Sandengen <97030809+vegardgs-ksat@users.noreply.github.com> Date: Mon, 11 Nov 2024 16:56:25 +0000 Subject: [PATCH] Add ErrorKind::DeserializeError to specialize ErrorKind::Message (`extract::path::ErrorKind`) (#2720) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit introduces another `extract::path::ErrorKind` variant that captures the serde error nominally captured through the `serde::de::Error` trait impl on `PathDeserializeError`. We augment the deserialization error with the captured (key, value), allowing `extract::Path`, and wrapping extractors, to gain programmatic access to the key name, and attempted deserialized value. The `PathDeserializationError::custom` is used two places in addition to capture the deserialization error. These usages should still be unaffected. Co-authored-by: David Mládek --- axum-extra/src/extract/optional_path.rs | 2 +- axum/CHANGELOG.md | 4 ++ axum/src/extract/path/de.rs | 52 ++++++++++++++++++++- axum/src/extract/path/mod.rs | 62 +++++++++++++++++++++++-- 4 files changed, 114 insertions(+), 6 deletions(-) diff --git a/axum-extra/src/extract/optional_path.rs b/axum-extra/src/extract/optional_path.rs index 0d41a66c..53443f19 100644 --- a/axum-extra/src/extract/optional_path.rs +++ b/axum-extra/src/extract/optional_path.rs @@ -94,7 +94,7 @@ mod tests { let res = client.get("/NaN").await; assert_eq!( res.text().await, - "Invalid URL: Cannot parse `\"NaN\"` to a `u32`" + "Invalid URL: Cannot parse `NaN` to a `u32`" ); } } diff --git a/axum/CHANGELOG.md b/axum/CHANGELOG.md index 81c9587a..94eca6b2 100644 --- a/axum/CHANGELOG.md +++ b/axum/CHANGELOG.md @@ -19,6 +19,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 This allows middleware to add bodies to requests without needing to manually set `content-length` ([#2897]) - **breaking:** Remove `WebSocket::close`. Users should explicitly send close messages themselves. ([#2974]) +- **added:** Extend `FailedToDeserializePathParams::kind` enum with (`ErrorKind::DeserializeError`) + This new variant captures both `key`, `value`, and `message` from named path parameters parse errors, + instead of only deserialization error message in `ErrorKind::Message`. ([#2720]) [#2897]: https://github.com/tokio-rs/axum/pull/2897 [#2903]: https://github.com/tokio-rs/axum/pull/2903 @@ -28,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#2974]: https://github.com/tokio-rs/axum/pull/2974 [#2978]: https://github.com/tokio-rs/axum/pull/2978 [#2992]: https://github.com/tokio-rs/axum/pull/2992 +[#2720]: https://github.com/tokio-rs/axum/pull/2720 # 0.8.0 diff --git a/axum/src/extract/path/de.rs b/axum/src/extract/path/de.rs index 2d0b04f5..77fbd76c 100644 --- a/axum/src/extract/path/de.rs +++ b/axum/src/extract/path/de.rs @@ -94,7 +94,21 @@ impl<'de> Deserializer<'de> for PathDeserializer<'de> { .got(self.url_params.len()) .expected(1)); } - visitor.visit_borrowed_str(&self.url_params[0].1) + let key = &self.url_params[0].0; + let value = &self.url_params[0].1; + visitor + .visit_borrowed_str(value) + .map_err(|e: PathDeserializationError| { + if let ErrorKind::Message(message) = &e.kind { + PathDeserializationError::new(ErrorKind::DeserializeError { + key: key.to_string(), + value: value.as_str().to_owned(), + message: message.to_owned(), + }) + } else { + e + } + }) } fn deserialize_unit(self, visitor: V) -> Result @@ -362,7 +376,19 @@ impl<'de> Deserializer<'de> for ValueDeserializer<'de> { where V: Visitor<'de>, { - visitor.visit_borrowed_str(self.value) + visitor + .visit_borrowed_str(self.value) + .map_err(|e: PathDeserializationError| { + if let (ErrorKind::Message(message), Some(key)) = (&e.kind, self.key.as_ref()) { + PathDeserializationError::new(ErrorKind::DeserializeError { + key: key.key().to_owned(), + value: self.value.as_str().to_owned(), + message: message.to_owned(), + }) + } else { + e + } + }) } fn deserialize_bytes(self, visitor: V) -> Result @@ -608,6 +634,15 @@ enum KeyOrIdx<'de> { Idx { idx: usize, key: &'de str }, } +impl<'de> KeyOrIdx<'de> { + fn key(&self) -> &'de str { + match &self { + Self::Key(key) => key, + Self::Idx { key, .. } => key, + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -926,4 +961,17 @@ mod tests { } ); } + + #[test] + fn test_deserialize_key_value() { + test_parse_error!( + vec![("id", "123123-123-123123")], + uuid::Uuid, + ErrorKind::DeserializeError { + key: "id".to_owned(), + value: "123123-123-123123".to_owned(), + message: "UUID parsing failed: invalid group count: expected 5, found 3".to_owned(), + } + ); + } } diff --git a/axum/src/extract/path/mod.rs b/axum/src/extract/path/mod.rs index ebef78db..26a23fc8 100644 --- a/axum/src/extract/path/mod.rs +++ b/axum/src/extract/path/mod.rs @@ -303,6 +303,16 @@ pub enum ErrorKind { name: &'static str, }, + /// Failed to deserialize the value with a custom deserialization error. + DeserializeError { + /// The key at which the invalid value was located. + key: String, + /// The value that failed to deserialize. + value: String, + /// The deserializaation failure message. + message: String, + }, + /// Catch-all variant for errors that don't fit any other variant. Message(String), } @@ -331,20 +341,25 @@ impl fmt::Display for ErrorKind { expected_type, } => write!( f, - "Cannot parse `{key}` with value `{value:?}` to a `{expected_type}`" + "Cannot parse `{key}` with value `{value}` to a `{expected_type}`" ), ErrorKind::ParseError { value, expected_type, - } => write!(f, "Cannot parse `{value:?}` to a `{expected_type}`"), + } => write!(f, "Cannot parse `{value}` to a `{expected_type}`"), ErrorKind::ParseErrorAtIndex { index, value, expected_type, } => write!( f, - "Cannot parse value at index {index} with value `{value:?}` to a `{expected_type}`" + "Cannot parse value at index {index} with value `{value}` to a `{expected_type}`" ), + ErrorKind::DeserializeError { + key, + value, + message, + } => write!(f, "Cannot parse `{key}` with value `{value}`: {message}"), } } } @@ -369,6 +384,7 @@ impl FailedToDeserializePathParams { pub fn body_text(&self) -> String { match self.0.kind { ErrorKind::Message(_) + | ErrorKind::DeserializeError { .. } | ErrorKind::InvalidUtf8InPathParam { .. } | ErrorKind::ParseError { .. } | ErrorKind::ParseErrorAtIndex { .. } @@ -383,6 +399,7 @@ impl FailedToDeserializePathParams { pub fn status(&self) -> StatusCode { match self.0.kind { ErrorKind::Message(_) + | ErrorKind::DeserializeError { .. } | ErrorKind::InvalidUtf8InPathParam { .. } | ErrorKind::ParseError { .. } | ErrorKind::ParseErrorAtIndex { .. } @@ -917,4 +934,43 @@ mod tests { let body = res.text().await; assert_eq!(body, "a=foo b=bar c=baz"); } + + #[crate::test] + async fn deserialize_error_single_value() { + let app = Router::new().route( + "/resources/{res}", + get(|res: Path| async move { + let _res = res; + }), + ); + + let client = TestClient::new(app); + let res = client.get("/resources/123123-123-123123").await; + let body = res.text().await; + assert_eq!( + body, + r#"Invalid URL: Cannot parse `res` with value `123123-123-123123`: UUID parsing failed: invalid group count: expected 5, found 3"# + ); + } + + #[crate::test] + async fn deserialize_error_multi_value() { + let app = Router::new().route( + "/resources/{res}/sub/{sub}", + get( + |Path((res, sub)): Path<(uuid::Uuid, uuid::Uuid)>| async move { + let _res = res; + let _sub = sub; + }, + ), + ); + + let client = TestClient::new(app); + let res = client.get("/resources/456456-123-456456/sub/123").await; + let body = res.text().await; + assert_eq!( + body, + r#"Invalid URL: Cannot parse `res` with value `456456-123-456456`: UUID parsing failed: invalid group count: expected 5, found 3"# + ); + } }