From d8f27e95a6a3f11a8dd6879779c537511e12bc40 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Sat, 2 Dec 2017 10:17:15 -0800 Subject: [PATCH] added FromParam trait for path segment conversions, FramParam impl for PathBuf --- guide/src/qs_5.md | 66 ++++++++++- src/dev.rs | 2 +- src/error.rs | 59 +++++++--- src/h1.rs | 2 +- src/middlewares/session.rs | 4 +- src/recognizer.rs | 225 ++++++++++++++++++++++++++++--------- 6 files changed, 279 insertions(+), 79 deletions(-) diff --git a/guide/src/qs_5.md b/guide/src/qs_5.md index 157b55c57..adf96c903 100644 --- a/guide/src/qs_5.md +++ b/guide/src/qs_5.md @@ -60,17 +60,16 @@ Resource may have *variable path*also. For instance, a resource with the path '/a/{name}/c' would match all incoming requests with paths such as '/a/b/c', '/a/1/c', and '/a/etc/c'. -A *variable part*is specified in the form {identifier}, where the identifier can be +A *variable part* is specified in the form {identifier}, where the identifier can be used later in a request handler to access the matched value for that part. This is done by looking up the identifier in the `HttpRequest.match_info` object: - ```rust extern crate actix; use actix_web::*; fn index(req: Httprequest) -> String { - format!("Hello, {}", req.match_info.get('name').unwrap()) + format!("Hello, {}", req.match_info["name"]) } fn main() { @@ -92,8 +91,31 @@ fn main() { } ``` -To match path tail, `{tail:*}` pattern could be used. Tail pattern has to be last -segment in path otherwise it panics. +Any matched parameter can be deserialized into specific type if this type +implements `FromParam` trait. For example most of standard integer types +implements `FromParam` trait. i.e.: + +```rust +extern crate actix; +use actix_web::*; + +fn index(req: Httprequest) -> String { + let v1: u8 = req.match_info().query("v1")?; + let v2: u8 = req.match_info().query("v2")?; + format!("Values {} {}", v1, v2) +} + +fn main() { + Application::default("/") + .resource(r"/a/{v1}/{v2}/", |r| r.get(index)) + .finish(); +} +``` + +For this example for path '/a/1/2/', values v1 and v2 will resolve to "1" and "2". + +To match path tail, `{tail:*}` pattern could be used. Tail pattern must to be last +component of a path, any text after tail pattern will result in panic. ```rust,ignore fn main() { @@ -105,3 +127,37 @@ fn main() { Above example would match all incoming requests with path such as '/test/b/c', '/test/index.html', and '/test/etc/test'. + +It is possible to create a `PathBuf` from a tail path parameter. The returned `PathBuf` is +percent-decoded. If a segment is equal to "..", the previous segment (if +any) is skipped. + +For security purposes, if a segment meets any of the following conditions, +an `Err` is returned indicating the condition met: + + * Decoded segment starts with any of: `.` (except `..`), `*` + * Decoded segment ends with any of: `:`, `>`, `<` + * Decoded segment contains any of: `/` + * On Windows, decoded segment contains any of: '\' + * Percent-encoding results in invalid UTF8. + +As a result of these conditions, a `PathBuf` parsed from request path parameter is +safe to interpolate within, or use as a suffix of, a path without additional +checks. + +```rust +extern crate actix; +use actix_web::*; +use std::path::PathBuf; + +fn index(req: Httprequest) -> String { + let path: PathBuf = req.match_info().query("tail")?; + format!("Path {:?}", path) +} + +fn main() { + Application::default("/") + .resource(r"/a/{tail:**}", |r| r.get(index)) + .finish(); +} +``` diff --git a/src/dev.rs b/src/dev.rs index cb409ccc0..11484107f 100644 --- a/src/dev.rs +++ b/src/dev.rs @@ -11,8 +11,8 @@ // dev specific pub use pipeline::Pipeline; pub use route::Handler; -pub use recognizer::RouteRecognizer; pub use channel::{HttpChannel, HttpHandler}; +pub use recognizer::{FromParam, RouteRecognizer}; pub use application::ApplicationBuilder; pub use httpresponse::HttpResponseBuilder; diff --git a/src/error.rs b/src/error.rs index c6c4a7eb9..f9b1fca07 100644 --- a/src/error.rs +++ b/src/error.rs @@ -33,20 +33,20 @@ pub type Result = result::Result; /// General purpose actix web error #[derive(Debug)] pub struct Error { - cause: Box, + cause: Box, } impl Error { /// Returns a reference to the underlying cause of this Error. // this should return &Fail but needs this https://github.com/rust-lang/rust/issues/5665 - pub fn cause(&self) -> &ErrorResponse { + pub fn cause(&self) -> &ResponseError { self.cause.as_ref() } } /// Error that can be converted to `HttpResponse` -pub trait ErrorResponse: Fail { +pub trait ResponseError: Fail { /// Create response for error /// @@ -69,8 +69,8 @@ impl From for HttpResponse { } } -/// `Error` for any error that implements `ErrorResponse` -impl From for Error { +/// `Error` for any error that implements `ResponseError` +impl From for Error { fn from(err: T) -> Error { Error { cause: Box::new(err) } } @@ -78,31 +78,31 @@ impl From for Error { /// Default error is `InternalServerError` #[cfg(actix_nightly)] -default impl ErrorResponse for T { +default impl ResponseError for T { fn error_response(&self) -> HttpResponse { HttpResponse::new(StatusCode::INTERNAL_SERVER_ERROR, Body::Empty) } } /// `InternalServerError` for `JsonError` -impl ErrorResponse for JsonError {} +impl ResponseError for JsonError {} /// Return `InternalServerError` for `HttpError`, /// Response generation can return `HttpError`, so it is internal error -impl ErrorResponse for HttpError {} +impl ResponseError for HttpError {} /// Return `InternalServerError` for `io::Error` -impl ErrorResponse for IoError {} +impl ResponseError for IoError {} /// `InternalServerError` for `InvalidHeaderValue` -impl ErrorResponse for header::InvalidHeaderValue {} +impl ResponseError for header::InvalidHeaderValue {} /// Internal error #[derive(Fail, Debug)] #[fail(display="Unexpected task frame")] pub struct UnexpectedTaskFrame; -impl ErrorResponse for UnexpectedTaskFrame {} +impl ResponseError for UnexpectedTaskFrame {} /// A set of errors that can occur during parsing HTTP streams #[derive(Fail, Debug)] @@ -141,7 +141,7 @@ pub enum ParseError { } /// Return `BadRequest` for `ParseError` -impl ErrorResponse for ParseError { +impl ResponseError for ParseError { fn error_response(&self) -> HttpResponse { HttpResponse::new(StatusCode::BAD_REQUEST, Body::Empty) } @@ -203,7 +203,7 @@ impl From for PayloadError { } /// Return `BadRequest` for `cookie::ParseError` -impl ErrorResponse for cookie::ParseError { +impl ResponseError for cookie::ParseError { fn error_response(&self) -> HttpResponse { HttpResponse::new(StatusCode::BAD_REQUEST, Body::Empty) } @@ -223,7 +223,7 @@ pub enum HttpRangeError { } /// Return `BadRequest` for `HttpRangeError` -impl ErrorResponse for HttpRangeError { +impl ResponseError for HttpRangeError { fn error_response(&self) -> HttpResponse { HttpResponse::new( StatusCode::BAD_REQUEST, Body::from("Invalid Range header provided")) @@ -272,7 +272,7 @@ impl From for MultipartError { } /// Return `BadRequest` for `MultipartError` -impl ErrorResponse for MultipartError { +impl ResponseError for MultipartError { fn error_response(&self) -> HttpResponse { HttpResponse::new(StatusCode::BAD_REQUEST, Body::Empty) @@ -290,7 +290,7 @@ pub enum ExpectError { UnknownExpect, } -impl ErrorResponse for ExpectError { +impl ResponseError for ExpectError { fn error_response(&self) -> HttpResponse { HTTPExpectationFailed.with_body("Unknown Expect") @@ -320,7 +320,7 @@ pub enum WsHandshakeError { BadWebsocketKey, } -impl ErrorResponse for WsHandshakeError { +impl ResponseError for WsHandshakeError { fn error_response(&self) -> HttpResponse { match *self { @@ -363,7 +363,30 @@ pub enum UrlencodedError { } /// Return `BadRequest` for `UrlencodedError` -impl ErrorResponse for UrlencodedError { +impl ResponseError for UrlencodedError { + + fn error_response(&self) -> HttpResponse { + HttpResponse::new(StatusCode::BAD_REQUEST, Body::Empty) + } +} + +/// Errors which can occur when attempting to interpret a segment string as a +/// valid path segment. +#[derive(Fail, Debug, PartialEq)] +pub enum UriSegmentError { + /// The segment started with the wrapped invalid character. + #[fail(display="The segment started with the wrapped invalid character")] + BadStart(char), + /// The segment contained the wrapped invalid character. + #[fail(display="The segment contained the wrapped invalid character")] + BadChar(char), + /// The segment ended with the wrapped invalid character. + #[fail(display="The segment ended with the wrapped invalid character")] + BadEnd(char), +} + +/// Return `BadRequest` for `UriSegmentError` +impl ResponseError for UriSegmentError { fn error_response(&self) -> HttpResponse { HttpResponse::new(StatusCode::BAD_REQUEST, Body::Empty) diff --git a/src/h1.rs b/src/h1.rs index ff358f154..1222eb618 100644 --- a/src/h1.rs +++ b/src/h1.rs @@ -19,7 +19,7 @@ use channel::HttpHandler; use h1writer::H1Writer; use httpcodes::HTTPNotFound; use httprequest::HttpRequest; -use error::{ParseError, PayloadError, ErrorResponse}; +use error::{ParseError, PayloadError, ResponseError}; use payload::{Payload, PayloadWriter, DEFAULT_BUFFER_SIZE}; const KEEPALIVE_PERIOD: u64 = 15; // seconds diff --git a/src/middlewares/session.rs b/src/middlewares/session.rs index 24ad9fefb..1ed02ee0c 100644 --- a/src/middlewares/session.rs +++ b/src/middlewares/session.rs @@ -13,7 +13,7 @@ use cookie::{CookieJar, Cookie, Key}; use futures::Future; use futures::future::{FutureResult, ok as FutOk, err as FutErr}; -use error::{Result, Error, ErrorResponse}; +use error::{Result, Error, ResponseError}; use httprequest::HttpRequest; use httpresponse::HttpResponse; use middlewares::{Middleware, Started, Response}; @@ -177,7 +177,7 @@ pub enum CookieSessionError { Serialize(JsonError), } -impl ErrorResponse for CookieSessionError {} +impl ResponseError for CookieSessionError {} impl SessionImpl for CookieSession { diff --git a/src/recognizer.rs b/src/recognizer.rs index 17366bdb8..b5c8ea9ec 100644 --- a/src/recognizer.rs +++ b/src/recognizer.rs @@ -1,10 +1,180 @@ +use std; use std::rc::Rc; +use std::path::PathBuf; +use std::ops::Index; +use std::str::FromStr; use std::collections::HashMap; - use regex::{Regex, RegexSet, Captures}; +use error::{ResponseError, UriSegmentError}; + +/// A trait to abstract the idea of creating a new instance of a type from a path parameter. +pub trait FromParam: Sized { + /// The associated error which can be returned from parsing. + type Err: ResponseError; + + /// Parses a string `s` to return a value of this type. + fn from_param(s: &str) -> Result; +} + +/// Route match information +/// +/// If resource path contains variable patterns, `Params` stores this variables. +#[derive(Debug)] +pub struct Params { + text: String, + matches: Vec>, + names: Rc>, +} + +impl Params { + pub(crate) fn new(names: Rc>, + text: &str, + captures: &Captures) -> Self + { + Params { + names, + text: text.into(), + matches: captures + .iter() + .map(|capture| capture.map(|m| (m.start(), m.end()))) + .collect(), + } + } + + pub(crate) fn empty() -> Self + { + Params { + text: String::new(), + names: Rc::new(HashMap::new()), + matches: Vec::new(), + } + } + + /// Check if there are any matched patterns + pub fn is_empty(&self) -> bool { + self.names.is_empty() + } + + fn by_idx(&self, index: usize) -> Option<&str> { + self.matches + .get(index + 1) + .and_then(|m| m.map(|(start, end)| &self.text[start..end])) + } + + /// Get matched parameter by name without type conversion + pub fn get(&self, key: &str) -> Option<&str> { + self.names.get(key).and_then(|&i| self.by_idx(i - 1)) + } + + /// Get matched `FromParam` compatible parameter by name. + /// + /// If keyed parameter is not available empty string is used as default value. + /// + /// ```rust,ignore + /// fn index(req: HttpRequest) -> String { + /// let ivalue: isize = req.match_info().query()?; + /// format!("isuze value: {:?}", ivalue) + /// } + /// ``` + pub fn query(&self, key: &str) -> Result::Err> + { + if let Some(s) = self.get(key) { + T::from_param(s) + } else { + T::from_param("") + } + } +} + +impl<'a> Index<&'a str> for Params { + type Output = str; + + fn index(&self, name: &'a str) -> &str { + self.get(name).expect("Value for parameter is not available") + } +} + +/// Creates a `PathBuf` from a path parameter. The returned `PathBuf` is +/// percent-decoded. If a segment is equal to "..", the previous segment (if +/// any) is skipped. +/// +/// For security purposes, if a segment meets any of the following conditions, +/// an `Err` is returned indicating the condition met: +/// +/// * Decoded segment starts with any of: `.` (except `..`), `*` +/// * Decoded segment ends with any of: `:`, `>`, `<` +/// * Decoded segment contains any of: `/` +/// * On Windows, decoded segment contains any of: '\' +/// * Percent-encoding results in invalid UTF8. +/// +/// As a result of these conditions, a `PathBuf` parsed from request path parameter is +/// safe to interpolate within, or use as a suffix of, a path without additional +/// checks. +impl FromParam for PathBuf { + type Err = UriSegmentError; + + fn from_param(val: &str) -> Result { + let mut buf = PathBuf::new(); + for segment in val.split('/') { + if segment == ".." { + buf.pop(); + } else if segment.starts_with('.') { + return Err(UriSegmentError::BadStart('.')) + } else if segment.starts_with('*') { + return Err(UriSegmentError::BadStart('*')) + } else if segment.ends_with(':') { + return Err(UriSegmentError::BadEnd(':')) + } else if segment.ends_with('>') { + return Err(UriSegmentError::BadEnd('>')) + } else if segment.ends_with('<') { + return Err(UriSegmentError::BadEnd('<')) + } else if segment.contains('/') { + return Err(UriSegmentError::BadChar('/')) + } else if cfg!(windows) && segment.contains('\\') { + return Err(UriSegmentError::BadChar('\\')) + } else { + buf.push(segment) + } + } + + Ok(buf) + } +} + +macro_rules! FROM_STR { + ($type:ty) => { + impl FromParam for $type { + type Err = <$type as FromStr>::Err; + + fn from_param(val: &str) -> Result { + <$type as FromStr>::from_str(val) + } + } + } +} + +FROM_STR!(u8); +FROM_STR!(u16); +FROM_STR!(u32); +FROM_STR!(u64); +FROM_STR!(usize); +FROM_STR!(i8); +FROM_STR!(i16); +FROM_STR!(i32); +FROM_STR!(i64); +FROM_STR!(isize); +FROM_STR!(f32); +FROM_STR!(f64); +FROM_STR!(String); +FROM_STR!(std::net::IpAddr); +FROM_STR!(std::net::Ipv4Addr); +FROM_STR!(std::net::Ipv6Addr); +FROM_STR!(std::net::SocketAddr); +FROM_STR!(std::net::SocketAddrV4); +FROM_STR!(std::net::SocketAddrV6); + -#[doc(hidden)] pub struct RouteRecognizer { prefix: usize, patterns: RegexSet, @@ -173,56 +343,6 @@ fn parse(pattern: &str) -> String { re } -/// Route match information -/// -/// If resource path contains variable patterns, `Params` stores this variables. -#[derive(Debug)] -pub struct Params { - text: String, - matches: Vec>, - names: Rc>, -} - -impl Params { - pub(crate) fn new(names: Rc>, - text: &str, - captures: &Captures) -> Self - { - Params { - names, - text: text.into(), - matches: captures - .iter() - .map(|capture| capture.map(|m| (m.start(), m.end()))) - .collect(), - } - } - - pub(crate) fn empty() -> Self - { - Params { - text: String::new(), - names: Rc::new(HashMap::new()), - matches: Vec::new(), - } - } - - pub fn is_empty(&self) -> bool { - self.names.is_empty() - } - - fn by_idx(&self, index: usize) -> Option<&str> { - self.matches - .get(index + 1) - .and_then(|m| m.map(|(start, end)| &self.text[start..end])) - } - - /// Get matched parameter by name - pub fn get(&self, key: &str) -> Option<&str> { - self.names.get(key).and_then(|&i| self.by_idx(i - 1)) - } -} - #[cfg(test)] mod tests { use regex::Regex; @@ -249,6 +369,7 @@ mod tests { assert_eq!(*val, 2); assert!(!params.as_ref().unwrap().is_empty()); assert_eq!(params.as_ref().unwrap().get("val").unwrap(), "value"); + assert_eq!(¶ms.as_ref().unwrap()["val"], "value"); let (params, val) = rec.recognize("/name/value2/index.html").unwrap(); assert_eq!(*val, 3);