decode reserved characters when extracting path with configuration

src/de.rs

@@ -1,7 +1,10 @@
+use std::rc::Rc;
+
 use serde::de::{self, Deserializer, Error as DeError, Visitor};
 
 use httprequest::HttpRequest;
 use param::ParamsIter;
+use uri::RESERVED_QUOTER;
 
 macro_rules! unsupported_type {
     ($trait_fn:ident, $name:expr) => {
@@ -23,11 +26,20 @@ macro_rules! parse_single_value {
                     format!("wrong number of parameters: {} expected 1",
                             self.req.match_info().len()).as_str()))
             } else {
-                let v = self.req.match_info()[0].parse().map_err(
+                let v: &str = &self.req.match_info()[0];
-                    |_| de::value::Error::custom(
+                let v_parsed = if self.decode {
-                        format!("can not parse {:?} to a {}",
+                    let decoded = RESERVED_QUOTER.requote(v.as_bytes());
-                                &self.req.match_info()[0], $tp)))?;
+                    if let Some(ref value) = decoded {
-                visitor.$visit_fn(v)
+                        Rc::make_mut(&mut value.clone()).parse()
+                    } else {
+                        v.parse()
+                    }
+                } else {
+                    v.parse()
+                }.map_err(|_| de::value::Error::custom(
+                    format!("can not parse {:?} to a {}", &self.req.match_info()[0], $tp)
+                ))?;
+                visitor.$visit_fn(v_parsed)
             }
         }
     }
@@ -35,11 +47,12 @@ macro_rules! parse_single_value {
 
 pub struct PathDeserializer<'de, S: 'de> {
     req: &'de HttpRequest<S>,
+    decode: bool,
 }
 
 impl<'de, S: 'de> PathDeserializer<'de, S> {
-    pub fn new(req: &'de HttpRequest<S>) -> Self {
+    pub fn new(req: &'de HttpRequest<S>, decode: bool) -> Self {
-        PathDeserializer { req }
+        PathDeserializer { req, decode }
     }
 }
 
@@ -53,6 +66,7 @@ impl<'de, S: 'de> Deserializer<'de> for PathDeserializer<'de, S> {
         visitor.visit_map(ParamsDeserializer {
             params: self.req.match_info().iter(),
             current: None,
+            decode: self.decode,
         })
     }
 
@@ -107,6 +121,7 @@ impl<'de, S: 'de> Deserializer<'de> for PathDeserializer<'de, S> {
         } else {
             visitor.visit_seq(ParamsSeq {
                 params: self.req.match_info().iter(),
+                decode: self.decode,
             })
         }
     }
@@ -128,6 +143,7 @@ impl<'de, S: 'de> Deserializer<'de> for PathDeserializer<'de, S> {
         } else {
             visitor.visit_seq(ParamsSeq {
                 params: self.req.match_info().iter(),
+                decode: self.decode,
             })
         }
     }
@@ -141,28 +157,13 @@ impl<'de, S: 'de> Deserializer<'de> for PathDeserializer<'de, S> {
         Err(de::value::Error::custom("unsupported type: enum"))
     }
 
-    fn deserialize_str<V>(self, visitor: V) -> Result<V::Value, Self::Error>
-    where
-        V: Visitor<'de>,
-    {
-        if self.req.match_info().len() != 1 {
-            Err(de::value::Error::custom(
-                format!(
-                    "wrong number of parameters: {} expected 1",
-                    self.req.match_info().len()
-                ).as_str(),
-            ))
-        } else {
-            visitor.visit_str(&self.req.match_info()[0])
-        }
-    }
-
     fn deserialize_seq<V>(self, visitor: V) -> Result<V::Value, Self::Error>
     where
         V: Visitor<'de>,
     {
         visitor.visit_seq(ParamsSeq {
             params: self.req.match_info().iter(),
+            decode: self.decode,
         })
     }
 
@@ -184,13 +185,16 @@ impl<'de, S: 'de> Deserializer<'de> for PathDeserializer<'de, S> {
     parse_single_value!(deserialize_f32, visit_f32, "f32");
     parse_single_value!(deserialize_f64, visit_f64, "f64");
     parse_single_value!(deserialize_string, visit_string, "String");
+    parse_single_value!(deserialize_str, visit_string, "String");
     parse_single_value!(deserialize_byte_buf, visit_string, "String");
     parse_single_value!(deserialize_char, visit_char, "char");
+
 }
 
 struct ParamsDeserializer<'de> {
     params: ParamsIter<'de>,
     current: Option<(&'de str, &'de str)>,
+    decode: bool,
 }
 
 impl<'de> de::MapAccess<'de> for ParamsDeserializer<'de> {
