From 5a25fd95f59b0de193bcae4403b66b5bcf2c1976 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Thu, 22 Mar 2018 18:08:12 -0700 Subject: [PATCH] Fix panic on invalid URL characters #130 --- CHANGES.md | 2 ++ src/client/parser.rs | 2 +- src/error.rs | 13 +++++++++---- src/server/h1.rs | 15 +++++++++------ 4 files changed, 21 insertions(+), 11 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index b45ec272..9043d3fd 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,6 +4,8 @@ * Fix long client urls #129 +* Fix panic on invalid URL characters #130 + * Fix client connection pooling * Use more ergonomic `actix_web::Error` instead of `http::Error` for `HttpResponseBuilder::body()` diff --git a/src/client/parser.rs b/src/client/parser.rs index 8fe39900..e0c49406 100644 --- a/src/client/parser.rs +++ b/src/client/parser.rs @@ -126,7 +126,7 @@ impl HttpResponseParser { let mut resp = httparse::Response::new(&mut headers); match resp.parse(b)? { httparse::Status::Complete(len) => { - let version = if resp.version.unwrap() == 1 { + let version = if resp.version.unwrap_or(1) == 1 { Version::HTTP_11 } else { Version::HTTP_10 diff --git a/src/error.rs b/src/error.rs index 98c48cbc..72ec5660 100644 --- a/src/error.rs +++ b/src/error.rs @@ -8,11 +8,10 @@ use cookie; use httparse; use actix::MailboxError; use futures::Canceled; -use failure; -use failure::{Fail, Backtrace}; +use failure::{self, Fail, Backtrace}; use http2::Error as Http2Error; use http::{header, StatusCode, Error as HttpError}; -use http::uri::InvalidUriBytes; +use http::uri::InvalidUri; use http_range::HttpRangeParseError; use serde_json::error::Error as JsonError; pub use url::ParseError as UrlParseError; @@ -157,7 +156,7 @@ pub enum ParseError { Method, /// An invalid `Uri`, such as `exam ple.domain`. #[fail(display="Uri error: {}", _0)] - Uri(InvalidUriBytes), + Uri(InvalidUri), /// An invalid `HttpVersion`, such as `HTP/1.1` #[fail(display="Invalid HTTP version specified")] Version, @@ -198,6 +197,12 @@ impl From for ParseError { } } +impl From for ParseError { + fn from(err: InvalidUri) -> ParseError { + ParseError::Uri(err) + } +} + impl From for ParseError { fn from(err: Utf8Error) -> ParseError { ParseError::Utf8(err) diff --git a/src/server/h1.rs b/src/server/h1.rs index 51f8f322..656c40c6 100644 --- a/src/server/h1.rs +++ b/src/server/h1.rs @@ -164,6 +164,8 @@ impl Http1 }, Ok(Async::NotReady) => (), Err(err) => { + trace!("Parse error: {:?}", err); + // notify all tasks self.stream.disconnected(); for entry in &mut self.tasks { @@ -293,9 +295,9 @@ impl Http1 // deal with keep-alive if self.tasks.is_empty() { // no keep-alive situations - if (self.flags.contains(Flags::ERROR) - || !self.flags.contains(Flags::KEEPALIVE) - || !self.settings.keep_alive_enabled()) && + if self.flags.contains(Flags::ERROR) || + (!self.flags.contains(Flags::KEEPALIVE) + || !self.settings.keep_alive_enabled()) && self.flags.contains(Flags::STARTED) { return Ok(Async::Ready(false)) @@ -502,10 +504,11 @@ impl Reader { let mut req = httparse::Request::new(&mut headers); match req.parse(b)? { httparse::Status::Complete(len) => { - let method = Method::from_bytes(req.method.unwrap().as_bytes()) + let method = Method::from_bytes( + req.method.unwrap_or("").as_bytes()) .map_err(|_| ParseError::Method)?; - let path = Uri::try_from(req.path.unwrap()).unwrap(); - let version = if req.version.unwrap() == 1 { + let path = Uri::try_from(req.path.unwrap())?; + let version = if req.version.unwrap_or(1) == 1 { Version::HTTP_11 } else { Version::HTTP_10