Deuxfleurs/garage
48
56
Fork 73
Code Issues 99 Pull requests 13 Releases 30 Wiki Activity

Improved handling of HTTP ranges
Some checks failed
continuous-integration/drone/push Build is failing
Details

Browse source 
- 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: #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>
This commit is contained in:
trinity-1686a 2021-11-29 11:52:42 +01:00 committed by Alex
parent 8811bb08e6
commit 7f26ed55cd
5 changed files with 46 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
src/api/api_server.rs
View file

@ -65,10 +65,15 @@ async fn handler(
} }
Err(e) => { Err(e) => {
let body: Body = Body::from(e.aws_xml(&garage.config.s3_api.s3_region, uri.path())); 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()) .status(e.http_status_code())
.header("Content-Type", "application/xml") .header("Content-Type", "application/xml");
.body(body)?;
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() { if e.http_status_code().is_server_error() {
warn!("Response: error {}, {}", e.http_status_code(), e); warn!("Response: error {}, {}", e.http_status_code(), e);

24
src/api/error.rs
View file

@ -1,5 +1,8 @@
use std::convert::TryInto;
use err_derive::Error; use err_derive::Error;
use hyper::StatusCode; use hyper::header::HeaderValue;
use hyper::{HeaderMap, StatusCode};
use garage_util::error::Error as GarageError; use garage_util::error::Error as GarageError;
@ -57,7 +60,7 @@ pub enum Error {
/// The client sent a range header with invalid value /// The client sent a range header with invalid value
#[error(display = "Invalid HTTP range: {:?}", _0)] #[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 /// The client sent an invalid request
#[error(display = "Bad request: {}", _0)] #[error(display = "Bad request: {}", _0)]
@ -90,6 +93,7 @@ impl Error {
Error::InternalError(_) | Error::Hyper(_) | Error::Http(_) => { Error::InternalError(_) | Error::Hyper(_) | Error::Http(_) => {
StatusCode::INTERNAL_SERVER_ERROR StatusCode::INTERNAL_SERVER_ERROR
} }
Error::InvalidRange(_) => StatusCode::RANGE_NOT_SATISFIABLE,
_ => StatusCode::BAD_REQUEST, _ => StatusCode::BAD_REQUEST,
} }
} }
@ -127,6 +131,22 @@ impl Error {
.into() .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 /// Trait to map error to the Bad Request error code

13
src/api/s3_get.rs
View file

@ -156,11 +156,12 @@ pub async fn handle_get(
let range = match req.headers().get("range") { let range = match req.headers().get("range") {
Some(range) => { Some(range) => {
let range_str = range.to_str()?; 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 { if ranges.len() > 1 {
return Err(Error::BadRequest( // garage does not support multi-range requests yet, so we respond with the entire
"Multiple ranges not supported".to_string(), // object when multiple ranges are requested
)); None
} else { } else {
ranges.pop() ranges.pop()
} }
@ -235,10 +236,6 @@ async fn handle_get_range(
begin: u64, begin: u64,
end: u64, end: u64,
) -> Result<Response<Body>, Error> { ) -> 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) let resp_builder = object_headers(version, version_meta)
.header("Content-Length", format!("{}", end - begin)) .header("Content-Length", format!("{}", end - begin))
.header( .header(

11
src/web/error.rs
View file

@ -1,5 +1,6 @@
use err_derive::Error; use err_derive::Error;
use hyper::StatusCode; use hyper::header::HeaderValue;
use hyper::{HeaderMap, StatusCode};
use garage_util::error::Error as GarageError; use garage_util::error::Error as GarageError;
@ -47,4 +48,12 @@ impl Error {
_ => StatusCode::BAD_REQUEST, _ => 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),
_ => (),
}
}
} }

1
src/web/web_server.rs
View file

@ -62,6 +62,7 @@ fn error_to_res(e: Error) -> Response<Body> {
let body: Body = Body::from(format!("{}\n", e)); let body: Body = Body::from(format!("{}\n", e));
let mut http_error = Response::new(body); let mut http_error = Response::new(body);
*http_error.status_mut() = e.http_status_code(); *http_error.status_mut() = e.http_status_code();
e.add_headers(http_error.headers_mut());
http_error http_error
} }