add custom request path quoter #182

CHANGES.md

@@ -1,5 +1,9 @@
 # Changes
 
+## 0.5.3 (2018-04-xx)
+
+* Impossible to quote slashes in path parameters #182
+
 
 ## 0.5.2 (2018-04-16)
 

src/application.rs

@@ -418,6 +418,8 @@ where
     /// `/app/test` would match, but the path `/application` would
     /// not.
     ///
+    /// Path tail is available as `tail` parameter in request's match_dict.
+    ///
     /// ```rust
     /// # extern crate actix_web;
     /// use actix_web::{http, App, HttpRequest, HttpResponse};

src/error.rs

@@ -888,7 +888,9 @@ mod tests {
     #[test]
     fn test_internal_error() {
         let err = InternalError::from_response(
-            ExpectError::Encoding, HttpResponse::Ok().into());
+            ExpectError::Encoding,
+            HttpResponse::Ok().into(),
+        );
         let resp: HttpResponse = err.error_response();
         assert_eq!(resp.status(), StatusCode::OK);
     }

src/fs.rs

@@ -15,7 +15,6 @@ use bytes::{BufMut, Bytes, BytesMut};
 use futures::{Async, Future, Poll, Stream};
 use futures_cpupool::{CpuFuture, CpuPool};
 use mime_guess::get_mime_type;
-use percent_encoding::percent_decode;
 
 use error::Error;
 use handler::{Handler, Reply, Responder, RouteHandler, WrapHandler};
@@ -457,7 +456,7 @@ lazy_static! {
                     error!("Can not parse ACTIX_FS_POOL value");
                     20
                 }
-            },
+            }
             Err(_) => 20,
         };
         Mutex::new(CpuPool::new(default))
@@ -477,7 +476,8 @@ impl<S: 'static> StaticFiles<S> {
         StaticFiles::with_pool(dir, pool)
     }
 
