From 8c45ad8e38094540d27394fd608c1c8a1e004ee4 Mon Sep 17 00:00:00 2001 From: Trinity Pointard Date: Fri, 19 Nov 2021 20:17:35 +0100 Subject: [PATCH 1/4] map range-error to the right http error code --- src/api/error.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/api/error.rs b/src/api/error.rs index a57f926b..a86d53df 100644 --- a/src/api/error.rs +++ b/src/api/error.rs @@ -90,6 +90,7 @@ impl Error { Error::InternalError(_) | Error::Hyper(_) | Error::Http(_) => { StatusCode::INTERNAL_SERVER_ERROR } + Error::InvalidRange(_) => StatusCode::RANGE_NOT_SATISFIABLE, _ => StatusCode::BAD_REQUEST, } } -- 2.43.0 From 991279cd4073aa9b30b79aecccc88da1962e504e Mon Sep 17 00:00:00 2001 From: Trinity Pointard Date: Thu, 25 Nov 2021 18:30:33 +0100 Subject: [PATCH 2/4] remove dead code and handle mutli-range by sending whole file --- src/api/s3_get.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/api/s3_get.rs b/src/api/s3_get.rs index 10d6591f..0f6feaed 100644 --- a/src/api/s3_get.rs +++ b/src/api/s3_get.rs @@ -158,9 +158,9 @@ pub async fn handle_get( let range_str = range.to_str()?; let mut ranges = http_range::HttpRange::parse(range_str, last_v_meta.size)?; if ranges.len() > 1 { - return Err(Error::BadRequest( - "Multiple ranges not supported".to_string(), - )); + // garage does not support multi-range requests yet, so we respond with the entire + // object when multiple ranges are requested + None } else { ranges.pop() } @@ -235,10 +235,6 @@ async fn handle_get_range( begin: u64, end: u64, ) -> Result, Error> { - if end > version_meta.size { - return Err(Error::BadRequest("Range not included in file".to_string())); - } - let resp_builder = object_headers(version, version_meta) .header("Content-Length", format!("{}", end - begin)) .header( -- 2.43.0 From 36e104b66558c51c5f9062f8f85028c487c6d9d4 Mon Sep 17 00:00:00 2001 From: Trinity Pointard Date: Sat, 27 Nov 2021 11:42:34 +0100 Subject: [PATCH 3/4] send content-range on invalid range error --- src/api/api_server.rs | 11 ++++++++--- src/api/error.rs | 22 ++++++++++++++++++++-- src/api/s3_get.rs | 3 ++- 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/api/api_server.rs b/src/api/api_server.rs index 74142453..240ffca9 100644 --- a/src/api/api_server.rs +++ b/src/api/api_server.rs @@ -65,10 +65,15 @@ async fn handler( } Err(e) => { let body: Body = Body::from(e.aws_xml(&garage.config.s3_api.s3_region, uri.path())); - let http_error = Response::builder() + let mut http_error_builder = Response::builder() .status(e.http_status_code()) - .header("Content-Type", "application/xml") - .body(body)?; + .header("Content-Type", "application/xml"); + + if let Some(header_map) = http_error_builder.headers_mut() { + e.add_headers(header_map) + } + + let http_error = http_error_builder.body(body)?; if e.http_status_code().is_server_error() { warn!("Response: error {}, {}", e.http_status_code(), e); diff --git a/src/api/error.rs b/src/api/error.rs index a86d53df..3be7d682 100644 --- a/src/api/error.rs +++ b/src/api/error.rs @@ -1,5 +1,8 @@ +use std::convert::TryInto; + use err_derive::Error; -use hyper::StatusCode; +use hyper::header::HeaderValue; +use hyper::{HeaderMap, StatusCode}; use garage_util::error::Error as GarageError; @@ -57,7 +60,7 @@ pub enum Error { /// The client sent a range header with invalid value #[error(display = "Invalid HTTP range: {:?}", _0)] - InvalidRange(#[error(from)] http_range::HttpRangeParseError), + InvalidRange(#[error(from)] (http_range::HttpRangeParseError, u64)), /// The client sent an invalid request #[error(display = "Bad request: {}", _0)] @@ -128,6 +131,21 @@ impl Error { .into() }) } + + pub fn add_headers(&self, header_map: &mut HeaderMap) { + use hyper::header; + match self { + Error::InvalidRange((_, len)) => { + header_map.append( + header::CONTENT_RANGE, + format!("bytes */{}", len) + .try_into() + .expect("header value only contain ascii"), + ); + } + _ => (), + } + } } /// Trait to map error to the Bad Request error code diff --git a/src/api/s3_get.rs b/src/api/s3_get.rs index 0f6feaed..428bbf34 100644 --- a/src/api/s3_get.rs +++ b/src/api/s3_get.rs @@ -156,7 +156,8 @@ pub async fn handle_get( let range = match req.headers().get("range") { Some(range) => { let range_str = range.to_str()?; - let mut ranges = http_range::HttpRange::parse(range_str, last_v_meta.size)?; + let mut ranges = http_range::HttpRange::parse(range_str, last_v_meta.size) + .map_err(|e| (e, last_v_meta.size))?; if ranges.len() > 1 { // garage does not support multi-range requests yet, so we respond with the entire // object when multiple ranges are requested -- 2.43.0 From 8a43ede301bafda25346d71c24fa23d1d0c78f57 Mon Sep 17 00:00:00 2001 From: Trinity Pointard Date: Sat, 27 Nov 2021 15:56:02 +0100 Subject: [PATCH 4/4] add range header on 416 on web request --- src/api/error.rs | 1 + src/web/error.rs | 11 ++++++++++- src/web/web_server.rs | 1 + 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/api/error.rs b/src/api/error.rs index 3be7d682..4ad8ef82 100644 --- a/src/api/error.rs +++ b/src/api/error.rs @@ -134,6 +134,7 @@ impl Error { pub fn add_headers(&self, header_map: &mut HeaderMap) { use hyper::header; + #[allow(clippy::single_match)] match self { Error::InvalidRange((_, len)) => { header_map.append( diff --git a/src/web/error.rs b/src/web/error.rs index 426155c1..55990e9d 100644 --- a/src/web/error.rs +++ b/src/web/error.rs @@ -1,5 +1,6 @@ use err_derive::Error; -use hyper::StatusCode; +use hyper::header::HeaderValue; +use hyper::{HeaderMap, StatusCode}; use garage_util::error::Error as GarageError; @@ -47,4 +48,12 @@ impl Error { _ => StatusCode::BAD_REQUEST, } } + + pub fn add_headers(&self, header_map: &mut HeaderMap) { + #[allow(clippy::single_match)] + match self { + Error::ApiError(e) => e.add_headers(header_map), + _ => (), + } + } } diff --git a/src/web/web_server.rs b/src/web/web_server.rs index e9c5039d..4a603c05 100644 --- a/src/web/web_server.rs +++ b/src/web/web_server.rs @@ -62,6 +62,7 @@ fn error_to_res(e: Error) -> Response { let body: Body = Body::from(format!("{}\n", e)); let mut http_error = Response::new(body); *http_error.status_mut() = e.http_status_code(); + e.add_headers(http_error.headers_mut()); http_error } -- 2.43.0