remove HttpMessage::range()

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
 

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"

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<HttpRangeParseError> 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();

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<P: AsRef<Path>>(path: P) -> io::Result<NamedFile> {
-        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(
+                None => {
-                    io::ErrorKind::InvalidInput,
+                    return Err(io::Error::new(
-                    "Provided path has no filename")),
+                        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![
+                parameters: vec![DispositionParam::Filename(
-                    DispositionParam::Filename(
+                    header::Charset::Ext("UTF-8".to_owned()),
-                        header::Charset::Ext("UTF-8".to_owned()),
+                    None,
-                        None,
+                    filename.as_bytes().to_vec(),
-                        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
+        resp.if_some(last_modified, |lm, resp| {
-            .if_some(last_modified, |lm, resp| {
+            resp.set(header::LastModified(lm));
-                resp.set(header::LastModified(lm));
+        }).if_some(etag, |etag, resp| {
-            })
+            resp.set(header::ETag(etag));
-            .if_some(etag, |etag, resp| {
+        });
-                resp.set(header::ETag(etag));
-            });
 
         resp.header(header::ACCEPT_RANGES, "bytes");
 
@@ -721,6 +725,101 @@ impl<S: 'static> Handler<S> for StaticFiles<S> {
     }
 }
 
+/// 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<Vec<HttpRange>, ()> {
+        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<Option<HttpRange>> = 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::<Result<_, _>>()?;
+
+        let ranges: Vec<HttpRange> = 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![
+            parameters: vec![DispositionParam::Filename(
-                DispositionParam::Filename(
+                header::Charset::Ext("UTF-8".to_owned()),
-                    header::Charset::Ext("UTF-8".to_owned()),
+                None,
-                    None,
+                "test.png".as_bytes().to_vec(),
-                    "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<HttpRange>);
+
+    #[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
+                    )
+                }
+            }
+        }
+    }
 }

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,
+    ContentTypeError, ParseError, PayloadError, ReadlinesError, UrlencodedError,
-    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<Vec<HttpRange>, 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();

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;