From 86df295ee29587a0800c4b59f948b5d0fe046744 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Tue, 4 Jan 2022 15:19:29 +0000 Subject: [PATCH] fully percent decode path segments when capturing (#2566) --- actix-router/CHANGES.md | 3 + actix-router/src/de.rs | 124 ++++++++++++++++++++++++++-------------- src/types/path.rs | 12 ++++ 3 files changed, 97 insertions(+), 42 deletions(-) diff --git a/actix-router/CHANGES.md b/actix-router/CHANGES.md index c85d10e2a..7b8615570 100644 --- a/actix-router/CHANGES.md +++ b/actix-router/CHANGES.md @@ -1,8 +1,11 @@ # Changes ## Unreleased - 2021-xx-xx +- `PathDeserializer` now decodes all percent encoded characters in dynamic segments. [#2566] - Minimum supported Rust version (MSRV) is now 1.54. +[#2566]: https://github.com/actix/actix-net/pull/2566 + ## 0.5.0-beta.3 - 2021-12-17 - Minimum supported Rust version (MSRV) is now 1.52. diff --git a/actix-router/src/de.rs b/actix-router/src/de.rs index 775c48b8a..ec7b1066a 100644 --- a/actix-router/src/de.rs +++ b/actix-router/src/de.rs @@ -2,7 +2,11 @@ use serde::de::{self, Deserializer, Error as DeError, Visitor}; use serde::forward_to_deserialize_any; use crate::path::{Path, PathIter}; -use crate::ResourcePath; +use crate::{Quoter, ResourcePath}; + +thread_local! { + static FULL_QUOTER: Quoter = Quoter::new(b"+/%", b""); +} macro_rules! unsupported_type { ($trait_fn:ident, $name:expr) => { @@ -10,16 +14,13 @@ macro_rules! unsupported_type { where V: Visitor<'de>, { - Err(de::value::Error::custom(concat!( - "unsupported type: ", - $name - ))) + Err(de::Error::custom(concat!("unsupported type: ", $name))) } }; } macro_rules! parse_single_value { - ($trait_fn:ident, $visit_fn:ident, $tp:tt) => { + ($trait_fn:ident, $visit_fn:ident, $tp:expr) => { fn $trait_fn(self, visitor: V) -> Result where V: Visitor<'de>, @@ -33,18 +34,39 @@ macro_rules! parse_single_value { .as_str(), )) } else { - let v = self.path[0].parse().map_err(|_| { - de::value::Error::custom(format!( - "can not parse {:?} to a {}", - &self.path[0], $tp - )) + let decoded = FULL_QUOTER + .with(|q| q.requote(self.path[0].as_bytes())) + .unwrap_or_else(|| self.path[0].to_owned()); + + let v = decoded.parse().map_err(|_| { + de::Error::custom(format!("can not parse {:?} to a {}", &self.path[0], $tp)) })?; + visitor.$visit_fn(v) } } }; } +macro_rules! parse_value { + ($trait_fn:ident, $visit_fn:ident, $tp:tt) => { + fn $trait_fn(self, visitor: V) -> Result + where + V: Visitor<'de>, + { + let decoded = FULL_QUOTER + .with(|q| q.requote(self.value.as_bytes())) + .unwrap_or_else(|| self.value.to_owned()); + + let v = decoded.parse().map_err(|_| { + de::value::Error::custom(format!("can not parse {:?} to a {}", self.value, $tp)) + })?; + + visitor.$visit_fn(v) + } + }; +} + pub struct PathDeserializer<'de, T: ResourcePath> { path: &'de Path, } @@ -172,23 +194,6 @@ impl<'de, T: ResourcePath + 'de> Deserializer<'de> for PathDeserializer<'de, T> } } - fn deserialize_str(self, visitor: V) -> Result - where - V: Visitor<'de>, - { - if self.path.segment_count() != 1 { - Err(de::value::Error::custom( - format!( - "wrong number of parameters: {} expected 1", - self.path.segment_count() - ) - .as_str(), - )) - } else { - visitor.visit_str(&self.path[0]) - } - } - fn deserialize_seq(self, visitor: V) -> Result where V: Visitor<'de>, @@ -215,6 +220,7 @@ impl<'de, T: ResourcePath + 'de> Deserializer<'de> for PathDeserializer<'de, T> parse_single_value!(deserialize_u64, visit_u64, "u64"); parse_single_value!(deserialize_f32, visit_f32, "f32"); parse_single_value!(deserialize_f64, visit_f64, "f64"); + parse_single_value!(deserialize_str, visit_string, "String"); parse_single_value!(deserialize_string, visit_string, "String"); parse_single_value!(deserialize_byte_buf, visit_string, "String"); parse_single_value!(deserialize_char, visit_char, "char"); @@ -279,20 +285,6 @@ impl<'de> Deserializer<'de> for Key<'de> { } } -macro_rules! parse_value { - ($trait_fn:ident, $visit_fn:ident, $tp:tt) => { - fn $trait_fn(self, visitor: V) -> Result - where - V: Visitor<'de>, - { - let v = self.value.parse().map_err(|_| { - de::value::Error::custom(format!("can not parse {:?} to a {}", self.value, $tp)) - })?; - visitor.$visit_fn(v) - } - }; -} - struct Value<'de> { value: &'de str, } @@ -497,6 +489,7 @@ mod tests { use super::*; use crate::path::Path; use crate::router::Router; + use crate::ResourceDef; #[derive(Deserialize)] struct MyStruct { @@ -657,6 +650,53 @@ mod tests { assert!(format!("{:?}", s).contains("can not parse")); } + #[test] + fn deserialize_path_decode_string() { + let rdef = ResourceDef::new("/{key}"); + + let mut path = Path::new("/%25"); + rdef.capture_match_info(&mut path); + let de = PathDeserializer::new(&path); + let segment: String = serde::Deserialize::deserialize(de).unwrap(); + assert_eq!(segment, "%"); + + let mut path = Path::new("/%2F"); + rdef.capture_match_info(&mut path); + let de = PathDeserializer::new(&path); + let segment: String = serde::Deserialize::deserialize(de).unwrap(); + assert_eq!(segment, "/") + } + + #[test] + fn deserialize_path_decode_seq() { + let rdef = ResourceDef::new("/{key}/{value}"); + + let mut path = Path::new("/%25/%2F"); + rdef.capture_match_info(&mut path); + let de = PathDeserializer::new(&path); + let segment: (String, String) = serde::Deserialize::deserialize(de).unwrap(); + assert_eq!(segment.0, "%"); + assert_eq!(segment.1, "/"); + } + + #[test] + fn deserialize_path_decode_map() { + #[derive(Deserialize)] + struct Vals { + key: String, + value: String, + } + + let rdef = ResourceDef::new("/{key}/{value}"); + + let mut path = Path::new("/%25/%2F"); + rdef.capture_match_info(&mut path); + let de = PathDeserializer::new(&path); + let vals: Vals = serde::Deserialize::deserialize(de).unwrap(); + assert_eq!(vals.key, "%"); + assert_eq!(vals.value, "/"); + } + // #[test] // fn test_extract_path_decode() { // let mut router = Router::<()>::default(); diff --git a/src/types/path.rs b/src/types/path.rs index 5d52e0e1e..c3efc22c0 100644 --- a/src/types/path.rs +++ b/src/types/path.rs @@ -285,6 +285,18 @@ mod tests { assert_eq!(res[1], "32".to_owned()); } + #[actix_rt::test] + async fn paths_decoded() { + let resource = ResourceDef::new("/{key}/{value}"); + let mut req = TestRequest::with_uri("/na%2Bme/us%2Fer%251").to_srv_request(); + resource.capture_match_info(req.match_info_mut()); + + let (req, mut pl) = req.into_parts(); + let path_items = Path::::from_request(&req, &mut pl).await.unwrap(); + assert_eq!(path_items.key, "na+me"); + assert_eq!(path_items.value, "us/er%1"); + } + #[actix_rt::test] async fn test_custom_err_handler() { let (req, mut pl) = TestRequest::with_uri("/name/user1/")