1
0
mirror of https://github.com/fafhrd91/actix-web synced 2024-11-27 17:52:56 +01:00

Do not use as it can not parse some valid paths

This commit is contained in:
Nikolay Kim 2017-10-27 22:19:00 -07:00
parent 76ffc60971
commit d93244aa4f
9 changed files with 91 additions and 58 deletions

View File

@ -3,6 +3,8 @@
## 0.2.0 (2017-10-xx) ## 0.2.0 (2017-10-xx)
* Do not use `http::Uri` as it can not parse some valid paths
* Refactor response `Body` * Refactor response `Body`
* Refactor `RouteRecognizer` usability * Refactor `RouteRecognizer` usability

View File

@ -41,6 +41,7 @@ regex = "0.2"
slab = "0.4" slab = "0.4"
sha1 = "0.2" sha1 = "0.2"
url = "1.5" url = "1.5"
percent-encoding = "1.0"
# tokio # tokio
bytes = "0.4" bytes = "0.4"

View File

@ -4,7 +4,7 @@ use std::collections::HashMap;
use bytes::BytesMut; use bytes::BytesMut;
use futures::{Async, Future, Stream, Poll}; use futures::{Async, Future, Stream, Poll};
use url::form_urlencoded; use url::form_urlencoded;
use http::{header, Method, Version, Uri, HeaderMap, Extensions}; use http::{header, Method, Version, HeaderMap, Extensions};
use {Cookie, CookieParseError}; use {Cookie, CookieParseError};
use {HttpRange, HttpRangeParseError}; use {HttpRange, HttpRangeParseError};
@ -18,7 +18,8 @@ use multipart::{Multipart, MultipartError};
pub struct HttpRequest { pub struct HttpRequest {
version: Version, version: Version,
method: Method, method: Method,
uri: Uri, path: String,
query: String,
headers: HeaderMap, headers: HeaderMap,
params: Params, params: Params,
cookies: Vec<Cookie<'static>>, cookies: Vec<Cookie<'static>>,
@ -28,10 +29,13 @@ pub struct HttpRequest {
impl HttpRequest { impl HttpRequest {
/// Construct a new Request. /// Construct a new Request.
#[inline] #[inline]
pub fn new(method: Method, uri: Uri, version: Version, headers: HeaderMap) -> Self { pub fn new(method: Method, path: String,
version: Version, headers: HeaderMap, query: String) -> Self
{
HttpRequest { HttpRequest {
method: method, method: method,
uri: uri, path: path,
query: query,
version: version, version: version,
headers: headers, headers: headers,
params: Params::empty(), params: Params::empty(),
@ -43,7 +47,8 @@ impl HttpRequest {
pub(crate) fn for_error() -> HttpRequest { pub(crate) fn for_error() -> HttpRequest {
HttpRequest { HttpRequest {
method: Method::GET, method: Method::GET,
uri: Uri::default(), path: String::new(),
query: String::new(),
version: Version::HTTP_11, version: Version::HTTP_11,
headers: HeaderMap::new(), headers: HeaderMap::new(),
params: Params::empty(), params: Params::empty(),
@ -58,10 +63,6 @@ impl HttpRequest {
&mut self.extensions &mut self.extensions
} }
/// Read the Request Uri.
#[inline]
pub fn uri(&self) -> &Uri { &self.uri }
/// Read the Request method. /// Read the Request method.
#[inline] #[inline]
pub fn method(&self) -> &Method { &self.method } pub fn method(&self) -> &Method { &self.method }
@ -79,13 +80,13 @@ impl HttpRequest {
/// The target path of this Request. /// The target path of this Request.
#[inline] #[inline]
pub fn path(&self) -> &str { pub fn path(&self) -> &str {
self.uri.path() &self.path
} }
/// Return a new iterator that yields pairs of `Cow<str>` for query parameters /// Return a new iterator that yields pairs of `Cow<str>` for query parameters
#[inline] #[inline]
pub fn query(&self) -> form_urlencoded::Parse { pub fn query(&self) -> form_urlencoded::Parse {
form_urlencoded::parse(self.query_string().as_ref()) form_urlencoded::parse(self.query.as_ref())
} }
/// The query string in the URL. /// The query string in the URL.
@ -93,7 +94,7 @@ impl HttpRequest {
/// E.g., id=10 /// E.g., id=10
#[inline] #[inline]
pub fn query_string(&self) -> &str { pub fn query_string(&self) -> &str {
self.uri.query().unwrap_or("") &self.query
} }
/// Return request cookies. /// Return request cookies.
@ -238,7 +239,8 @@ impl HttpRequest {
impl fmt::Debug for HttpRequest { impl fmt::Debug for HttpRequest {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let res = write!(f, "\nHttpRequest {:?} {}:{}\n", self.version, self.method, self.uri); let res = write!(f, "\nHttpRequest {:?} {}:{}\n",
self.version, self.method, self.path);
if !self.params.is_empty() { if !self.params.is_empty() {
let _ = write!(f, " params: {:?}\n", self.params); let _ = write!(f, " params: {:?}\n", self.params);
} }
@ -289,9 +291,6 @@ impl Future for UrlEncoded {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
use http::{Uri, HttpTryFrom};
// use futures::future::{lazy, result};
// use tokio_core::reactor::Core;
use payload::Payload; use payload::Payload;
#[test] #[test]
@ -300,7 +299,7 @@ mod tests {
headers.insert(header::TRANSFER_ENCODING, headers.insert(header::TRANSFER_ENCODING,
header::HeaderValue::from_static("chunked")); header::HeaderValue::from_static("chunked"));
let req = HttpRequest::new( let req = HttpRequest::new(
Method::GET, Uri::try_from("/").unwrap(), Version::HTTP_11, headers); Method::GET, "/".to_owned(), Version::HTTP_11, headers, String::new());
let (_, payload) = Payload::new(false); let (_, payload) = Payload::new(false);
assert!(req.urlencoded(payload).is_err()); assert!(req.urlencoded(payload).is_err());
@ -311,7 +310,7 @@ mod tests {
headers.insert(header::CONTENT_LENGTH, headers.insert(header::CONTENT_LENGTH,
header::HeaderValue::from_static("xxxx")); header::HeaderValue::from_static("xxxx"));
let req = HttpRequest::new( let req = HttpRequest::new(
Method::GET, Uri::try_from("/").unwrap(), Version::HTTP_11, headers); Method::GET, "/".to_owned(), Version::HTTP_11, headers, String::new());
let (_, payload) = Payload::new(false); let (_, payload) = Payload::new(false);
assert!(req.urlencoded(payload).is_err()); assert!(req.urlencoded(payload).is_err());
@ -322,7 +321,7 @@ mod tests {
headers.insert(header::CONTENT_LENGTH, headers.insert(header::CONTENT_LENGTH,
header::HeaderValue::from_static("1000000")); header::HeaderValue::from_static("1000000"));
let req = HttpRequest::new( let req = HttpRequest::new(
Method::GET, Uri::try_from("/").unwrap(), Version::HTTP_11, headers); Method::GET, "/".to_owned(), Version::HTTP_11, headers, String::new());
let (_, payload) = Payload::new(false); let (_, payload) = Payload::new(false);
assert!(req.urlencoded(payload).is_err()); assert!(req.urlencoded(payload).is_err());
@ -333,7 +332,7 @@ mod tests {
headers.insert(header::CONTENT_LENGTH, headers.insert(header::CONTENT_LENGTH,
header::HeaderValue::from_static("10")); header::HeaderValue::from_static("10"));
let req = HttpRequest::new( let req = HttpRequest::new(
Method::GET, Uri::try_from("/").unwrap(), Version::HTTP_11, headers); Method::GET, "/".to_owned(), Version::HTTP_11, headers, String::new());
let (_, payload) = Payload::new(false); let (_, payload) = Payload::new(false);
assert!(req.urlencoded(payload).is_err()); assert!(req.urlencoded(payload).is_err());

View File

@ -19,6 +19,7 @@ extern crate http_range;
extern crate mime; extern crate mime;
extern crate mime_guess; extern crate mime_guess;
extern crate url; extern crate url;
extern crate percent_encoding;
extern crate actix; extern crate actix;
mod application; mod application;

View File

@ -52,7 +52,13 @@ impl Logger {
match *text { match *text {
FormatText::Str(ref string) => fmt.write_str(string), FormatText::Str(ref string) => fmt.write_str(string),
FormatText::Method => req.method().fmt(fmt), FormatText::Method => req.method().fmt(fmt),
FormatText::URI => req.uri().fmt(fmt), FormatText::URI => {
if req.query_string().is_empty() {
fmt.write_fmt(format_args!("{}", req.path()))
} else {
fmt.write_fmt(format_args!("{}?{}", req.path(), req.query_string()))
}
},
FormatText::Status => resp.status().fmt(fmt), FormatText::Status => resp.status().fmt(fmt),
FormatText::ResponseTime => FormatText::ResponseTime =>
fmt.write_fmt(format_args!("{} ms", response_time_ms)), fmt.write_fmt(format_args!("{} ms", response_time_ms)),

View File

@ -1,11 +1,12 @@
use std::{self, io, ptr}; use std::{self, io, ptr};
use httparse; use httparse;
use http::{Method, Version, Uri, HttpTryFrom, HeaderMap}; use http::{Method, Version, HttpTryFrom, HeaderMap};
use http::header::{self, HeaderName, HeaderValue}; use http::header::{self, HeaderName, HeaderValue};
use bytes::{BytesMut, BufMut}; use bytes::{BytesMut, BufMut};
use futures::{Async, Poll}; use futures::{Async, Poll};
use tokio_io::AsyncRead; use tokio_io::AsyncRead;
use percent_encoding;
use error::ParseError; use error::ParseError;
use decode::Decoder; use decode::Decoder;
@ -248,8 +249,31 @@ impl Reader {
let slice = buf.split_to(len).freeze(); let slice = buf.split_to(len).freeze();
let path = slice.slice(path.0, path.1); let path = slice.slice(path.0, path.1);
// path was found to be utf8 by httparse
let uri = Uri::from_shared(path).map_err(|_| ParseError::Uri)?; // manually split path, path was found to be utf8 by httparse
let uri = {
if let Ok(path) = percent_encoding::percent_decode(&path).decode_utf8() {
let parts: Vec<&str> = path.splitn(2, '?').collect();
if parts.len() == 2 {
Some((parts[0].to_owned(), parts[1][1..].to_owned()))
} else {
Some((parts[0].to_owned(), String::new()))
}
} else {
None
}
};
let (path, query) = if let Some(uri) = uri {
uri
} else {
let parts: Vec<&str> = unsafe{
std::str::from_utf8_unchecked(&path)}.splitn(2, '?').collect();
if parts.len() == 2 {
(parts[0].to_owned(), parts[1][1..].to_owned())
} else {
(parts[0].to_owned(), String::new())
}
};
// convert headers // convert headers
let mut headers = HeaderMap::with_capacity(headers_len); let mut headers = HeaderMap::with_capacity(headers_len);
@ -267,7 +291,7 @@ impl Reader {
} }
} }
let msg = HttpRequest::new(method, uri, version, headers); let msg = HttpRequest::new(method, path, version, headers, query);
let decoder = if msg.upgrade() { let decoder = if msg.upgrade() {
Some(Decoder::eof()) Some(Decoder::eof())

View File

@ -325,20 +325,20 @@ impl WsWriter {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use http::{Method, HeaderMap, StatusCode, Uri, Version, HttpTryFrom, header}; use http::{Method, HeaderMap, StatusCode, Version, header};
use super::{HttpRequest, SEC_WEBSOCKET_VERSION, SEC_WEBSOCKET_KEY, handshake}; use super::{HttpRequest, SEC_WEBSOCKET_VERSION, SEC_WEBSOCKET_KEY, handshake};
#[test] #[test]
fn test_handshake() { fn test_handshake() {
let req = HttpRequest::new(Method::POST, Uri::try_from("/").unwrap(), let req = HttpRequest::new(Method::POST, "/".to_owned(),
Version::HTTP_11, HeaderMap::new()); Version::HTTP_11, HeaderMap::new(), String::new());
match handshake(&req) { match handshake(&req) {
Err(err) => assert_eq!(err.status(), StatusCode::METHOD_NOT_ALLOWED), Err(err) => assert_eq!(err.status(), StatusCode::METHOD_NOT_ALLOWED),
_ => panic!("should not happen"), _ => panic!("should not happen"),
} }
let req = HttpRequest::new(Method::GET, Uri::try_from("/").unwrap(), let req = HttpRequest::new(Method::GET, "/".to_owned(),
Version::HTTP_11, HeaderMap::new()); Version::HTTP_11, HeaderMap::new(), String::new());
match handshake(&req) { match handshake(&req) {
Err(err) => assert_eq!(err.status(), StatusCode::METHOD_NOT_ALLOWED), Err(err) => assert_eq!(err.status(), StatusCode::METHOD_NOT_ALLOWED),
_ => panic!("should not happen"), _ => panic!("should not happen"),
@ -347,8 +347,8 @@ mod tests {
let mut headers = HeaderMap::new(); let mut headers = HeaderMap::new();
headers.insert(header::UPGRADE, headers.insert(header::UPGRADE,
header::HeaderValue::from_static("test")); header::HeaderValue::from_static("test"));
let req = HttpRequest::new(Method::GET, Uri::try_from("/").unwrap(), let req = HttpRequest::new(Method::GET, "/".to_owned(),
Version::HTTP_11, headers); Version::HTTP_11, headers, String::new());
match handshake(&req) { match handshake(&req) {
Err(err) => assert_eq!(err.status(), StatusCode::METHOD_NOT_ALLOWED), Err(err) => assert_eq!(err.status(), StatusCode::METHOD_NOT_ALLOWED),
_ => panic!("should not happen"), _ => panic!("should not happen"),
@ -357,8 +357,8 @@ mod tests {
let mut headers = HeaderMap::new(); let mut headers = HeaderMap::new();
headers.insert(header::UPGRADE, headers.insert(header::UPGRADE,
header::HeaderValue::from_static("websocket")); header::HeaderValue::from_static("websocket"));
let req = HttpRequest::new(Method::GET, Uri::try_from("/").unwrap(), let req = HttpRequest::new(Method::GET, "/".to_owned(),
Version::HTTP_11, headers); Version::HTTP_11, headers, String::new());
match handshake(&req) { match handshake(&req) {
Err(err) => assert_eq!(err.status(), StatusCode::BAD_REQUEST), Err(err) => assert_eq!(err.status(), StatusCode::BAD_REQUEST),
_ => panic!("should not happen"), _ => panic!("should not happen"),
@ -369,8 +369,8 @@ mod tests {
header::HeaderValue::from_static("websocket")); header::HeaderValue::from_static("websocket"));
headers.insert(header::CONNECTION, headers.insert(header::CONNECTION,
header::HeaderValue::from_static("upgrade")); header::HeaderValue::from_static("upgrade"));
let req = HttpRequest::new(Method::GET, Uri::try_from("/").unwrap(), let req = HttpRequest::new(Method::GET, "/".to_owned(),
Version::HTTP_11, headers); Version::HTTP_11, headers, String::new());
match handshake(&req) { match handshake(&req) {
Err(err) => assert_eq!(err.status(), StatusCode::BAD_REQUEST), Err(err) => assert_eq!(err.status(), StatusCode::BAD_REQUEST),
_ => panic!("should not happen"), _ => panic!("should not happen"),
@ -383,8 +383,8 @@ mod tests {
header::HeaderValue::from_static("upgrade")); header::HeaderValue::from_static("upgrade"));
headers.insert(SEC_WEBSOCKET_VERSION, headers.insert(SEC_WEBSOCKET_VERSION,
header::HeaderValue::from_static("5")); header::HeaderValue::from_static("5"));
let req = HttpRequest::new(Method::GET, Uri::try_from("/").unwrap(), let req = HttpRequest::new(Method::GET, "/".to_owned(),
Version::HTTP_11, headers); Version::HTTP_11, headers, String::new());
match handshake(&req) { match handshake(&req) {
Err(err) => assert_eq!(err.status(), StatusCode::BAD_REQUEST), Err(err) => assert_eq!(err.status(), StatusCode::BAD_REQUEST),
_ => panic!("should not happen"), _ => panic!("should not happen"),
@ -397,8 +397,8 @@ mod tests {
header::HeaderValue::from_static("upgrade")); header::HeaderValue::from_static("upgrade"));
headers.insert(SEC_WEBSOCKET_VERSION, headers.insert(SEC_WEBSOCKET_VERSION,
header::HeaderValue::from_static("13")); header::HeaderValue::from_static("13"));
let req = HttpRequest::new(Method::GET, Uri::try_from("/").unwrap(), let req = HttpRequest::new(Method::GET, "/".to_owned(),
Version::HTTP_11, headers); Version::HTTP_11, headers, String::new());
match handshake(&req) { match handshake(&req) {
Err(err) => assert_eq!(err.status(), StatusCode::BAD_REQUEST), Err(err) => assert_eq!(err.status(), StatusCode::BAD_REQUEST),
_ => panic!("should not happen"), _ => panic!("should not happen"),
@ -413,8 +413,8 @@ mod tests {
header::HeaderValue::from_static("13")); header::HeaderValue::from_static("13"));
headers.insert(SEC_WEBSOCKET_KEY, headers.insert(SEC_WEBSOCKET_KEY,
header::HeaderValue::from_static("13")); header::HeaderValue::from_static("13"));
let req = HttpRequest::new(Method::GET, Uri::try_from("/").unwrap(), let req = HttpRequest::new(Method::GET, "/".to_owned(),
Version::HTTP_11, headers); Version::HTTP_11, headers, String::new());
match handshake(&req) { match handshake(&req) {
Ok(resp) => { Ok(resp) => {
assert_eq!(resp.status(), StatusCode::SWITCHING_PROTOCOLS) assert_eq!(resp.status(), StatusCode::SWITCHING_PROTOCOLS)

View File

@ -4,13 +4,13 @@ extern crate time;
use std::str; use std::str;
use actix_web::*; use actix_web::*;
use http::{header, Method, Uri, Version, HeaderMap, HttpTryFrom}; use http::{header, Method, Version, HeaderMap};
#[test] #[test]
fn test_no_request_cookies() { fn test_no_request_cookies() {
let mut req = HttpRequest::new( let mut req = HttpRequest::new(
Method::GET, Uri::try_from("/").unwrap(), Version::HTTP_11, HeaderMap::new()); Method::GET, "/".to_owned(), Version::HTTP_11, HeaderMap::new(), String::new());
assert!(req.cookies().is_empty()); assert!(req.cookies().is_empty());
let _ = req.load_cookies(); let _ = req.load_cookies();
assert!(req.cookies().is_empty()); assert!(req.cookies().is_empty());
@ -23,7 +23,7 @@ fn test_request_cookies() {
header::HeaderValue::from_static("cookie1=value1; cookie2=value2")); header::HeaderValue::from_static("cookie1=value1; cookie2=value2"));
let mut req = HttpRequest::new( let mut req = HttpRequest::new(
Method::GET, Uri::try_from("/").unwrap(), Version::HTTP_11, headers); Method::GET, "/".to_owned(), Version::HTTP_11, headers, String::new());
assert!(req.cookies().is_empty()); assert!(req.cookies().is_empty());
{ {
let cookies = req.load_cookies().unwrap(); let cookies = req.load_cookies().unwrap();
@ -46,8 +46,8 @@ fn test_request_cookies() {
#[test] #[test]
fn test_no_request_range_header() { fn test_no_request_range_header() {
let req = HttpRequest::new(Method::GET, Uri::try_from("/").unwrap(), let req = HttpRequest::new(Method::GET, "/".to_owned(),
Version::HTTP_11, HeaderMap::new()); Version::HTTP_11, HeaderMap::new(), String::new());
let ranges = req.range(100).unwrap(); let ranges = req.range(100).unwrap();
assert!(ranges.is_empty()); assert!(ranges.is_empty());
} }
@ -58,8 +58,8 @@ fn test_request_range_header() {
headers.insert(header::RANGE, headers.insert(header::RANGE,
header::HeaderValue::from_static("bytes=0-4")); header::HeaderValue::from_static("bytes=0-4"));
let req = HttpRequest::new(Method::GET, Uri::try_from("/").unwrap(), let req = HttpRequest::new(Method::GET, "/".to_owned(),
Version::HTTP_11, headers); Version::HTTP_11, headers, String::new());
let ranges = req.range(100).unwrap(); let ranges = req.range(100).unwrap();
assert_eq!(ranges.len(), 1); assert_eq!(ranges.len(), 1);
assert_eq!(ranges[0].start, 0); assert_eq!(ranges[0].start, 0);
@ -68,8 +68,8 @@ fn test_request_range_header() {
#[test] #[test]
fn test_request_query() { fn test_request_query() {
let req = HttpRequest::new(Method::GET, Uri::try_from("/?id=test").unwrap(), let req = HttpRequest::new(Method::GET, "/".to_owned(),
Version::HTTP_11, HeaderMap::new()); Version::HTTP_11, HeaderMap::new(), "id=test".to_owned());
assert_eq!(req.query_string(), "id=test"); assert_eq!(req.query_string(), "id=test");
let query: Vec<_> = req.query().collect(); let query: Vec<_> = req.query().collect();
@ -79,8 +79,8 @@ fn test_request_query() {
#[test] #[test]
fn test_request_match_info() { fn test_request_match_info() {
let mut req = HttpRequest::new(Method::GET, Uri::try_from("/value/?id=test").unwrap(), let mut req = HttpRequest::new(Method::GET, "/value/".to_owned(),
Version::HTTP_11, HeaderMap::new()); Version::HTTP_11, HeaderMap::new(), "?id=test".to_owned());
let rec = RouteRecognizer::new("/".to_owned(), vec![("/{key}/".to_owned(), 1)]); let rec = RouteRecognizer::new("/".to_owned(), vec![("/{key}/".to_owned(), 1)]);
let (params, _) = rec.recognize(req.path()).unwrap(); let (params, _) = rec.recognize(req.path()).unwrap();
@ -93,14 +93,14 @@ fn test_request_match_info() {
#[test] #[test]
fn test_chunked() { fn test_chunked() {
let req = HttpRequest::new( let req = HttpRequest::new(
Method::GET, Uri::try_from("/").unwrap(), Version::HTTP_11, HeaderMap::new()); Method::GET, "/".to_owned(), Version::HTTP_11, HeaderMap::new(), String::new());
assert!(!req.chunked().unwrap()); assert!(!req.chunked().unwrap());
let mut headers = HeaderMap::new(); let mut headers = HeaderMap::new();
headers.insert(header::TRANSFER_ENCODING, headers.insert(header::TRANSFER_ENCODING,
header::HeaderValue::from_static("chunked")); header::HeaderValue::from_static("chunked"));
let req = HttpRequest::new( let req = HttpRequest::new(
Method::GET, Uri::try_from("/").unwrap(), Version::HTTP_11, headers); Method::GET, "/".to_owned(), Version::HTTP_11, headers, String::new());
assert!(req.chunked().unwrap()); assert!(req.chunked().unwrap());
let mut headers = HeaderMap::new(); let mut headers = HeaderMap::new();
@ -109,6 +109,6 @@ fn test_chunked() {
headers.insert(header::TRANSFER_ENCODING, headers.insert(header::TRANSFER_ENCODING,
header::HeaderValue::from_str(s).unwrap()); header::HeaderValue::from_str(s).unwrap());
let req = HttpRequest::new( let req = HttpRequest::new(
Method::GET, Uri::try_from("/").unwrap(), Version::HTTP_11, headers); Method::GET, "/".to_owned(), Version::HTTP_11, headers, String::new());
assert!(req.chunked().is_err()); assert!(req.chunked().is_err());
} }

View File

@ -4,7 +4,7 @@ extern crate time;
use actix_web::*; use actix_web::*;
use time::Duration; use time::Duration;
use http::{header, Method, Uri, Version, HeaderMap, HttpTryFrom}; use http::{header, Method, Version, HeaderMap};
#[test] #[test]
@ -14,7 +14,7 @@ fn test_response_cookies() {
header::HeaderValue::from_static("cookie1=value1; cookie2=value2")); header::HeaderValue::from_static("cookie1=value1; cookie2=value2"));
let mut req = HttpRequest::new( let mut req = HttpRequest::new(
Method::GET, Uri::try_from("/").unwrap(), Version::HTTP_11, headers); Method::GET, "/".to_owned(), Version::HTTP_11, headers, String::new());
let cookies = req.load_cookies().unwrap(); let cookies = req.load_cookies().unwrap();
let resp = httpcodes::HTTPOk let resp = httpcodes::HTTPOk