From c5d661027611a08a8ca2d4698128cb63397d8362 Mon Sep 17 00:00:00 2001 From: "Aode (lion)" Date: Wed, 3 Nov 2021 12:04:59 -0500 Subject: [PATCH] Properly handle out-of-bounds range requests --- src/main.rs | 92 ++++++++++++++++++++++++---------------------------- src/range.rs | 64 +++++++++++++++++++++--------------- 2 files changed, 79 insertions(+), 77 deletions(-) diff --git a/src/main.rs b/src/main.rs index d49d56c..1a4fb6b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -5,7 +5,10 @@ use actix_web::{ web, App, HttpResponse, HttpResponseBuilder, HttpServer, }; use awc::Client; -use futures_util::{stream::once, Stream}; +use futures_util::{ + stream::{empty, once}, + Stream, +}; use once_cell::sync::{Lazy, OnceCell}; use std::{ collections::HashSet, @@ -437,40 +440,34 @@ where let (details, bytes) = CancelSafeProcessor::new(thumbnail_path.clone(), process_fut).await?; - match range { - Some(range_header) => { - if !range_header.is_bytes() { - return Err(UploadError::Range.into()); - } - - if range_header.is_empty() { - Err(UploadError::Range.into()) - } else if range_header.len() == 1 { - let range = range_header.ranges().next().unwrap(); - let content_range = range.to_content_range(bytes.len() as u64); - let stream = range.chop_bytes(bytes); + let (builder, stream) = if let Some(range_header) = range { + if let Some(range) = range_header.single_bytes_range() { + if let Some(content_range) = range.to_content_range(bytes.len() as u64) { let mut builder = HttpResponse::PartialContent(); builder.insert_header(content_range); + let stream = range.chop_bytes(bytes); - Ok(srv_response( - builder, - stream, - details.content_type(), - 7 * DAYS, - details.system_time(), - )) + (builder, Either::left(Either::left(stream))) } else { - Err(UploadError::Range.into()) + ( + HttpResponse::RangeNotSatisfiable(), + Either::left(Either::right(empty())), + ) } + } else { + return Err(UploadError::Range.into()); } - None => Ok(srv_response( - HttpResponse::Ok(), - once(ready(Ok(bytes) as Result<_, Error>)), - details.content_type(), - 7 * DAYS, - details.system_time(), - )), - } + } else { + (HttpResponse::Ok(), Either::right(once(ready(Ok(bytes))))) + }; + + Ok(srv_response( + builder, + stream, + details.content_type(), + 7 * DAYS, + details.system_time(), + )) } /// Fetch file details @@ -541,39 +538,34 @@ async fn ranged_file_resp( where Error: From, { - let (builder, stream) = match range { + let (builder, stream) = if let Some(range_header) = range { //Range header exists - return as ranged - Some(range_header) => { - if !range_header.is_bytes() { - return Err(UploadError::Range.into()); - } - - if range_header.is_empty() { - return Err(UploadError::Range.into()); - } else if range_header.len() == 1 { - let len = store.len(&identifier).await?; - - let range = range_header.ranges().next().unwrap(); + if let Some(range) = range_header.single_bytes_range() { + let len = store.len(&identifier).await?; + if let Some(content_range) = range.to_content_range(len) { let mut builder = HttpResponse::PartialContent(); - builder.insert_header(range.to_content_range(len)); + builder.insert_header(content_range); ( builder, - Either::left(map_error::map_crate_error( + Either::left(Either::left(map_error::map_crate_error( range.chop_store(store, identifier).await?, - )), + ))), ) } else { - return Err(UploadError::Range.into()); + ( + HttpResponse::RangeNotSatisfiable(), + Either::left(Either::right(empty())), + ) } + } else { + return Err(UploadError::Range.into()); } + } else { //No Range header in the request - return the entire document - None => { - let stream = - map_error::map_crate_error(store.to_stream(&identifier, None, None).await?); - (HttpResponse::Ok(), Either::right(stream)) - } + let stream = map_error::map_crate_error(store.to_stream(&identifier, None, None).await?); + (HttpResponse::Ok(), Either::right(stream)) }; Ok(srv_response( diff --git a/src/range.rs b/src/range.rs index 7fc20b7..481533f 100644 --- a/src/range.rs +++ b/src/range.rs @@ -28,20 +28,38 @@ pub(crate) struct RangeHeader { } impl Range { - pub(crate) fn to_content_range(&self, instance_length: u64) -> ContentRange { + pub(crate) fn to_content_range(&self, instance_length: u64) -> Option { match self { - Range::Start(start) => ContentRange(ContentRangeSpec::Bytes { - range: Some((*start, instance_length)), - instance_length: Some(instance_length), - }), - Range::SuffixLength(from_start) => ContentRange(ContentRangeSpec::Bytes { - range: Some((0, *from_start)), - instance_length: Some(instance_length), - }), - Range::Segment(start, end) => ContentRange(ContentRangeSpec::Bytes { - range: Some((*start, *end)), - instance_length: Some(instance_length), - }), + Range::Start(start) => { + if *start >= instance_length { + return None; + } + + Some(ContentRange(ContentRangeSpec::Bytes { + range: Some((*start, instance_length - *start)), + instance_length: Some(instance_length), + })) + } + Range::SuffixLength(from_start) => { + if *from_start > instance_length { + return None; + } + + Some(ContentRange(ContentRangeSpec::Bytes { + range: Some((0, *from_start)), + instance_length: Some(instance_length), + })) + } + Range::Segment(start, end) => { + if *start >= instance_length || *end > instance_length { + return None; + } + + Some(ContentRange(ContentRangeSpec::Bytes { + range: Some((*start, *end)), + instance_length: Some(instance_length), + })) + } } } @@ -76,20 +94,12 @@ impl Range { } impl RangeHeader { - pub(crate) fn is_bytes(&self) -> bool { - self.unit == "bytes" - } - - pub(crate) fn ranges(&self) -> impl Iterator + '_ { - self.ranges.iter() - } - - pub(crate) fn len(&self) -> usize { - self.ranges.len() - } - - pub(crate) fn is_empty(&self) -> bool { - self.ranges.is_empty() + pub(crate) fn single_bytes_range(&self) -> Option<&'_ Range> { + if self.ranges.len() == 1 && self.unit == "bytes" { + self.ranges.iter().next() + } else { + None + } } }