🐛 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

@@ -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/*"]

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]

crates/always_equal/src/lib.rs

@@ -0,0 +1,146 @@
+pub mod test {
+	#[derive (Debug)]
+	pub struct AlwaysEqual <T> {
+		inner: Option <T>,
+	}
+
+	impl <T> AlwaysEqual <T> {
+		pub fn into_inner (self) -> T {
+			match self.inner {
+				Some (x) => x,
+				None => unreachable! (),
+			}
+		}
+		
+		pub fn testing_blank () -> Self {
+			Self {
+				inner: None,
+			}
+		}
+	}
+
+	impl <T: Default> Default for AlwaysEqual <T> {
+		fn default () -> Self {
+			Self::from (T::default ())
+		}
+	}
+
+	impl <T> From <T> for AlwaysEqual <T> {
+		fn from (inner: T) -> Self {
+			Self {
+				inner: Some (inner),
+			}
+		}
+	}
+
+	impl <T> PartialEq for AlwaysEqual <T> {
+		fn eq (&self, other: &Self) -> bool {
+			self.inner.is_none () || other.inner.is_none ()
+		}
+	}
+}
+
+pub mod prod {
+	use std::fmt;
+	
+	pub struct AlwaysEqual <T> {
+		inner: T,
+	}
+	
+	impl <T: fmt::Debug> fmt::Debug for AlwaysEqual <T> {
+		fn fmt (&self, f: &mut fmt::Formatter) -> fmt::Result {
+			self.inner.fmt (f)
+		}
+	}
+	
+	impl <T: fmt::Display> fmt::Display for AlwaysEqual <T> {
+		fn fmt (&self, f: &mut fmt::Formatter) -> fmt::Result {
+			self.inner.fmt (f)
+		}
+	}
+	
+	impl <T> AlwaysEqual <T> {
+		pub fn into_inner (self) -> T {
+			self.inner
+		}
+	}
+	
+	impl <T> From <T> for AlwaysEqual <T> {
+		fn from (inner: T) -> Self {
+			Self {
+				inner,
+			}
+		}
+	}
+	
+	impl <T> PartialEq for AlwaysEqual <T> {
+		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 <File>;
+
+	#[derive (Debug, Default, PartialEq)]
+	struct MyStruct
+	{
+		file: AlwaysEqual <CantCompare>,
+		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::<AlwaysEqual <File>>::new ();
+		let v2 = Vec::<AlwaysEqual <File>>::new ();
+		
+		assert_eq! (v1, v2);
+	}
+}

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 <StatusCode> 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 <StatusCode> for hyper::StatusCode {
 			StatusCode::Forbidden => Self::FORBIDDEN,
 			StatusCode::NotFound => Self::NOT_FOUND,
 			StatusCode::MethodNotAllowed => Self::METHOD_NOT_ALLOWED,
+			StatusCode::RangeNotSatisfiable => Self::RANGE_NOT_SATISFIABLE,
 		}
 	}
 }

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 <u64>, Option <u64>) {
 	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 <u64>),
+	PartialContent (Range <u64>),
+	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 <u64>,
+	range: Range <u64>,
-	range_end: Option <u64>
+	range_requested: bool
 ) 
 	-> Response 
 {
@@ -241,18 +292,11 @@ async fn serve_file (
 		None
 	};
 	
-	let file_md = f.metadata ().await.unwrap ();
+	f.seek (SeekFrom::Start (range.start)).await.unwrap ();
-	let file_len = file_md.len ();
 	
-	let start = range_start.unwrap_or (0);
+	info! ("Serving range {}-{}", range.start, range.end);
-	let end = range_end.unwrap_or (file_len);
 	
-	let start = max (0, min (start, file_len));
+	let content_length = range.end - range.start;
-	let end = max (0, min (end, file_len));
-	
-	f.seek (SeekFrom::Start (start)).await.unwrap ();
-	
-	info! ("Serving range {}-{}", start, end);
 	
 	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 () {
+		if range_requested {
-			response.status_code (StatusCode::Ok);
+			response.status_code (StatusCode::PartialContent);
-			response.header (String::from ("content-length"), end.to_string ().into_bytes ());
+			response.header (String::from ("content-range"), format! ("bytes {}-{}/{}", range.start, range.end - 1, range.end).into_bytes ());
 		}
 		else {
-			response.status_code (StatusCode::PartialContent);
+			response.status_code (StatusCode::Ok);
-			response.header (String::from ("content-range"), format! ("bytes {}-{}/{}", start, end - 1, end).into_bytes ());
+			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))]
+// Sort of an internal API endpoint to make testing work better.
-pub async fn serve_all (
+// Eventually we could expose this as JSON or Msgpack or whatever. For now
-	handlebars: &Handlebars <'static>,
+// 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 <ReadDir>,
+}
+
+#[derive (Debug, PartialEq)]
+struct ServeFileParams {
+	send_body: bool,
+	range: Range <u64>,
+	range_requested: bool,
+	file: AlwaysEqual <File>,
+}
+
+#[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 <String, Vec <u8>>,
 	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 => {
+		None => return Root,
-			return serve_root (handlebars).await;
-		},
 	};
 	
+	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 (
+		let dir = dir.into ();
-			handlebars, 
+		
-			full_path.to_string_lossy (),
+		ServeDir (ServeDirParams {
-			dir
+			dir,
-		).await
+			path: full_path,
+		})
 	}
 	else if let Ok (file) = File::open (&full_path).await {
-		let mut range_start = None;
+		let file_md = file.metadata ().await.unwrap ();
-		let mut range_end = None;
+		let file_len = file_md.len ();
 		
-		if let Some (v) = headers.get ("range") {
+		let range_header = headers.get ("range").map (|v| std::str::from_utf8 (v).ok ()).flatten ();
-			let v = std::str::from_utf8 (v).unwrap ();
 		
-			let (start, end) = parse_range_header (v);
+		let file = file.into ();
-			range_start = start;
-			range_end = end;
-		}
 		
-		let should_send_body = match &method {
+		match check_range (range_header, file_len) {
-			Method::Get => true,
+			ParsedRange::RangeNotSatisfiable (file_len) => RangeNotSatisfiable (file_len),
-			Method::Head => false,
+			ParsedRange::Ok (range) => ServeFile (ServeFileParams {
-			m => {
-				debug! ("Unsupported method {:?}", m);
-				return serve_error (StatusCode::MethodNotAllowed, "Unsupported method");
-			}
-		};
-		
-		serve_file (
 				file, 
-			should_send_body,
+				send_body, 
-			range_start,
+				range,
-			range_end
+				range_requested: false,
-		).await
+			}),
+			ParsedRange::PartialContent (range) => ServeFile (ServeFileParams {
+				file,
+				send_body, 
+				range,
+				range_requested: true,
+			}),
+		}
 	}
 	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 <String, Vec <u8>>,
+	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));
+			}
 		});
 	}
 }

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