move quoter

actix-files/CHANGES.md

@@ -1,8 +1,8 @@
 # Changes
 
 ## Unreleased - 2021-xx-xx
-- `Files`: request URL paths with `%2F` are now rejected. [#2398]
+- The `Files` service now rejects requests with URL paths that include `%2F` (decoded: `/`). [#2398]
-- `Files`: Fixed a regression where `%25` in the URL path is not decoded to `%` in the file path. [#2398]
+- The `Files` service now correctly decodes `%25` in the URL path to `%` for the file path. [#2398]
 - Minimum supported Rust version (MSRV) is now 1.54.
 
 [#2398]: https://github.com/actix/actix-web/pull/2398

actix-files/src/error.rs

@@ -38,7 +38,7 @@ pub enum UriSegmentError {
     BadEnd(char),
 
     /// The path is not a valid UTF-8 string after doing percent decoding.
-    #[display(fmt = "The path is not a valif UTF-8 string after percent-decoding")]
+    #[display(fmt = "The path is not a valid UTF-8 string after percent-decoding")]
     NotValidUtf8,
 }
 

actix-router/src/lib.rs

@@ -8,6 +8,7 @@
 mod de;
 mod path;
 mod pattern;
+mod quoter;
 mod resource;
 mod resource_path;
 mod router;
@@ -18,9 +19,10 @@ mod url;
 pub use self::de::PathDeserializer;
 pub use self::path::Path;
 pub use self::pattern::{IntoPatterns, Patterns};
+pub use self::quoter::Quoter;
 pub use self::resource::ResourceDef;
 pub use self::resource_path::{Resource, ResourcePath};
 pub use self::router::{ResourceInfo, Router, RouterBuilder};
 
 #[cfg(feature = "http")]
-pub use self::url::{Quoter, Url};
+pub use self::url::Url;

actix-router/src/quoter.rs

@@ -0,0 +1,219 @@
+#[allow(dead_code)]
+const GEN_DELIMS: &[u8] = b":/?#[]@";
+
+#[allow(dead_code)]
+const SUB_DELIMS_WITHOUT_QS: &[u8] = b"!$'()*,";
+
+#[allow(dead_code)]
+const SUB_DELIMS: &[u8] = b"!$'()*,+?=;";
+
+#[allow(dead_code)]
+const RESERVED: &[u8] = b":/?#[]@!$'()*,+?=;";
+
+#[allow(dead_code)]
+const UNRESERVED: &[u8] = b"abcdefghijklmnopqrstuvwxyz
+    ABCDEFGHIJKLMNOPQRSTUVWXYZ
+    1234567890
+    -._~";
+
+const ALLOWED: &[u8] = b"abcdefghijklmnopqrstuvwxyz
+    ABCDEFGHIJKLMNOPQRSTUVWXYZ
+    1234567890
+    -._~
+    !$'()*,";
+
+const QS: &[u8] = b"+&=;b";
+
+/// A quoter
+pub struct Quoter {
+    /// Simple bit-map of safe values in the 0-127 ASCII range.
+    safe_table: [u8; 16],
+
+    /// Simple bit-map of protected values in the 0-127 ASCII range.
+    protected_table: [u8; 16],
+}
+
+impl Quoter {
+    pub fn new(safe: &[u8], protected: &[u8]) -> Quoter {
+        let mut quoter = Quoter {
+            safe_table: [0; 16],
+            protected_table: [0; 16],
+        };
+
+        // prepare safe table
+        for ch in 0..128 {
+            if ALLOWED.contains(&ch) {
+                set_bit(&mut quoter.safe_table, ch);
+            }
+
+            if QS.contains(&ch) {
+                set_bit(&mut quoter.safe_table, ch);
+            }
+        }
+
+        for &ch in safe {
+            set_bit(&mut quoter.safe_table, ch)
+        }
+
+        // prepare protected table
+        for &ch in protected {
+            set_bit(&mut quoter.safe_table, ch);
+            set_bit(&mut quoter.protected_table, ch);
+        }
+
+        quoter
+    }
+
+    /// Re-quotes... ?
+    ///
+    /// Returns `None` when no modification to the original string was required.
+    pub fn requote(&self, val: &[u8]) -> Option<String> {
+        let mut has_pct = 0;
+        let mut pct = [b'%', 0, 0];
+        let mut idx = 0;
+        let mut cloned: Option<Vec<u8>> = None;
+
+        let len = val.len();
+
+        while idx < len {
+            let ch = val[idx];
+
+            if has_pct != 0 {
+                pct[has_pct] = val[idx];
+                has_pct += 1;
+
+                if has_pct == 3 {
+                    has_pct = 0;
+                    let buf = cloned.as_mut().unwrap();
+
+                    if let Some(ch) = hex_pair_to_char(pct[1], pct[2]) {
+                        if ch < 128 {
+                            if bit_at(&self.protected_table, ch) {
+                                buf.extend_from_slice(&pct);
+                                idx += 1;
+                                continue;
+                            }
+
+                            if bit_at(&self.safe_table, ch) {
+                                buf.push(ch);
+                                idx += 1;
+                                continue;
+                            }
+                        }
+
+                        buf.push(ch);
+                    } else {
+                        buf.extend_from_slice(&pct[..]);
+                    }
+                }
+            } else if ch == b'%' {
+                has_pct = 1;
+
+                if cloned.is_none() {
+                    let mut c = Vec::with_capacity(len);
+                    c.extend_from_slice(&val[..idx]);
+                    cloned = Some(c);
+                }
+            } else if let Some(ref mut cloned) = cloned {
+                cloned.push(ch)
+            }
+
+            idx += 1;
+        }
+
+        cloned.map(|data| String::from_utf8_lossy(&data).into_owned())
+    }
+}
+
+/// Converts an ASCII character in the hex-encoded set (`0-9`, `A-F`, `a-f`) to its integer
+/// representation from `0x0`–`0xF`.
+///
+/// - `0x30 ('0') => 0x0`
+/// - `0x39 ('9') => 0x9`
+/// - `0x41 ('a') => 0xA`
+/// - `0x61 ('A') => 0xA`
+/// - `0x46 ('f') => 0xF`
+/// - `0x66 ('F') => 0xF`
+fn from_ascii_hex(v: u8) -> Option<u8> {
+    match v {
+        b'0'..=b'9' => Some(v - 0x30),      // ord('0') == 0x30
+        b'A'..=b'F' => Some(v - 0x41 + 10), // ord('A') == 0x41
+        b'a'..=b'f' => Some(v - 0x61 + 10), // ord('a') == 0x61
+        _ => None,
+    }
+}
+
+/// Decode a ASCII hex-encoded pair to an integer.
+///
+/// Returns `None` if either portion of the decoded pair does not evaluate to a valid hex value.
+///
+/// - `0x33 ('3'), 0x30 ('0') => 0x30 ('0')`
+/// - `0x34 ('4'), 0x31 ('1') => 0x41 ('A')`
+/// - `0x36 ('6'), 0x31 ('1') => 0x61 ('a')`
+fn hex_pair_to_char(d1: u8, d2: u8) -> Option<u8> {
+    let (d_high, d_low) = (from_ascii_hex(d1)?, from_ascii_hex(d2)?);
+
+    // left shift high nibble by 4 bits
+    Some(d_high << 4 | d_low)
+}
+
+/// Sets bit in given bit-map to 1=true.
+///
+/// # Panics
+/// Panics if `ch` index is out of bounds.
+fn set_bit(array: &mut [u8], ch: u8) {
+    array[(ch >> 3) as usize] |= 0b1 << (ch & 0b111)
+}
+
+/// Returns true if bit to true in given bit-map.
+///
+/// # Panics
+/// Panics if `ch` index is out of bounds.
+fn bit_at(array: &[u8], ch: u8) -> bool {
+    array[(ch >> 3) as usize] & (0b1 << (ch & 0b111)) != 0
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn hex_encoding() {
+        let hex = b"0123456789abcdefABCDEF";
+
+        for i in 0..256 {
+            let c = i as u8;
+            if hex.contains(&c) {
+                assert!(from_ascii_hex(c).is_some())
+            } else {
+                assert!(from_ascii_hex(c).is_none())
+            }
+        }
+
+        let expected = [
+            0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 10, 11, 12, 13, 14, 15,
+        ];
+        for i in 0..hex.len() {
+            assert_eq!(from_ascii_hex(hex[i]).unwrap(), expected[i]);
+        }
+    }
+
+    #[test]
+    fn custom_quoter() {
+        let q = Quoter::new(b"", b"+");
+        assert_eq!(q.requote(b"/a%25c").unwrap(), "/a%c");
+        assert_eq!(q.requote(b"/a%2Bc").unwrap(), "/a%2Bc");
+
+        let q = Quoter::new(b"%+", b"/");
+        assert_eq!(q.requote(b"/a%25b%2Bc").unwrap(), "/a%b+c");
+        assert_eq!(q.requote(b"/a%2fb").unwrap(), "/a%2fb");
+        assert_eq!(q.requote(b"/a%2Fb").unwrap(), "/a%2Fb");
+        assert_eq!(q.requote(b"/a%0Ab").unwrap(), "/a\nb");
+    }
+
+    #[test]
+    fn quoter_no_modification() {
+        let q = Quoter::new(b"", b"");
+        assert_eq!(q.requote(b"/abc/../efg"), None);
+    }
+}

actix-router/src/url.rs

@@ -1,30 +1,6 @@
 use crate::ResourcePath;
 
-#[allow(dead_code)]
+use crate::Quoter;
-const GEN_DELIMS: &[u8] = b":/?#[]@";
-
-#[allow(dead_code)]
-const SUB_DELIMS_WITHOUT_QS: &[u8] = b"!$'()*,";
-
-#[allow(dead_code)]
-const SUB_DELIMS: &[u8] = b"!$'()*,+?=;";
-
-#[allow(dead_code)]
-const RESERVED: &[u8] = b":/?#[]@!$'()*,+?=;";
-
-#[allow(dead_code)]
-const UNRESERVED: &[u8] = b"abcdefghijklmnopqrstuvwxyz
-    ABCDEFGHIJKLMNOPQRSTUVWXYZ
-    1234567890
-    -._~";
-
-const ALLOWED: &[u8] = b"abcdefghijklmnopqrstuvwxyz
-    ABCDEFGHIJKLMNOPQRSTUVWXYZ
-    1234567890
-    -._~
-    !$'()*,";
-
-const QS: &[u8] = b"+&=;b";
 
 thread_local! {
     static DEFAULT_QUOTER: Quoter = Quoter::new(b"@:", b"%/+");
@@ -44,18 +20,20 @@ impl Url {
     }
 
     #[inline]
-    pub fn with_quoter(uri: http::Uri, quoter: &Quoter) -> Url {
+    pub fn new_with_quoter(uri: http::Uri, quoter: &Quoter) -> Url {
         Url {
             path: quoter.requote(uri.path().as_bytes()),
             uri,
         }
     }
 
+    /// Returns URI.
     #[inline]
     pub fn uri(&self) -> &http::Uri {
         &self.uri
     }
 
+    /// Returns path.
     #[inline]
     pub fn path(&self) -> &str {
         match self.path {
@@ -84,155 +62,6 @@ impl ResourcePath for Url {
     }
 }
 
-/// A quoter
-pub struct Quoter {
-    /// Simple bit-map of safe values in the 0-127 ASCII range.
-    safe_table: [u8; 16],
-
-    /// Simple bit-map of protected values in the 0-127 ASCII range.
-    protected_table: [u8; 16],
-}
-
-impl Quoter {
-    pub fn new(safe: &[u8], protected: &[u8]) -> Quoter {
-        let mut quoter = Quoter {
-            safe_table: [0; 16],
-            protected_table: [0; 16],
-        };
-
-        // prepare safe table
-        for ch in 0..128 {
-            if ALLOWED.contains(&ch) {
-                set_bit(&mut quoter.safe_table, ch);
-            }
-
-            if QS.contains(&ch) {
-                set_bit(&mut quoter.safe_table, ch);
-            }
-        }
-
-        for &ch in safe {
-            set_bit(&mut quoter.safe_table, ch)
-        }
-
-        // prepare protected table
-        for &ch in protected {
-            set_bit(&mut quoter.safe_table, ch);
-            set_bit(&mut quoter.protected_table, ch);
-        }
-
-        quoter
-    }
-
-    /// Re-quotes... ?
-    ///
-    /// Returns `None` when no modification to the original string was required.
-    pub fn requote(&self, val: &[u8]) -> Option<String> {
-        let mut has_pct = 0;
-        let mut pct = [b'%', 0, 0];
-        let mut idx = 0;
-        let mut cloned: Option<Vec<u8>> = None;
-
-        let len = val.len();
-
-        while idx < len {
-            let ch = val[idx];
-
-            if has_pct != 0 {
-                pct[has_pct] = val[idx];
-                has_pct += 1;
-
-                if has_pct == 3 {
-                    has_pct = 0;
-                    let buf = cloned.as_mut().unwrap();
-
-                    if let Some(ch) = hex_pair_to_char(pct[1], pct[2]) {
-                        if ch < 128 {
-                            if bit_at(&self.protected_table, ch) {
-                                buf.extend_from_slice(&pct);
-                                idx += 1;
-                                continue;
-                            }
-
-                            if bit_at(&self.safe_table, ch) {
-                                buf.push(ch);
-                                idx += 1;
-                                continue;
-                            }
-                        }
-
-                        buf.push(ch);
-                    } else {
-                        buf.extend_from_slice(&pct[..]);
-                    }
-                }
-            } else if ch == b'%' {
-                has_pct = 1;
-
-                if cloned.is_none() {
-                    let mut c = Vec::with_capacity(len);
-                    c.extend_from_slice(&val[..idx]);
-                    cloned = Some(c);
-                }
-            } else if let Some(ref mut cloned) = cloned {
-                cloned.push(ch)
-            }
-
-            idx += 1;
-        }
-
-        cloned.map(|data| String::from_utf8_lossy(&data).into_owned())
-    }
-}
-
-/// Converts an ASCII character in the hex-encoded set (`0-9`, `A-F`, `a-f`) to its integer
-/// representation from `0x0`–`0xF`.
-///
-/// - `0x30 ('0') => 0x0`
-/// - `0x39 ('9') => 0x9`
-/// - `0x41 ('a') => 0xA`
-/// - `0x61 ('A') => 0xA`
-/// - `0x46 ('f') => 0xF`
-/// - `0x66 ('F') => 0xF`
-fn from_ascii_hex(v: u8) -> Option<u8> {
-    match v {
-        b'0'..=b'9' => Some(v - 0x30),      // ord('0') == 0x30
-        b'A'..=b'F' => Some(v - 0x41 + 10), // ord('A') == 0x41
-        b'a'..=b'f' => Some(v - 0x61 + 10), // ord('a') == 0x61
-        _ => None,
-    }
-}
-
-/// Decode a ASCII hex-encoded pair to an integer.
-///
-/// Returns `None` if either portion of the decoded pair does not evaluate to a valid hex value.
-///
-/// - `0x33 ('3'), 0x30 ('0') => 0x30 ('0')`
-/// - `0x34 ('4'), 0x31 ('1') => 0x41 ('A')`
-/// - `0x36 ('6'), 0x31 ('1') => 0x61 ('a')`
-fn hex_pair_to_char(d1: u8, d2: u8) -> Option<u8> {
-    let (d_high, d_low) = (from_ascii_hex(d1)?, from_ascii_hex(d2)?);
-
-    // left shift high nibble by 4 bits
-    Some(d_high << 4 | d_low)
-}
-
-/// Sets bit in given bit-map to 1=true.
-///
-/// # Panics
-/// Panics if `ch` index is out of bounds.
-fn set_bit(array: &mut [u8], ch: u8) {
-    array[(ch >> 3) as usize] |= 0b1 << (ch & 0b111)
-}
-
-/// Returns true if bit to true in given bit-map.
-///
-/// # Panics
-/// Panics if `ch` index is out of bounds.
-fn bit_at(array: &[u8], ch: u8) -> bool {
-    array[(ch >> 3) as usize] & (0b1 << (ch & 0b111)) != 0
-}
-
 #[cfg(test)]
 mod tests {
     use http::Uri;
@@ -308,44 +137,4 @@ mod tests {
         // We should always get a valid utf8 string
         assert!(String::from_utf8(path.path().as_bytes().to_owned()).is_ok());
     }
-
-    #[test]
-    fn hex_encoding() {
-        let hex = b"0123456789abcdefABCDEF";
-
-        for i in 0..256 {
-            let c = i as u8;
-            if hex.contains(&c) {
-                assert!(from_ascii_hex(c).is_some())
-            } else {
-                assert!(from_ascii_hex(c).is_none())
-            }
-        }
-
-        let expected = [
-            0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 10, 11, 12, 13, 14, 15,
-        ];
-        for i in 0..hex.len() {
-            assert_eq!(from_ascii_hex(hex[i]).unwrap(), expected[i]);
-        }
-    }
-
-    #[test]
-    fn custom_quoter() {
-        let q = Quoter::new(b"", b"+");
-        assert_eq!(q.requote(b"/a%25c").unwrap(), "/a%c");
-        assert_eq!(q.requote(b"/a%2Bc").unwrap(), "/a%2Bc");
-
-        let q = Quoter::new(b"%+", b"/");
-        assert_eq!(q.requote(b"/a%25b%2Bc").unwrap(), "/a%b+c");
-        assert_eq!(q.requote(b"/a%2fb").unwrap(), "/a%2fb");
-        assert_eq!(q.requote(b"/a%2Fb").unwrap(), "/a%2Fb");
-        assert_eq!(q.requote(b"/a%0Ab").unwrap(), "/a\nb");
-    }
-
-    #[test]
-    fn quoter_no_modification() {
-        let q = Quoter::new(b"", b"");
-        assert_eq!(q.requote(b"/abc/../efg"), None);
-    }
 }

src/request.rs

@@ -122,7 +122,7 @@ impl HttpRequest {
 
     /// Returns a reference to the URL parameters container.
     ///
-    /// A url parameter is specified in the form `{identifier}`, where the identifier can be used
+    /// A URL parameter is specified in the form `{identifier}`, where the identifier can be used
     /// later in a request handler to access the matched value for that parameter.
     ///
     /// # Percent Encoding and URL Parameters

src/types/path.rs

@@ -138,6 +138,7 @@ where
 /// enum Folder {
 ///     #[serde(rename = "inbox")]
 ///     Inbox,
+///
 ///     #[serde(rename = "outbox")]
 ///     Outbox,
 /// }
@@ -147,7 +148,6 @@ where
 ///     format!("Selected folder: {:?}!", folder)
 /// }
 ///
-/// fn main() {
 /// let app = App::new().service(
 ///     web::resource("/messages/{folder}")
 ///         .app_data(PathConfig::default().error_handler(|err, req| {
@@ -159,7 +159,6 @@ where
 ///         }))
 ///         .route(web::post().to(index)),
 /// );
-/// }
 /// ```
 #[derive(Clone, Default)]
 pub struct PathConfig {
@@ -167,7 +166,7 @@ pub struct PathConfig {
 }
 
 impl PathConfig {
-    /// Set custom error handler
+    /// Set custom error handler.
     pub fn error_handler<F>(mut self, f: F) -> Self
     where
         F: Fn(PathError, &HttpRequest) -> Error + Send + Sync + 'static,

tests/weird_poll.rs

@@ -0,0 +1,30 @@
+//! Regression test for https://github.com/actix/actix-web/issues/1321
+
+// use actix_http::body::{BodyStream, MessageBody};
+// use bytes::Bytes;
+// use futures_channel::oneshot;
+// use futures_util::{
+//     stream::once,
+//     task::{noop_waker, Context},
+// };
+
+// #[test]
+// fn weird_poll() {
+//     let (sender, receiver) = oneshot::channel();
+//     let mut body_stream = Ok(BodyStream::new(once(async {
+//         let x = Box::new(0);
+//         let y = &x;
+//         receiver.await.unwrap();
+//         let _z = **y;
+//         Ok::<_, ()>(Bytes::new())
+//     })));
+
+//     let waker = noop_waker();
+//     let mut cx = Context::from_waker(&waker);
+
+//     let _ = body_stream.as_mut().unwrap().poll_next(&mut cx);
+//     sender.send(()).unwrap();
+//     let _ = std::mem::replace(&mut body_stream, Err([0; 32]))
+//         .unwrap()
+//         .poll_next(&mut cx);
+// }