@@ -212,7 +216,7 @@ impl<'de> de::MapAccess<'de> for ParamsDeserializer<'de> {
         V: de::DeserializeSeed<'de>,
     {
         if let Some((_, value)) = self.current.take() {
-            seed.deserialize(Value { value })
+            seed.deserialize(Value { value, decode: self.decode })
         } else {
             Err(de::value::Error::custom("unexpected item"))
         }
@@ -252,16 +256,27 @@ macro_rules! parse_value {
         fn $trait_fn<V>(self, visitor: V) -> Result<V::Value, Self::Error>
             where V: Visitor<'de>
         {
-            let v = self.value.parse().map_err(
+            let v: &str = &self.value;
-                |_| de::value::Error::custom(
+            let v_parsed = if self.decode {
-                    format!("can not parse {:?} to a {}", self.value, $tp)))?;
+                let decoded = RESERVED_QUOTER.requote(v.as_bytes());
-            visitor.$visit_fn(v)
+                if let Some(ref value) = decoded {
+                    Rc::make_mut(&mut value.clone()).parse()
+                } else {
+                    v.parse()
+                }
+            } else {
+                v.parse()
+            }.map_err(|_| de::value::Error::custom(
+                format!("can not parse {:?} to a {}", &self.value, $tp)
+            ))?;
+            visitor.$visit_fn(v_parsed)
         }
     }
 }
 
 struct Value<'de> {
     value: &'de str,
+    decode: bool,
 }
 
 impl<'de> Deserializer<'de> for Value<'de> {
@@ -377,6 +392,7 @@ impl<'de> Deserializer<'de> for Value<'de> {
 
 struct ParamsSeq<'de> {
     params: ParamsIter<'de>,
+    decode: bool,
 }
 
 impl<'de> de::SeqAccess<'de> for ParamsSeq<'de> {
@@ -387,7 +403,7 @@ impl<'de> de::SeqAccess<'de> for ParamsSeq<'de> {
         T: de::DeserializeSeed<'de>,
     {
         match self.params.next() {
-            Some(item) => Ok(Some(seed.deserialize(Value { value: item.1 })?)),
+            Some(item) => Ok(Some(seed.deserialize(Value { value: item.1, decode: self.decode })?)),
             None => Ok(None),
         }
     }

src/extractor.rs

@@ -119,7 +119,7 @@ where
         let req = req.clone();
         let req2 = req.clone();
         let err = Rc::clone(&cfg.ehandler);
-        de::Deserialize::deserialize(PathDeserializer::new(&req))
+        de::Deserialize::deserialize(PathDeserializer::new(&req, cfg.decode))
             .map_err(move |e| (*err)(e, &req2))
             .map(|inner| Path { inner })
     }
@@ -149,6 +149,7 @@ where
 /// ```
 pub struct PathConfig<S> {
     ehandler: Rc<Fn(serde_urlencoded::de::Error, &HttpRequest<S>) -> Error>,
+    decode: bool,
 }
 impl<S> PathConfig<S> {
     /// Set custom error handler
@@ -159,12 +160,20 @@ impl<S> PathConfig<S> {
         self.ehandler = Rc::new(f);
         self
     }
+
+    /// Disable decoding
+    pub fn disable_decoding(&mut self) -> &mut Self
+    {
+        self.decode = false;
+        self
+    }
 }
 
 impl<S> Default for PathConfig<S> {
     fn default() -> Self {
         PathConfig {
             ehandler: Rc::new(|e, _| ErrorNotFound(e)),
+            decode: true,
         }
     }
 }
@@ -1090,6 +1099,50 @@ mod tests {
         assert_eq!(*Path::<i8>::from_request(&req, &&PathConfig::default()).unwrap(), 32);
     }
 
