From b6ed778775a83f9813ff067678dea72e02cf6756 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Sun, 17 Jun 2018 08:48:50 +0600 Subject: [PATCH] remove HttpMessage::range() --- CHANGES.md | 2 + Cargo.toml | 1 - src/error.rs | 43 ----- src/fs.rs | 435 +++++++++++++++++++++++++++++++++++++++++---- src/httpmessage.rs | 31 +--- src/lib.rs | 2 - 6 files changed, 407 insertions(+), 107 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 9f1651632..be17ef3d1 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -46,6 +46,8 @@ * Remove `Route::with2()` and `Route::with3()` use tuple of extractors instead. +* Remove `HttpMessage::range()` + ## [0.6.13] - 2018-06-11 diff --git a/Cargo.toml b/Cargo.toml index fe5dfba02..b8157421c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -60,7 +60,6 @@ h2 = "0.1" fnv = "1.0.5" http = "^0.1.5" httparse = "1.2" -http-range = "0.1" libc = "0.2" log = "0.4" mime = "0.3" diff --git a/src/error.rs b/src/error.rs index c272a2dca..395418d9e 100644 --- a/src/error.rs +++ b/src/error.rs @@ -12,7 +12,6 @@ use futures::Canceled; use http::uri::InvalidUri; use http::{header, Error as HttpError, StatusCode}; use http2::Error as Http2Error; -use http_range::HttpRangeParseError; use httparse; use serde::de::value::Error as DeError; use serde_json::error::Error as JsonError; @@ -395,37 +394,6 @@ impl ResponseError for cookie::ParseError { } } -/// Http range header parsing error -#[derive(Fail, PartialEq, Debug)] -pub enum HttpRangeError { - /// Returned if range is invalid. - #[fail(display = "Range header is invalid")] - InvalidRange, - /// Returned if first-byte-pos of all of the byte-range-spec - /// values is greater than the content size. - /// See `https://github.com/golang/go/commit/aa9b3d7` - #[fail( - display = "First-byte-pos of all of the byte-range-spec values is greater than the content size" - )] - NoOverlap, -} - -/// Return `BadRequest` for `HttpRangeError` -impl ResponseError for HttpRangeError { - fn error_response(&self) -> HttpResponse { - HttpResponse::with_body(StatusCode::BAD_REQUEST, "Invalid Range header provided") - } -} - -impl From for HttpRangeError { - fn from(err: HttpRangeParseError) -> HttpRangeError { - match err { - HttpRangeParseError::InvalidRange => HttpRangeError::InvalidRange, - HttpRangeParseError::NoOverlap => HttpRangeError::NoOverlap, - } - } -} - /// A set of errors that can occur during parsing multipart streams #[derive(Fail, Debug)] pub enum MultipartError { @@ -953,9 +921,6 @@ mod tests { let resp: HttpResponse = ParseError::Incomplete.error_response(); assert_eq!(resp.status(), StatusCode::BAD_REQUEST); - let resp: HttpResponse = HttpRangeError::InvalidRange.error_response(); - assert_eq!(resp.status(), StatusCode::BAD_REQUEST); - let resp: HttpResponse = CookieParseError::EmptyName.error_response(); assert_eq!(resp.status(), StatusCode::BAD_REQUEST); @@ -1005,14 +970,6 @@ mod tests { assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); } - #[test] - fn test_range_error() { - let e: HttpRangeError = HttpRangeParseError::InvalidRange.into(); - assert_eq!(e, HttpRangeError::InvalidRange); - let e: HttpRangeError = HttpRangeParseError::NoOverlap.into(); - assert_eq!(e, HttpRangeError::NoOverlap); - } - #[test] fn test_expect_error() { let resp: HttpResponse = ExpectError::Encoding.error_response(); diff --git a/src/fs.rs b/src/fs.rs index 35c78b736..68d6977cf 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -20,7 +20,7 @@ use mime_guess::{get_mime_type, guess_mime_type}; use error::Error; use handler::{AsyncResult, Handler, Responder, RouteHandler, WrapHandler}; use header; -use http::{ContentEncoding, HttpRange, Method, StatusCode}; +use http::{ContentEncoding, Method, StatusCode}; use httpmessage::HttpMessage; use httprequest::HttpRequest; use httpresponse::HttpResponse; @@ -63,18 +63,20 @@ impl NamedFile { /// let file = NamedFile::open("foo.txt"); /// ``` pub fn open>(path: P) -> io::Result { - use header::{ContentDisposition, DispositionType, DispositionParam}; + use header::{ContentDisposition, DispositionParam, DispositionType}; let path = path.as_ref().to_path_buf(); // Get the name of the file and use it to construct default Content-Type // and Content-Disposition values - let (content_type, content_disposition) = - { + let (content_type, content_disposition) = { let filename = match path.file_name() { Some(name) => name.to_string_lossy(), - None => return Err(io::Error::new( - io::ErrorKind::InvalidInput, - "Provided path has no filename")), + None => { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "Provided path has no filename", + )) + } }; let ct = guess_mime_type(&path); @@ -84,13 +86,11 @@ impl NamedFile { }; let cd = ContentDisposition { disposition: disposition_type, - parameters: vec![ - DispositionParam::Filename( - header::Charset::Ext("UTF-8".to_owned()), - None, - filename.as_bytes().to_vec(), - ) - ], + parameters: vec![DispositionParam::Filename( + header::Charset::Ext("UTF-8".to_owned()), + None, + filename.as_bytes().to_vec(), + )], }; (ct, cd) }; @@ -276,7 +276,10 @@ impl Responder for NamedFile { if self.status_code != StatusCode::OK { let mut resp = HttpResponse::build(self.status_code); resp.set(header::ContentType(self.content_type.clone())) - .header(header::CONTENT_DISPOSITION, self.content_disposition.to_string()); + .header( + header::CONTENT_DISPOSITION, + self.content_disposition.to_string(), + ); if let Some(current_encoding) = self.encoding { resp.content_encoding(current_encoding); @@ -327,19 +330,20 @@ impl Responder for NamedFile { let mut resp = HttpResponse::build(self.status_code); resp.set(header::ContentType(self.content_type.clone())) - .header(header::CONTENT_DISPOSITION, self.content_disposition.to_string()); + .header( + header::CONTENT_DISPOSITION, + self.content_disposition.to_string(), + ); if let Some(current_encoding) = self.encoding { resp.content_encoding(current_encoding); } - resp - .if_some(last_modified, |lm, resp| { - resp.set(header::LastModified(lm)); - }) - .if_some(etag, |etag, resp| { - resp.set(header::ETag(etag)); - }); + resp.if_some(last_modified, |lm, resp| { + resp.set(header::LastModified(lm)); + }).if_some(etag, |etag, resp| { + resp.set(header::ETag(etag)); + }); resp.header(header::ACCEPT_RANGES, "bytes"); @@ -721,6 +725,101 @@ impl Handler for StaticFiles { } } +/// HTTP Range header representation. +#[derive(Debug, Clone, Copy)] +struct HttpRange { + pub start: u64, + pub length: u64, +} + +static PREFIX: &'static str = "bytes="; +const PREFIX_LEN: usize = 6; + +impl HttpRange { + /// Parses Range HTTP header string as per RFC 2616. + /// + /// `header` is HTTP Range header (e.g. `bytes=bytes=0-9`). + /// `size` is full size of response (file). + fn parse(header: &str, size: u64) -> Result, ()> { + if header.is_empty() { + return Ok(Vec::new()); + } + if !header.starts_with(PREFIX) { + return Err(()); + } + + let size_sig = size as i64; + let mut no_overlap = false; + + let all_ranges: Vec> = header[PREFIX_LEN..] + .split(',') + .map(|x| x.trim()) + .filter(|x| !x.is_empty()) + .map(|ra| { + let mut start_end_iter = ra.split('-'); + + let start_str = start_end_iter.next().ok_or(())?.trim(); + let end_str = start_end_iter.next().ok_or(())?.trim(); + + if start_str.is_empty() { + // If no start is specified, end specifies the + // range start relative to the end of the file. + let mut length: i64 = try!(end_str.parse().map_err(|_| ())); + + if length > size_sig { + length = size_sig; + } + + Ok(Some(HttpRange { + start: (size_sig - length) as u64, + length: length as u64, + })) + } else { + let start: i64 = start_str.parse().map_err(|_| ())?; + + if start < 0 { + return Err(()); + } + if start >= size_sig { + no_overlap = true; + return Ok(None); + } + + let length = if end_str.is_empty() { + // If no end is specified, range extends to end of the file. + size_sig - start + } else { + let mut end: i64 = end_str.parse().map_err(|_| ())?; + + if start > end { + return Err(()); + } + + if end >= size_sig { + end = size_sig - 1; + } + + end - start + 1 + }; + + Ok(Some(HttpRange { + start: start as u64, + length: length as u64, + })) + } + }) + .collect::>()?; + + let ranges: Vec = all_ranges.into_iter().filter_map(|x| x).collect(); + + if no_overlap && ranges.is_empty() { + return Err(()); + } + + Ok(ranges) + } +} + #[cfg(test)] mod tests { use super::*; @@ -816,16 +915,14 @@ mod tests { #[test] fn test_named_file_image_attachment() { - use header::{ContentDisposition, DispositionType, DispositionParam}; + use header::{ContentDisposition, DispositionParam, DispositionType}; let cd = ContentDisposition { disposition: DispositionType::Attachment, - parameters: vec![ - DispositionParam::Filename( - header::Charset::Ext("UTF-8".to_owned()), - None, - "test.png".as_bytes().to_vec(), - ) - ], + parameters: vec![DispositionParam::Filename( + header::Charset::Ext("UTF-8".to_owned()), + None, + "test.png".as_bytes().to_vec(), + )], }; let mut file = NamedFile::open("tests/test.png") .unwrap() @@ -1241,4 +1338,280 @@ mod tests { let response = srv.execute(request.send()).unwrap(); assert_eq!(response.status(), StatusCode::OK); } + + struct T(&'static str, u64, Vec); + + #[test] + fn test_parse() { + let tests = vec![ + T("", 0, vec![]), + T("", 1000, vec![]), + T("foo", 0, vec![]), + T("bytes=", 0, vec![]), + T("bytes=7", 10, vec![]), + T("bytes= 7 ", 10, vec![]), + T("bytes=1-", 0, vec![]), + T("bytes=5-4", 10, vec![]), + T("bytes=0-2,5-4", 10, vec![]), + T("bytes=2-5,4-3", 10, vec![]), + T("bytes=--5,4--3", 10, vec![]), + T("bytes=A-", 10, vec![]), + T("bytes=A- ", 10, vec![]), + T("bytes=A-Z", 10, vec![]), + T("bytes= -Z", 10, vec![]), + T("bytes=5-Z", 10, vec![]), + T("bytes=Ran-dom, garbage", 10, vec![]), + T("bytes=0x01-0x02", 10, vec![]), + T("bytes= ", 10, vec![]), + T("bytes= , , , ", 10, vec![]), + T( + "bytes=0-9", + 10, + vec![HttpRange { + start: 0, + length: 10, + }], + ), + T( + "bytes=0-", + 10, + vec![HttpRange { + start: 0, + length: 10, + }], + ), + T( + "bytes=5-", + 10, + vec![HttpRange { + start: 5, + length: 5, + }], + ), + T( + "bytes=0-20", + 10, + vec![HttpRange { + start: 0, + length: 10, + }], + ), + T( + "bytes=15-,0-5", + 10, + vec![HttpRange { + start: 0, + length: 6, + }], + ), + T( + "bytes=1-2,5-", + 10, + vec![ + HttpRange { + start: 1, + length: 2, + }, + HttpRange { + start: 5, + length: 5, + }, + ], + ), + T( + "bytes=-2 , 7-", + 11, + vec![ + HttpRange { + start: 9, + length: 2, + }, + HttpRange { + start: 7, + length: 4, + }, + ], + ), + T( + "bytes=0-0 ,2-2, 7-", + 11, + vec![ + HttpRange { + start: 0, + length: 1, + }, + HttpRange { + start: 2, + length: 1, + }, + HttpRange { + start: 7, + length: 4, + }, + ], + ), + T( + "bytes=-5", + 10, + vec![HttpRange { + start: 5, + length: 5, + }], + ), + T( + "bytes=-15", + 10, + vec![HttpRange { + start: 0, + length: 10, + }], + ), + T( + "bytes=0-499", + 10000, + vec![HttpRange { + start: 0, + length: 500, + }], + ), + T( + "bytes=500-999", + 10000, + vec![HttpRange { + start: 500, + length: 500, + }], + ), + T( + "bytes=-500", + 10000, + vec![HttpRange { + start: 9500, + length: 500, + }], + ), + T( + "bytes=9500-", + 10000, + vec![HttpRange { + start: 9500, + length: 500, + }], + ), + T( + "bytes=0-0,-1", + 10000, + vec![ + HttpRange { + start: 0, + length: 1, + }, + HttpRange { + start: 9999, + length: 1, + }, + ], + ), + T( + "bytes=500-600,601-999", + 10000, + vec![ + HttpRange { + start: 500, + length: 101, + }, + HttpRange { + start: 601, + length: 399, + }, + ], + ), + T( + "bytes=500-700,601-999", + 10000, + vec![ + HttpRange { + start: 500, + length: 201, + }, + HttpRange { + start: 601, + length: 399, + }, + ], + ), + // Match Apache laxity: + T( + "bytes= 1 -2 , 4- 5, 7 - 8 , ,,", + 11, + vec![ + HttpRange { + start: 1, + length: 2, + }, + HttpRange { + start: 4, + length: 2, + }, + HttpRange { + start: 7, + length: 2, + }, + ], + ), + ]; + + for t in tests { + let header = t.0; + let size = t.1; + let expected = t.2; + + let res = HttpRange::parse(header, size); + + if res.is_err() { + if expected.is_empty() { + continue; + } else { + assert!( + false, + "parse({}, {}) returned error {:?}", + header, + size, + res.unwrap_err() + ); + } + } + + let got = res.unwrap(); + + if got.len() != expected.len() { + assert!( + false, + "len(parseRange({}, {})) = {}, want {}", + header, + size, + got.len(), + expected.len() + ); + continue; + } + + for i in 0..expected.len() { + if got[i].start != expected[i].start { + assert!( + false, + "parseRange({}, {})[{}].start = {}, want {}", + header, size, i, got[i].start, expected[i].start + ) + } + if got[i].length != expected[i].length { + assert!( + false, + "parseRange({}, {})[{}].length = {}, want {}", + header, size, i, got[i].length, expected[i].length + ) + } + } + } + } } diff --git a/src/httpmessage.rs b/src/httpmessage.rs index 1ce04b477..cac82f04c 100644 --- a/src/httpmessage.rs +++ b/src/httpmessage.rs @@ -5,15 +5,13 @@ use encoding::types::{DecoderTrap, Encoding}; use encoding::EncodingRef; use futures::{Async, Future, Poll, Stream}; use http::{header, HeaderMap}; -use http_range::HttpRange; use mime::Mime; use serde::de::DeserializeOwned; use serde_urlencoded; use std::str; use error::{ - ContentTypeError, HttpRangeError, ParseError, PayloadError, ReadlinesError, - UrlencodedError, + ContentTypeError, ParseError, PayloadError, ReadlinesError, UrlencodedError, }; use header::Header; use json::JsonBody; @@ -95,17 +93,6 @@ pub trait HttpMessage { } } - /// Parses Range HTTP header string as per RFC 2616. - /// `size` is full size of response (file). - fn range(&self, size: u64) -> Result, HttpRangeError> { - if let Some(range) = self.headers().get(header::RANGE) { - HttpRange::parse(unsafe { str::from_utf8_unchecked(range.as_bytes()) }, size) - .map_err(|e| e.into()) - } else { - Ok(Vec::new()) - } - } - /// Load http message body. /// /// By default only 256Kb payload reads to a memory, then @@ -637,22 +624,6 @@ mod tests { ); } - #[test] - fn test_no_request_range_header() { - let req = HttpRequest::default(); - let ranges = req.range(100).unwrap(); - assert!(ranges.is_empty()); - } - - #[test] - fn test_request_range_header() { - let req = TestRequest::with_header(header::RANGE, "bytes=0-4").finish(); - let ranges = req.range(100).unwrap(); - assert_eq!(ranges.len(), 1); - assert_eq!(ranges[0].start, 0); - assert_eq!(ranges[0].length, 5); - } - #[test] fn test_chunked() { let req = HttpRequest::default(); diff --git a/src/lib.rs b/src/lib.rs index b3e143a5c..6e8cc2462 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -105,7 +105,6 @@ extern crate futures; extern crate cookie; extern crate futures_cpupool; extern crate http as modhttp; -extern crate http_range; extern crate httparse; extern crate language_tags; extern crate libc; @@ -258,7 +257,6 @@ pub mod http { pub use modhttp::{uri, Error, Extensions, HeaderMap, HttpTryFrom, Uri}; pub use cookie::{Cookie, CookieBuilder}; - pub use http_range::HttpRange; pub use helpers::NormalizePath;