diff --git a/CHANGES.md b/CHANGES.md index d81744762..67a8694cc 100644 --- a/CHANGES.md +++ b/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) diff --git a/src/application.rs b/src/application.rs index f78335f1f..411b738e7 100644 --- a/src/application.rs +++ b/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}; diff --git a/src/error.rs b/src/error.rs index aafd9b4b7..da56c35c9 100644 --- a/src/error.rs +++ b/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); } diff --git a/src/fs.rs b/src/fs.rs index e865a6dd3..ce0e42d57 100644 --- a/src/fs.rs +++ b/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 StaticFiles { 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>(dir: T, pool: CpuPool) -> StaticFiles { let dir = dir.into(); @@ -543,8 +543,7 @@ impl Handler for StaticFiles { } 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)), diff --git a/src/httprequest.rs b/src/httprequest.rs index d33346025..62efa4834 100644 --- a/src/httprequest.rs +++ b/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, ) -> HttpRequest { + let url = InnerUrl::new(uri); HttpRequest( SharedHttpInnerMessage::from_message(HttpInnerMessage { method, - uri, + url, version, headers, payload, @@ -241,15 +241,17 @@ impl HttpRequest { /// 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 HttpRequest { /// 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 { - 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 fmt::Debug for HttpRequest { "\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()); diff --git a/src/json.rs b/src/json.rs index 73128b975..96ac415f1 100644 --- a/src/json.rs +++ b/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 JsonConfig { /// Set custom error handler pub fn error_handler(&mut self, f: F) -> &mut Self where - F: Fn(JsonPayloadError, HttpRequest) -> Error + 'static + F: Fn(JsonPayloadError, HttpRequest) -> Error + 'static, { self.ehandler = Rc::new(f); self @@ -202,8 +202,10 @@ impl JsonConfig { impl Default for JsonConfig { fn default() -> Self { - JsonConfig { limit: 262_144, - ehandler: Rc::new(|e, _| e.into()) } + JsonConfig { + limit: 262_144, + ehandler: Rc::new(|e, _| e.into()), + } } } diff --git a/src/lib.rs b/src/lib.rs index 1e32dcc7f..13be3ef41 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -147,6 +147,7 @@ mod pipeline; mod resource; mod route; mod router; +mod uri; mod with; pub mod client; diff --git a/src/router.rs b/src/router.rs index 74225a1a6..4257d739e 100644 --- a/src/router.rs +++ b/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); } diff --git a/src/server/h1.rs b/src/server/h1.rs index aa27d0294..c60762b6e 100644 --- a/src/server/h1.rs +++ b/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; } diff --git a/src/server/h2.rs b/src/server/h2.rs index 08a97626c..a5ac2cfca 100644 --- a/src/server/h2.rs +++ b/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 Entry { 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; diff --git a/src/uri.rs b/src/uri.rs new file mode 100644 index 000000000..d30fe5cb4 --- /dev/null +++ b/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, +} + +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 { + let mut has_pct = 0; + let mut pct = [b'%', 0, 0]; + let mut idx = 0; + let mut cloned: Option> = 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 { + 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 { + from_hex(d1).and_then(|d1| from_hex(d2).and_then(move |d2| Some(d1 << 4 | d2))) +} diff --git a/tests/test_handlers.rs b/tests/test_handlers.rs index 12cf9709c..7a9abe974 100644 --- a/tests/test_handlers.rs +++ b/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") + ); +}