+    #[test]
+    fn test_extract_path_decode() {
+        let mut router = Router::<()>::default();
+        router.register_resource(Resource::new(ResourceDef::new("/{value}/")));
+
+        let req = TestRequest::with_uri("/%25/").finish();
+        let info = router.recognize(&req, &(), 0);
+        let req = req.with_route_info(info);
+        assert_eq!(*Path::<String>::from_request(&req, &&PathConfig::default()).unwrap(), "%");
+
+        let req = TestRequest::with_uri("/%25/7/?id=test").finish();
+
+        let mut router = Router::<()>::default();
+        router.register_resource(Resource::new(ResourceDef::new("/{key}/{value}/")));
+        let info = router.recognize(&req, &(), 0);
+        let req = req.with_route_info(info);
+
+        let s = Path::<Test2>::from_request(&req, &PathConfig::default()).unwrap();
+        assert_eq!(s.key, "%");
+        assert_eq!(s.value, 7);
+
+        let s = Path::<(String, String)>::from_request(&req, &PathConfig::default()).unwrap();
+        assert_eq!(s.0, "%");
+        assert_eq!(s.1, "7");
+
+    }
+
+    #[test]
+    fn test_extract_path_no_decode() {
+        let mut router = Router::<()>::default();
+        router.register_resource(Resource::new(ResourceDef::new("/{value}/")));
+
+        let req = TestRequest::with_uri("/%25/").finish();
+        let info = router.recognize(&req, &(), 0);
+        let req = req.with_route_info(info);
+        assert_eq!(
+            *Path::<String>::from_request(
+                &req,
+                &&PathConfig::default().disable_decoding()
+            ).unwrap(),
+            "%25"
+        );
+    }
+
     #[test]
     fn test_tuple_extract() {
         let mut router = Router::<()>::default();

src/uri.rs

@@ -7,18 +7,24 @@ const GEN_DELIMS: &[u8] = b":/?#[]@";
 const SUB_DELIMS_WITHOUT_QS: &[u8] = b"!$'()*,";
 #[allow(dead_code)]
 const SUB_DELIMS: &[u8] = b"!$'()*,+?=;";
+
+// https://tools.ietf.org/html/rfc3986#section-2.2
 #[allow(dead_code)]
 const RESERVED: &[u8] = b":/?#[]@!$'()*,+?=;";
 #[allow(dead_code)]
-const UNRESERVED: &[u8] = b"abcdefghijklmnopqrstuvwxyz
+const RESERVED_PLUS_PERCENT: &[u8] = b":/?#[]@!$'()*,+?=;%";
-    ABCDEFGHIJKLMNOPQRSTUVWXYZ
+
-    1234567890
+// https://tools.ietf.org/html/rfc3986#section-2.3
-    -._~";
+#[allow(dead_code)]
+const UNRESERVED: &[u8] =
+    b"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890-._~";
+#[allow(dead_code)]
 const ALLOWED: &[u8] = b"abcdefghijklmnopqrstuvwxyz
     ABCDEFGHIJKLMNOPQRSTUVWXYZ
     1234567890
     -._~
     !$'()*,";
+#[allow(dead_code)]
 const QS: &[u8] = b"+&=;b";
 
 #[inline]
@@ -32,7 +38,8 @@ fn set_bit(array: &mut [u8], ch: u8) {
 }
 
 lazy_static! {
-    static ref DEFAULT_QUOTER: Quoter = { Quoter::new(b"@:", b"/+") };
+    static ref UNRESERVED_QUOTER: Quoter = { Quoter::new(UNRESERVED, b"") };
+    pub(crate) static ref RESERVED_QUOTER: Quoter = { Quoter::new(RESERVED_PLUS_PERCENT, b"") };
 }
 
 #[derive(Default, Clone, Debug)]
@@ -43,7 +50,7 @@ pub(crate) struct Url {
 
 impl Url {
     pub fn new(uri: Uri) -> Url {
-        let path = DEFAULT_QUOTER.requote(uri.path().as_bytes());
+        let path = UNRESERVED_QUOTER.requote(uri.path().as_bytes());
 
         Url { uri, path }
     }
@@ -74,15 +81,6 @@ impl Quoter {
         };
 
         // prepare safe table
-        for i in 0..128 {
-            if ALLOWED.contains(&i) {
-                set_bit(&mut q.safe_table, i);
-            }
-            if QS.contains(&i) {
-                set_bit(&mut q.safe_table, i);
-            }
-        }
-
         for ch in safe {
             set_bit(&mut q.safe_table, *ch)
         }
@@ -126,8 +124,12 @@ impl Quoter {
                                 idx += 1;
                                 continue;
                             }
+
+                            buf.extend_from_slice(&pct);
+                        } else {
+                            // Not ASCII, decode it
+                            buf.push(ch);
                         }
-                        buf.push(ch);
                     } else {
                         buf.extend_from_slice(&pct[..]);
                     }
@@ -172,3 +174,37 @@ fn from_hex(v: u8) -> Option<u8> {
 fn restore_ch(d1: u8, d2: u8) -> Option<u8> {
     from_hex(d1).and_then(|d1| from_hex(d2).and_then(move |d2| Some(d1 << 4 | d2)))
 }
+
+
+#[cfg(test)]
+mod tests {
+    use std::rc::Rc;
+
+    use super::*;
+
+    #[test]
+    fn decode_path() {
+        assert_eq!(UNRESERVED_QUOTER.requote(b"https://localhost:80/foo"), None);
+
+        assert_eq!(
+            Rc::try_unwrap(UNRESERVED_QUOTER.requote(
+                b"https://localhost:80/foo%25"
+            ).unwrap()).unwrap(),
+            "https://localhost:80/foo%25".to_string()
+        );
+
+        assert_eq!(
+            Rc::try_unwrap(UNRESERVED_QUOTER.requote(
+                b"http://cache-service/http%3A%2F%2Flocalhost%3A80%2Ffoo"
+            ).unwrap()).unwrap(),
+            "http://cache-service/http%3A%2F%2Flocalhost%3A80%2Ffoo".to_string()
+        );
+
+        assert_eq!(
+            Rc::try_unwrap(UNRESERVED_QUOTER.requote(
+                b"http://cache/http%3A%2F%2Flocal%3A80%2Ffile%2F%252Fvar%252Flog%0A"
+            ).unwrap()).unwrap(),
+            "http://cache/http%3A%2F%2Flocal%3A80%2Ffile%2F%252Fvar%252Flog%0A".to_string()
+        );
+    }
+}

tests/test_handlers.rs

@@ -672,6 +672,6 @@ fn test_unsafe_path_route() {
     let bytes = srv.execute(response.body()).unwrap();
     assert_eq!(
         bytes,
-        Bytes::from_static(b"success: http:%2F%2Fexample.com")
+        Bytes::from_static(b"success: http%3A%2F%2Fexample.com")
     );
 }