map range-error to the right http error code #157

Merged
lx merged 4 commits from trinity-1686a/garage:range-error into main 2021-11-29 10:52:42 +00:00
3 changed files with 30 additions and 6 deletions
Showing only changes of commit 36e104b665 - Show all commits

View file

@ -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() {
Review

It's kind of confusing that there is an if but as I understand the codepath is always taken. Using .unwrap() on .headers_mut() would have seemed more logical.

It's kind of confusing that there is an `if` but as I understand the codepath is always taken. Using `.unwrap()` on `.headers_mut()` would have seemed more logical.
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);

View file

@ -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<HeaderValue>) {
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

View file

@ -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