aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authortrinity-1686a <trinity.pointard@gmail.com>2021-11-29 11:52:42 +0100
committerAlex <alex@adnab.me>2021-11-29 11:52:42 +0100
commit7f26ed55cdad4a67300447cf92bf8e4975a5c978 (patch)
tree4b19ae34f53351bc4850a6c3ab2c04dfe87f10d3
parent8811bb08e6d5eb024bacdfbb20d039c6b696e1a6 (diff)
downloadgarage-7f26ed55cdad4a67300447cf92bf8e4975a5c978.tar.gz
garage-7f26ed55cdad4a67300447cf92bf8e4975a5c978.zip
Improved handling of HTTP ranges
- correct HTTP code when range syntax is invalid (fix #140) - when multiple ranges are given, simply ignore and send whole file Co-authored-by: Trinity Pointard <trinity.pointard@gmail.com> Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/157 Reviewed-by: Alex <alex@adnab.me> Co-authored-by: trinity-1686a <trinity.pointard@gmail.com> Co-committed-by: trinity-1686a <trinity.pointard@gmail.com>
-rw-r--r--src/api/api_server.rs11
-rw-r--r--src/api/error.rs24
-rw-r--r--src/api/s3_get.rs13
-rw-r--r--src/web/error.rs11
-rw-r--r--src/web/web_server.rs1
5 files changed, 46 insertions, 14 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 a57f926b..4ad8ef82 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)]
@@ -90,6 +93,7 @@ impl Error {
Error::InternalError(_) | Error::Hyper(_) | Error::Http(_) => {
StatusCode::INTERNAL_SERVER_ERROR
}
+ Error::InvalidRange(_) => StatusCode::RANGE_NOT_SATISFIABLE,
_ => StatusCode::BAD_REQUEST,
}
}
@@ -127,6 +131,22 @@ impl Error {
.into()
})
}
+
+ pub fn add_headers(&self, header_map: &mut HeaderMap<HeaderValue>) {
+ use hyper::header;
+ #[allow(clippy::single_match)]
+ 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 10d6591f..428bbf34 100644
--- a/src/api/s3_get.rs
+++ b/src/api/s3_get.rs
@@ -156,11 +156,12 @@ 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 {
- 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 +236,6 @@ async fn handle_get_range(
begin: u64,
end: u64,
) -> Result<Response<Body>, 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(
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<HeaderValue>) {
+ #[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<Body> {
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
}