fully percent decode path segments when capturing (#2566)

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.

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<V>(self, visitor: V) -> Result<V::Value, Self::Error>
         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<V>(self, visitor: V) -> Result<V::Value, Self::Error>
+        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<T>,
 }
@@ -172,23 +194,6 @@ impl<'de, T: ResourcePath + 'de> Deserializer<'de> for PathDeserializer<'de, T>
         }
     }
 
-    fn deserialize_str<V>(self, visitor: V) -> Result<V::Value, Self::Error>
-    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<V>(self, visitor: V) -> Result<V::Value, Self::Error>
     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<V>(self, visitor: V) -> Result<V::Value, Self::Error>
-        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();

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::<MyStruct>::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/")