-    /// Create new `StaticFiles` instance for specified base directory and `CpuPool`.
+    /// Create new `StaticFiles` instance for specified base directory and
+    /// `CpuPool`.
     pub fn with_pool<T: Into<PathBuf>>(dir: T, pool: CpuPool) -> StaticFiles<S> {
         let dir = dir.into();
 
@@ -543,8 +543,7 @@ impl<S: 'static> Handler<S> for StaticFiles<S> {
         } else {
             let relpath = match req.match_info()
                 .get("tail")
-                .map(|tail| percent_decode(tail.as_bytes()).decode_utf8().unwrap())
-                .map(|tail| PathBuf::from_param(tail.as_ref()))
+                .map(|tail| PathBuf::from_param(tail))
             {
                 Some(Ok(path)) => path,
                 _ => return Ok(self.default.handle(req)),

src/httprequest.rs

@@ -6,8 +6,6 @@ use futures::future::{result, FutureResult};
 use futures::{Async, Poll, Stream};
 use futures_cpupool::CpuPool;
 use http::{header, Extensions, HeaderMap, Method, StatusCode, Uri, Version};
-use percent_encoding::percent_decode;
-use std::borrow::Cow;
 use std::net::SocketAddr;
 use std::rc::Rc;
 use std::{cmp, fmt, io, mem, str};
@@ -24,11 +22,12 @@ use param::Params;
 use payload::Payload;
 use router::{Resource, Router};
 use server::helpers::SharedHttpInnerMessage;
+use uri::Url as InnerUrl;
 
 pub struct HttpInnerMessage {
     pub version: Version,
     pub method: Method,
-    pub uri: Uri,
+    pub(crate) url: InnerUrl,
     pub headers: HeaderMap,
     pub extensions: Extensions,
     pub params: Params<'static>,
@@ -51,7 +50,7 @@ impl Default for HttpInnerMessage {
     fn default() -> HttpInnerMessage {
         HttpInnerMessage {
             method: Method::GET,
-            uri: Uri::default(),
+            url: InnerUrl::default(),
             version: Version::HTTP_11,
             headers: HeaderMap::with_capacity(16),
             params: Params::new(),
@@ -116,10 +115,11 @@ impl HttpRequest<()> {
         method: Method, uri: Uri, version: Version, headers: HeaderMap,
         payload: Option<Payload>,
     ) -> HttpRequest {
+        let url = InnerUrl::new(uri);
         HttpRequest(
             SharedHttpInnerMessage::from_message(HttpInnerMessage {
                 method,
-                uri,
+                url,
                 version,
                 headers,
                 payload,
@@ -241,15 +241,17 @@ impl<S> HttpRequest<S> {
     /// Read the Request Uri.
     #[inline]
     pub fn uri(&self) -> &Uri {
-        &self.as_ref().uri
+        self.as_ref().url.uri()
     }
 
+    #[doc(hidden)]
+    #[deprecated(since = "0.5.3")]
     /// Returns mutable the Request Uri.
     ///
     /// This might be useful for middlewares, e.g. path normalization.
     #[inline]
     pub fn uri_mut(&mut self) -> &mut Uri {
-        &mut self.as_mut().uri
+        self.as_mut().url.uri_mut()
     }
 
     /// Read the Request method.
@@ -275,15 +277,7 @@ impl<S> HttpRequest<S> {
     /// The target path of this Request.
     #[inline]
     pub fn path(&self) -> &str {
-        self.uri().path()
-    }
-
-    /// Percent decoded path of this Request.
-    #[inline]
-    pub fn path_decoded(&self) -> Cow<str> {
-        percent_decode(self.uri().path().as_bytes())
-            .decode_utf8()
-            .unwrap()
+        self.as_ref().url.path()
     }
 
     /// Get *ConnectionInfo* for correct request.
@@ -578,7 +572,7 @@ impl<S> fmt::Debug for HttpRequest<S> {
             "\nHttpRequest {:?} {}:{}",
             self.as_ref().version,
             self.as_ref().method,
-            self.path_decoded()
+            self.path()
         );
         if !self.query_string().is_empty() {
             let _ = writeln!(f, "  query: ?{:?}", self.query_string());

src/json.rs

@@ -2,8 +2,8 @@ use bytes::{Bytes, BytesMut};
 use futures::{Future, Poll, Stream};
 use http::header::CONTENT_LENGTH;
 use std::fmt;
-use std::rc::Rc;
 use std::ops::{Deref, DerefMut};
+use std::rc::Rc;
 
 use mime;
 use serde::Serialize;
@@ -193,7 +193,7 @@ impl<S> JsonConfig<S> {
     /// Set custom error handler
     pub fn error_handler<F>(&mut self, f: F) -> &mut Self
     where
-        F: Fn(JsonPayloadError, HttpRequest<S>) -> Error + 'static
+        F: Fn(JsonPayloadError, HttpRequest<S>) -> Error + 'static,
     {
         self.ehandler = Rc::new(f);
         self
@@ -202,8 +202,10 @@ impl<S> JsonConfig<S> {
 
 impl<S> Default for JsonConfig<S> {
     fn default() -> Self {
-        JsonConfig { limit: 262_144,
-                     ehandler: Rc::new(|e, _| e.into()) }
+        JsonConfig {
+            limit: 262_144,
+            ehandler: Rc::new(|e, _| e.into()),
+        }
     }
 }
 

src/lib.rs

@@ -147,6 +147,7 @@ mod pipeline;
 mod resource;
 mod route;
 mod router;
+mod uri;
 mod with;
 
 pub mod client;

src/router.rs

@@ -3,7 +3,6 @@ use std::hash::{Hash, Hasher};
 use std::mem;
 use std::rc::Rc;
 
-use percent_encoding::percent_decode;
 use regex::{escape, Regex};
 
 use error::UrlGenerationError;
@@ -82,12 +81,9 @@ impl Router {
         }
         let path: &str = unsafe { mem::transmute(&req.path()[self.0.prefix_len..]) };
         let route_path = if path.is_empty() { "/" } else { path };
-        let p = percent_decode(route_path.as_bytes())
-            .decode_utf8()
-            .unwrap();
 
         for (idx, pattern) in self.0.patterns.iter().enumerate() {
-            if pattern.match_with_params(p.as_ref(), req.match_info_mut()) {
+            if pattern.match_with_params(route_path, req.match_info_mut()) {
                 req.set_resource(idx);
                 return Some(idx);
             }

src/server/h1.rs

@@ -19,6 +19,7 @@ use httprequest::HttpRequest;
 use httpresponse::HttpResponse;
 use payload::{Payload, PayloadStatus, PayloadWriter};
 use pipeline::Pipeline;
+use uri::Url;
 
 use super::encoding::PayloadType;
 use super::h1writer::H1Writer;
@@ -527,7 +528,7 @@ impl Reader {
                     httparse::Status::Complete(len) => {
                         let method = Method::from_bytes(req.method.unwrap().as_bytes())
                             .map_err(|_| ParseError::Method)?;
-                        let path = Uri::try_from(req.path.unwrap())?;
+                        let path = Url::new(Uri::try_from(req.path.unwrap())?);
                         let version = if req.version.unwrap() == 1 {
                             Version::HTTP_11
                         } else {
@@ -563,7 +564,7 @@ impl Reader {
                     }
                 }
 
-                msg_mut.uri = path;
+                msg_mut.url = path;
                 msg_mut.method = method;
                 msg_mut.version = version;
             }

src/server/h2.rs

@@ -22,6 +22,7 @@ use httprequest::HttpRequest;
 use httpresponse::HttpResponse;
 use payload::{Payload, PayloadStatus, PayloadWriter};
 use pipeline::Pipeline;
+use uri::Url;
 
 use super::encoding::PayloadType;
 use super::h2writer::H2Writer;
@@ -304,7 +305,7 @@ impl<H: 'static> Entry<H> {
         let (psender, payload) = Payload::new(false);
 
         let msg = settings.get_http_message();
-        msg.get_mut().uri = parts.uri;
+        msg.get_mut().url = Url::new(parts.uri);
         msg.get_mut().method = parts.method;
         msg.get_mut().version = parts.version;
         msg.get_mut().headers = parts.headers;

src/uri.rs

@@ -0,0 +1,175 @@
+use http::Uri;
+
+#[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";
+
+#[inline]
+fn bit_at(array: &[u8], ch: u8) -> bool {
+    array[(ch >> 3) as usize] & (1 << (ch & 7)) != 0
+}
+
+#[inline]
+fn set_bit(array: &mut [u8], ch: u8) {
+    array[(ch >> 3) as usize] |= 1 << (ch & 7)
+}
+
+lazy_static! {
+    static ref DEFAULT_QUOTER: Quoter = { Quoter::new(b"@:", b"/+") };
+}
+
+#[derive(Default)]
+pub(crate) struct Url {
+    uri: Uri,
+    path: Option<String>,
+}
+
+impl Url {
+    pub fn new(uri: Uri) -> Url {
+        let path = DEFAULT_QUOTER.requote(uri.path().as_bytes());
+
+        Url { uri, path }
+    }
+
+    pub fn uri(&self) -> &Uri {
+        &self.uri
+    }
+
+    pub fn uri_mut(&mut self) -> &mut Uri {
+        &mut self.uri
+    }
+
+    pub fn path(&self) -> &str {
+        if let Some(ref s) = self.path {
+            s
+        } else {
+            self.uri.path()
+        }
+    }
+}
+
+pub(crate) struct Quoter {
+    safe_table: [u8; 16],
+    protected_table: [u8; 16],
+}
+
+impl Quoter {
+    pub fn new(safe: &[u8], protected: &[u8]) -> Quoter {
+        let mut q = Quoter {
+            safe_table: [0; 16],
+            protected_table: [0; 16],
+        };
+
+        // 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)
+        }
+
+        // prepare protected table
+        for ch in protected {
+            set_bit(&mut q.safe_table, *ch);
+            set_bit(&mut q.protected_table, *ch);
+        }
+
+        q
+    }
+
+    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) = restore_ch(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;
+        }
+
+        if let Some(data) = cloned {
+            Some(unsafe { String::from_utf8_unchecked(data) })
+        } else {
+            None
+        }
+    }
+}
+
+#[inline]
+fn from_hex(v: u8) -> Option<u8> {
+    if v >= b'0' && v <= b'9' {
+        Some(v - 0x30) // ord('0') == 0x30
+    } else if v >= b'A' && v <= b'F' {
+        Some(v - 0x41 + 10) // ord('A') == 0x41
+    } else if v > b'a' && v <= b'f' {
+        Some(v - 0x61 + 10) // ord('a') == 0x61
+    } else {
+        None
+    }
+}
+
+#[inline]
+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)))
+}

tests/test_handlers.rs

@@ -148,3 +148,27 @@ fn test_non_ascii_route() {
     let bytes = srv.execute(response.body()).unwrap();
     assert_eq!(bytes, Bytes::from_static(b"success"));
 }
+
+#[test]
+fn test_unsafe_path_route() {
+    let mut srv = test::TestServer::new(|app| {
+        app.resource("/test/{url}", |r| {
+            r.f(|r| format!("success: {}", &r.match_info()["url"]))
+        });
+    });
+
+    // client request
+    let request = srv.get()
+        .uri(srv.url("/test/http%3A%2F%2Fexample.com"))
+        .finish()
+        .unwrap();
+    let response = srv.execute(request.send()).unwrap();
+    assert!(response.status().is_success());
+
+    // read response
+    let bytes = srv.execute(response.body()).unwrap();
+    assert_eq!(
+        bytes,
+        Bytes::from_static(b"success: http:%2F%2Fexample.com")
+    );
+}