From 2b93aa8b8370a9115103b3de18c5156de2e00a57 Mon Sep 17 00:00:00 2001 From: _ <> Date: Sun, 8 Nov 2020 18:12:45 +0000 Subject: [PATCH] :bug: Implement 416 Range Not Satisfiable I had a lot of trouble getting AlwaysEqual to compile, so I tested it in a completely separate crate and vendored it back into PTTH. It's also AGPLv3. --- Cargo.toml | 6 + crates/always_equal/Cargo.toml | 9 + crates/always_equal/src/lib.rs | 146 ++++++++++++++ src/http_serde.rs | 4 + src/server/file_server.rs | 344 ++++++++++++++++++++++++++------- todo.md | 2 + 6 files changed, 445 insertions(+), 66 deletions(-) create mode 100644 crates/always_equal/Cargo.toml create mode 100644 crates/always_equal/src/lib.rs diff --git a/Cargo.toml b/Cargo.toml index 6972530..6cf0f5b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,3 +34,9 @@ tracing-futures = "0.2.4" tracing-subscriber = "0.2.15" toml = "0.5.7" ulid = "0.4.1" + +always_equal = { path = "crates/always_equal" } + +[workspace] + +members = ["crates/*"] diff --git a/crates/always_equal/Cargo.toml b/crates/always_equal/Cargo.toml new file mode 100644 index 0000000..fdf764a --- /dev/null +++ b/crates/always_equal/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "always_equal" +version = "0.1.0" +authors = ["_"] +edition = "2018" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] diff --git a/crates/always_equal/src/lib.rs b/crates/always_equal/src/lib.rs new file mode 100644 index 0000000..2e15641 --- /dev/null +++ b/crates/always_equal/src/lib.rs @@ -0,0 +1,146 @@ +pub mod test { + #[derive (Debug)] + pub struct AlwaysEqual { + inner: Option , + } + + impl AlwaysEqual { + pub fn into_inner (self) -> T { + match self.inner { + Some (x) => x, + None => unreachable! (), + } + } + + pub fn testing_blank () -> Self { + Self { + inner: None, + } + } + } + + impl Default for AlwaysEqual { + fn default () -> Self { + Self::from (T::default ()) + } + } + + impl From for AlwaysEqual { + fn from (inner: T) -> Self { + Self { + inner: Some (inner), + } + } + } + + impl PartialEq for AlwaysEqual { + fn eq (&self, other: &Self) -> bool { + self.inner.is_none () || other.inner.is_none () + } + } +} + +pub mod prod { + use std::fmt; + + pub struct AlwaysEqual { + inner: T, + } + + impl fmt::Debug for AlwaysEqual { + fn fmt (&self, f: &mut fmt::Formatter) -> fmt::Result { + self.inner.fmt (f) + } + } + + impl fmt::Display for AlwaysEqual { + fn fmt (&self, f: &mut fmt::Formatter) -> fmt::Result { + self.inner.fmt (f) + } + } + + impl AlwaysEqual { + pub fn into_inner (self) -> T { + self.inner + } + } + + impl From for AlwaysEqual { + fn from (inner: T) -> Self { + Self { + inner, + } + } + } + + impl PartialEq for AlwaysEqual { + fn eq (&self, _other: &Self) -> bool { + false + } + } +} + +#[cfg (test)] +mod tests { + use std::fs::File; + + use super::test::*; + + // Can't impl Clone or PartialEq because of the File + type CantCompare = Option ; + + #[derive (Debug, Default, PartialEq)] + struct MyStruct + { + file: AlwaysEqual , + name: &'static str, + } + + #[test] + fn test_1 () { + let concrete_1 = MyStruct { + file: None.into (), + name: "my_struct", + }; + let concrete_2 = MyStruct { + file: None.into (), + name: "my_struct", + }; + let concrete_bad = MyStruct { + file: None.into (), + name: "not_my_struct", + }; + + assert_ne! (concrete_1, concrete_2); + assert_ne! (concrete_2, concrete_bad); + assert_ne! (concrete_bad, concrete_1); + + let dummy_1 = MyStruct { + file: AlwaysEqual::testing_blank (), + name: "my_struct", + }; + let dummy_2 = MyStruct { + file: AlwaysEqual::testing_blank (), + name: "my_struct", + }; + let dummy_bad = MyStruct { + file: AlwaysEqual::testing_blank (), + name: "not_my_struct", + }; + + assert_eq! (dummy_1, dummy_2); + assert_ne! (dummy_2, dummy_bad); + assert_ne! (dummy_bad, dummy_1); + + assert_eq! (concrete_1, dummy_1); + assert_eq! (concrete_bad, dummy_bad); + } + + #[test] + fn test_2 () { + let v1 = Vec::>::new (); + let v2 = Vec::>::new (); + + assert_eq! (v1, v2); + } +} diff --git a/src/http_serde.rs b/src/http_serde.rs index 52aae59..a39e131 100644 --- a/src/http_serde.rs +++ b/src/http_serde.rs @@ -88,6 +88,7 @@ pub struct WrappedRequest { #[derive (Debug, Deserialize, Serialize, PartialEq)] pub enum StatusCode { Ok, // 200 + NoContent, // 204 PartialContent, // 206 TemporaryRedirect, // 307 @@ -96,6 +97,7 @@ pub enum StatusCode { Forbidden, // 403 NotFound, // 404 MethodNotAllowed, // 405 + RangeNotSatisfiable, // 416 } impl Default for StatusCode { @@ -108,6 +110,7 @@ impl From for hyper::StatusCode { fn from (x: StatusCode) -> Self { match x { StatusCode::Ok => Self::OK, + StatusCode::NoContent => Self::NO_CONTENT, StatusCode::PartialContent => Self::PARTIAL_CONTENT, StatusCode::TemporaryRedirect => Self::TEMPORARY_REDIRECT, @@ -116,6 +119,7 @@ impl From for hyper::StatusCode { StatusCode::Forbidden => Self::FORBIDDEN, StatusCode::NotFound => Self::NOT_FOUND, StatusCode::MethodNotAllowed => Self::METHOD_NOT_ALLOWED, + StatusCode::RangeNotSatisfiable => Self::RANGE_NOT_SATISFIABLE, } } } diff --git a/src/server/file_server.rs b/src/server/file_server.rs index 65f18ed..d602c19 100644 --- a/src/server/file_server.rs +++ b/src/server/file_server.rs @@ -2,15 +2,17 @@ use std::{ borrow::Cow, - cmp::{min, max}, + cmp::min, collections::*, convert::{Infallible, TryInto}, error::Error, + fmt::Debug, io::SeekFrom, path::{Path, PathBuf}, }; use handlebars::Handlebars; +use percent_encoding::*; use serde::Serialize; use tokio::{ fs::{ @@ -28,6 +30,12 @@ use tracing::instrument; use regex::Regex; +#[cfg (test)] +use always_equal::test::AlwaysEqual; + +#[cfg (not (test))] +use always_equal::prod::AlwaysEqual; + use crate::{ http_serde::{ Method, @@ -90,11 +98,54 @@ fn parse_range_header (range_str: &str) -> (Option , Option ) { let end = caps.get (2).map (|x| x.as_str ()); let start = start.map (|x| u64::from_str_radix (x, 10).ok ()).flatten (); - let end = end.map (|x| u64::from_str_radix (x, 10).ok ()).flatten (); + + // HTTP specifies ranges as [start inclusive, end inclusive] + // But that's dumb and [start inclusive, end exclusive) is better + + let end = end.map (|x| u64::from_str_radix (x, 10).ok ().map (|x| x + 1)).flatten (); (start, end) } +use std::ops::Range; + +#[derive (Debug, PartialEq)] +enum ParsedRange { + Ok (Range ), + PartialContent (Range ), + RangeNotSatisfiable (u64), +} + +fn check_range (range_str: Option <&str>, file_len: u64) + -> ParsedRange +{ + use ParsedRange::*; + + let not_satisfiable = RangeNotSatisfiable (file_len); + + let range_str = match range_str { + None => return Ok (0..file_len), + Some (x) => x, + }; + + let (start, end) = parse_range_header (range_str); + + let start = start.unwrap_or (0); + if start >= file_len { + return not_satisfiable; + } + + let end = end.unwrap_or (file_len); + if end > file_len { + return not_satisfiable; + } + if end < start { + return not_satisfiable; + } + + PartialContent (start..end) +} + async fn read_dir_entry (entry: DirEntry) -> TemplateDirEntry { let file_name = match entry.file_name ().into_string () { @@ -228,8 +279,8 @@ async fn serve_dir ( async fn serve_file ( mut f: File, should_send_body: bool, - range_start: Option , - range_end: Option + range: Range , + range_requested: bool ) -> Response { @@ -241,18 +292,11 @@ async fn serve_file ( None }; - let file_md = f.metadata ().await.unwrap (); - let file_len = file_md.len (); + f.seek (SeekFrom::Start (range.start)).await.unwrap (); - let start = range_start.unwrap_or (0); - let end = range_end.unwrap_or (file_len); + info! ("Serving range {}-{}", range.start, range.end); - let start = max (0, min (start, file_len)); - let end = max (0, min (end, file_len)); - - f.seek (SeekFrom::Start (start)).await.unwrap (); - - info! ("Serving range {}-{}", start, end); + let content_length = range.end - range.start; if should_send_body { tokio::spawn (async move { @@ -260,7 +304,7 @@ async fn serve_file ( let mut tx = tx; let mut bytes_sent = 0; - let mut bytes_left = end - start; + let mut bytes_left = content_length; loop { let mut buffer = vec! [0u8; 65_536]; @@ -275,7 +319,7 @@ async fn serve_file ( } if tx.send (Ok::<_, Infallible> (buffer)).await.is_err () { - warn! ("Cancelling file stream (Sent {} out of {} bytes)", bytes_sent, end - start); + warn! ("Cancelling file stream (Sent {} out of {} bytes)", bytes_sent, content_length); break; } @@ -298,16 +342,19 @@ async fn serve_file ( response.header (String::from ("accept-ranges"), b"bytes".to_vec ()); if should_send_body { - if range_start.is_none () && range_end.is_none () { - response.status_code (StatusCode::Ok); - response.header (String::from ("content-length"), end.to_string ().into_bytes ()); + if range_requested { + response.status_code (StatusCode::PartialContent); + response.header (String::from ("content-range"), format! ("bytes {}-{}/{}", range.start, range.end - 1, range.end).into_bytes ()); } else { - response.status_code (StatusCode::PartialContent); - response.header (String::from ("content-range"), format! ("bytes {}-{}/{}", start, end - 1, end).into_bytes ()); + response.status_code (StatusCode::Ok); + response.header (String::from ("content-length"), range.end.to_string ().into_bytes ()); } - response.content_length = Some (end - start); + response.content_length = Some (content_length); + } + else { + response.status_code (StatusCode::NoContent); } if let Some (body) = body { @@ -337,32 +384,73 @@ fn serve_307 (location: String) -> Response { resp } -#[instrument (level = "debug", skip (handlebars, headers))] -pub async fn serve_all ( - handlebars: &Handlebars <'static>, +// Sort of an internal API endpoint to make testing work better. +// Eventually we could expose this as JSON or Msgpack or whatever. For now +// it's just a Rust struct that we can test on without caring about +// human-readable HTML + +#[derive (Debug, PartialEq)] +struct ServeDirParams { + path: PathBuf, + dir: AlwaysEqual , +} + +#[derive (Debug, PartialEq)] +struct ServeFileParams { + send_body: bool, + range: Range , + range_requested: bool, + file: AlwaysEqual , +} + +#[derive (Debug, PartialEq)] +enum InternalResponse { + Favicon, + Forbidden, + MethodNotAllowed, + NotFound, + RangeNotSatisfiable (u64), + Redirect (String), + Root, + ServeDir (ServeDirParams), + ServeFile (ServeFileParams), +} + +async fn internal_serve_all ( root: &Path, method: Method, uri: &str, headers: &HashMap >, hidden_path: Option <&Path> ) --> Response + -> InternalResponse { + use InternalResponse::*; + info! ("Client requested {}", uri); - use percent_encoding::*; + let send_body = match &method { + Method::Get => true, + Method::Head => false, + m => { + debug! ("Unsupported method {:?}", m); + return MethodNotAllowed; + } + }; if uri == "/favicon.ico" { - return serve_error (StatusCode::NotFound, ""); + return Favicon; } let uri = match prefix_match (uri, "/files") { Some (x) => x, - None => { - return serve_root (handlebars).await; - }, + None => return Root, }; + if uri == "" { + return Redirect ("files/".to_string ()); + } + // TODO: There is totally a dir traversal attack in here somewhere let encoded_path = &uri [1..]; @@ -377,7 +465,7 @@ pub async fn serve_all ( if let Some (hidden_path) = hidden_path { if full_path == hidden_path { - return serve_error (StatusCode::Forbidden, "403 Forbidden"); + return Forbidden; } } @@ -385,45 +473,82 @@ pub async fn serve_all ( if let Ok (dir) = read_dir (&full_path).await { if ! has_trailing_slash { - return serve_307 (format! ("{}/", path.file_name ().unwrap ().to_str ().unwrap ())); + return Redirect (format! ("{}/", path.file_name ().unwrap ().to_str ().unwrap ())); } - serve_dir ( - handlebars, - full_path.to_string_lossy (), - dir - ).await + let dir = dir.into (); + + ServeDir (ServeDirParams { + dir, + path: full_path, + }) } else if let Ok (file) = File::open (&full_path).await { - let mut range_start = None; - let mut range_end = None; + let file_md = file.metadata ().await.unwrap (); + let file_len = file_md.len (); - if let Some (v) = headers.get ("range") { - let v = std::str::from_utf8 (v).unwrap (); - - let (start, end) = parse_range_header (v); - range_start = start; - range_end = end; + let range_header = headers.get ("range").map (|v| std::str::from_utf8 (v).ok ()).flatten (); + + let file = file.into (); + + match check_range (range_header, file_len) { + ParsedRange::RangeNotSatisfiable (file_len) => RangeNotSatisfiable (file_len), + ParsedRange::Ok (range) => ServeFile (ServeFileParams { + file, + send_body, + range, + range_requested: false, + }), + ParsedRange::PartialContent (range) => ServeFile (ServeFileParams { + file, + send_body, + range, + range_requested: true, + }), } - - let should_send_body = match &method { - Method::Get => true, - Method::Head => false, - m => { - debug! ("Unsupported method {:?}", m); - return serve_error (StatusCode::MethodNotAllowed, "Unsupported method"); - } - }; - - serve_file ( - file, - should_send_body, - range_start, - range_end - ).await } else { - serve_error (StatusCode::NotFound, "404 Not Found") + NotFound + } +} + +#[instrument (level = "debug", skip (handlebars, headers))] +pub async fn serve_all ( + handlebars: &Handlebars <'static>, + root: &Path, + method: Method, + uri: &str, + headers: &HashMap >, + hidden_path: Option <&Path> +) + -> Response +{ + use InternalResponse::*; + + match internal_serve_all (root, method, uri, headers, hidden_path).await { + Favicon => serve_error (StatusCode::NotFound, ""), + Forbidden => serve_error (StatusCode::Forbidden, "403 Forbidden"), + MethodNotAllowed => serve_error (StatusCode::MethodNotAllowed, "Unsupported method"), + NotFound => serve_error (StatusCode::NotFound, "404 Not Found"), + RangeNotSatisfiable (file_len) => { + let mut resp = Response::default (); + resp.status_code (StatusCode::RangeNotSatisfiable) + .header ("content-range".to_string (), format! ("bytes */{}", file_len).into_bytes ()); + resp + }, + Redirect (location) => serve_307 (location), + + Root => serve_root (handlebars).await, + ServeDir (ServeDirParams { + path, + dir, + }) => serve_dir (handlebars, path.to_string_lossy (), dir.into_inner ()).await, + ServeFile (ServeFileParams { + file, + send_body, + range, + range_requested, + }) => serve_file (file.into_inner (), send_body, range, range_requested).await, } } @@ -469,12 +594,52 @@ mod tests { }, }; + use maplit::*; use tokio::runtime::Runtime; + use always_equal::test::AlwaysEqual; + use crate::http_serde::{ StatusCode, }; + #[test] + fn parse_range_header () { + for (input, expected) in vec! [ + ("", (None, None)), + ("bytes=0-", (Some (0), None)), + ("bytes=0-999", (Some (0), Some (1000))), + ("bytes=111-999", (Some (111), Some (1000))), + ].into_iter () { + let actual = super::parse_range_header (input); + assert_eq! (actual, expected); + } + + use super::ParsedRange::*; + + for (header, file_len, expected) in vec! [ + (None, 0, Ok (0..0)), + (None, 1024, Ok (0..1024)), + + (Some (""), 0, RangeNotSatisfiable (0)), + (Some (""), 1024, PartialContent (0..1024)), + + (Some ("bytes=0-"), 1024, PartialContent (0..1024)), + (Some ("bytes=0-999"), 1024, PartialContent (0..1000)), + (Some ("bytes=0-1023"), 1024, PartialContent (0..1024)), + (Some ("bytes=111-999"), 1024, PartialContent (111..1000)), + (Some ("bytes=111-1023"), 1024, PartialContent (111..1024)), + (Some ("bytes=200-100"), 1024, RangeNotSatisfiable (1024)), + + (Some ("bytes=0-"), 512, PartialContent (0..512)), + (Some ("bytes=0-1023"), 512, RangeNotSatisfiable (512)), + (Some ("bytes=1000-1023"), 512, RangeNotSatisfiable (512)), + ].into_iter () { + let actual = super::check_range (header, file_len); + assert_eq! (actual, expected); + } + } + #[test] fn pretty_print_bytes () { for (input_after, expected_before, expected_after) in vec! [ @@ -521,23 +686,32 @@ mod tests { fn file_server () { use crate::{ http_serde::Method, - prelude::*, + //prelude::*, + }; + use super::{ + InternalResponse, + internal_serve_all, + load_templates, + serve_all, + ServeDirParams, + ServeFileParams, }; tracing_subscriber::fmt ().try_init ().ok (); let mut rt = Runtime::new ().unwrap (); rt.block_on (async { - let handlebars = super::load_templates ().unwrap (); + let handlebars = load_templates ().unwrap (); let file_server_root = PathBuf::from ("./"); let headers = Default::default (); for (uri_path, expected_status) in vec! [ ("/", StatusCode::Ok), + ("/files", StatusCode::TemporaryRedirect), ("/files/src", StatusCode::TemporaryRedirect), ("/files/src/", StatusCode::Ok), ].into_iter () { - let resp = super::serve_all ( + let resp = serve_all ( &handlebars, &file_server_root, Method::Get, @@ -548,6 +722,44 @@ mod tests { assert_eq! (resp.parts.status_code, expected_status); } + + { + use InternalResponse::*; + + for (uri_path, expected) in vec! [ + ("/", Root), + ("/files", Redirect ("files/".to_string ())), + ("/files/src", Redirect ("src/".to_string ())), + ("/files/src/bad_passwords.txt", ServeFile (ServeFileParams { + send_body: true, + range: 0..1_048_576, + range_requested: false, + file: AlwaysEqual::testing_blank (), + })), + ].into_iter () { + let resp = internal_serve_all ( + &file_server_root, + Method::Get, + uri_path, + &headers, + None + ).await; + + assert_eq! (resp, expected); + } + + let resp = internal_serve_all ( + &file_server_root, + Method::Get, + "/files/src/bad_passwords.txt", + &hashmap! { + "range".into () => b"bytes=0-2000000".to_vec (), + }, + None + ).await; + + assert_eq! (resp, RangeNotSatisfiable (1_048_576)); + } }); } } diff --git a/todo.md b/todo.md index 5f4fe46..1bdcf0e 100644 --- a/todo.md +++ b/todo.md @@ -1,6 +1,8 @@ - Not working behind Nginx (Works okay behind Caddy) - Reduce idle memory use? +- Flip match_prefix args +- Impl multi-range / multi-part byte serving - Allow spaces in server names - Deny unused HTTP methods for endpoints - ETag cache based on mtime