Error refactoring
Some checks failed
continuous-integration/drone/push Build is failing
continuous-integration/drone/pr Build is failing

This commit is contained in:
Alex 2022-05-13 15:04:53 +02:00
parent c0fb9fd0fe
commit 96b11524d5
Signed by: lx
GPG key ID: 0E496D15096376BE
24 changed files with 135 additions and 206 deletions

View file

@ -17,9 +17,9 @@ use garage_util::error::Error as GarageError;
use crate::generic_server::*; use crate::generic_server::*;
use crate::admin::error::*;
use crate::admin::bucket::*; use crate::admin::bucket::*;
use crate::admin::cluster::*; use crate::admin::cluster::*;
use crate::admin::error::*;
use crate::admin::key::*; use crate::admin::key::*;
use crate::admin::router::{Authorization, Endpoint}; use crate::admin::router::{Authorization, Endpoint};

View file

@ -16,8 +16,8 @@ use garage_model::garage::Garage;
use garage_model::permission::*; use garage_model::permission::*;
use garage_model::s3::object_table::ObjectFilter; use garage_model::s3::object_table::ObjectFilter;
use crate::admin::key::ApiBucketKeyPerm;
use crate::admin::error::*; use crate::admin::error::*;
use crate::admin::key::ApiBucketKeyPerm;
use crate::admin::parse_json_body; use crate::admin::parse_json_body;
pub async fn handle_list_buckets(garage: &Arc<Garage>) -> Result<Response<Body>, Error> { pub async fn handle_list_buckets(garage: &Arc<Garage>) -> Result<Response<Body>, Error> {
@ -98,7 +98,7 @@ pub async fn handle_get_bucket_info(
.ok_or_bad_request("Bucket not found")?, .ok_or_bad_request("Bucket not found")?,
_ => { _ => {
return Err(Error::bad_request( return Err(Error::bad_request(
"Either id or globalAlias must be provided (but not both)" "Either id or globalAlias must be provided (but not both)",
)); ));
} }
}; };

View file

@ -3,10 +3,10 @@ use hyper::header::HeaderValue;
use hyper::{Body, HeaderMap, StatusCode}; use hyper::{Body, HeaderMap, StatusCode};
use garage_model::helper::error::Error as HelperError; use garage_model::helper::error::Error as HelperError;
use garage_util::error::Error as GarageError;
use crate::generic_server::ApiError; use crate::generic_server::ApiError;
pub use crate::common_error::*; use crate::common_error::CommonError;
pub use crate::common_error::{OkOrBadRequest, OkOrInternalError};
/// Errors of this crate /// Errors of this crate
#[derive(Debug, Error)] #[derive(Debug, Error)]
@ -47,7 +47,9 @@ pub enum Error {
} }
impl<T> From<T> for Error impl<T> From<T> for Error
where CommonError: From<T> { where
CommonError: From<T>,
{
fn from(err: T) -> Self { fn from(err: T) -> Self {
Error::CommonError(CommonError::from(err)) Error::CommonError(CommonError::from(err))
} }
@ -83,7 +85,10 @@ impl ApiError for Error {
} }
fn http_body(&self, garage_region: &str, path: &str) -> Body { fn http_body(&self, garage_region: &str, path: &str) -> Body {
Body::from(format!("ERROR: {}\n\ngarage region: {}\npath: {}", self, garage_region, path)) Body::from(format!(
"ERROR: {}\n\ngarage region: {}\npath: {}",
self, garage_region, path
))
} }
} }

View file

@ -1,14 +1,13 @@
pub mod api_server; pub mod api_server;
mod router;
mod error; mod error;
mod router;
mod bucket; mod bucket;
mod cluster; mod cluster;
mod key; mod key;
use hyper::{Body, Request};
use serde::{Deserialize}; use serde::Deserialize;
use hyper::{Request, Body};
use error::*; use error::*;

View file

@ -1,12 +1,8 @@
use err_derive::Error; use err_derive::Error;
use hyper::header::HeaderValue; use hyper::StatusCode;
use hyper::{Body, HeaderMap, StatusCode};
use garage_model::helper::error::Error as HelperError;
use garage_util::error::Error as GarageError; use garage_util::error::Error as GarageError;
use crate::generic_server::ApiError;
/// Errors of this crate /// Errors of this crate
#[derive(Debug, Error)] #[derive(Debug, Error)]
pub enum CommonError { pub enum CommonError {
@ -36,8 +32,9 @@ impl CommonError {
| GarageError::RemoteError(_) | GarageError::RemoteError(_)
| GarageError::Quorum(_, _, _, _), | GarageError::Quorum(_, _, _, _),
) => StatusCode::SERVICE_UNAVAILABLE, ) => StatusCode::SERVICE_UNAVAILABLE,
CommonError::InternalError(_) | CommonError::Hyper(_) | CommonError::Http(_) => CommonError::InternalError(_) | CommonError::Hyper(_) | CommonError::Http(_) => {
StatusCode::INTERNAL_SERVER_ERROR, StatusCode::INTERNAL_SERVER_ERROR
}
CommonError::BadRequest(_) => StatusCode::BAD_REQUEST, CommonError::BadRequest(_) => StatusCode::BAD_REQUEST,
} }
} }
@ -57,7 +54,11 @@ where
fn ok_or_bad_request<M: AsRef<str>>(self, reason: M) -> Result<T, CommonError> { fn ok_or_bad_request<M: AsRef<str>>(self, reason: M) -> Result<T, CommonError> {
match self { match self {
Ok(x) => Ok(x), Ok(x) => Ok(x),
Err(e) => Err(CommonError::BadRequest(format!("{}: {}", reason.as_ref(), e))), Err(e) => Err(CommonError::BadRequest(format!(
"{}: {}",
reason.as_ref(),
e
))),
} }
} }
} }

View file

@ -7,24 +7,17 @@ use hyper::{Body, HeaderMap, StatusCode};
use garage_model::helper::error::Error as HelperError; use garage_model::helper::error::Error as HelperError;
use garage_util::error::Error as GarageError; use garage_util::error::Error as GarageError;
use crate::common_error::CommonError;
pub use crate::common_error::{OkOrBadRequest, OkOrInternalError};
use crate::generic_server::ApiError; use crate::generic_server::ApiError;
use crate::s3::xml as s3_xml; use crate::s3::xml as s3_xml;
/// Errors of this crate /// Errors of this crate
#[derive(Debug, Error)] #[derive(Debug, Error)]
pub enum Error { pub enum Error {
// Category: internal error #[error(display = "{}", _0)]
/// Error related to deeper parts of Garage /// Error from common error
#[error(display = "Internal error: {}", _0)] CommonError(CommonError),
InternalError(#[error(source)] GarageError),
/// Error related to Hyper
#[error(display = "Internal error (Hyper error): {}", _0)]
Hyper(#[error(source)] hyper::Error),
/// Error related to HTTP
#[error(display = "Internal error (HTTP error): {}", _0)]
Http(#[error(source)] http::Error),
// Category: cannot process // Category: cannot process
/// No proper api key was used, or the signature was invalid /// No proper api key was used, or the signature was invalid
@ -101,10 +94,6 @@ pub enum Error {
#[error(display = "Invalid HTTP range: {:?}", _0)] #[error(display = "Invalid HTTP range: {:?}", _0)]
InvalidRange(#[error(from)] (http_range::HttpRangeParseError, u64)), InvalidRange(#[error(from)] (http_range::HttpRangeParseError, u64)),
/// The client sent an invalid request
#[error(display = "Bad request: {}", _0)]
BadRequest(String),
/// The client asked for an invalid return format (invalid Accept header) /// The client asked for an invalid return format (invalid Accept header)
#[error(display = "Not acceptable: {}", _0)] #[error(display = "Not acceptable: {}", _0)]
NotAcceptable(String), NotAcceptable(String),
@ -114,6 +103,15 @@ pub enum Error {
NotImplemented(String), NotImplemented(String),
} }
impl<T> From<T> for Error
where
CommonError: From<T>,
{
fn from(err: T) -> Self {
Error::CommonError(CommonError::from(err))
}
}
impl From<roxmltree::Error> for Error { impl From<roxmltree::Error> for Error {
fn from(err: roxmltree::Error) -> Self { fn from(err: roxmltree::Error) -> Self {
Self::InvalidXml(format!("{}", err)) Self::InvalidXml(format!("{}", err))
@ -129,16 +127,16 @@ impl From<quick_xml::de::DeError> for Error {
impl From<HelperError> for Error { impl From<HelperError> for Error {
fn from(err: HelperError) -> Self { fn from(err: HelperError) -> Self {
match err { match err {
HelperError::Internal(i) => Self::InternalError(i), HelperError::Internal(i) => Self::CommonError(CommonError::InternalError(i)),
HelperError::BadRequest(b) => Self::BadRequest(b), HelperError::BadRequest(b) => Self::CommonError(CommonError::BadRequest(b)),
e => Self::BadRequest(format!("{}", e)), e => Self::CommonError(CommonError::BadRequest(format!("{}", e))),
} }
} }
} }
impl From<multer::Error> for Error { impl From<multer::Error> for Error {
fn from(err: multer::Error) -> Self { fn from(err: multer::Error) -> Self {
Self::BadRequest(err.to_string()) Self::bad_request(err)
} }
} }
@ -157,18 +155,26 @@ impl Error {
Error::Forbidden(_) => "AccessDenied", Error::Forbidden(_) => "AccessDenied",
Error::AuthorizationHeaderMalformed(_) => "AuthorizationHeaderMalformed", Error::AuthorizationHeaderMalformed(_) => "AuthorizationHeaderMalformed",
Error::NotImplemented(_) => "NotImplemented", Error::NotImplemented(_) => "NotImplemented",
Error::InternalError( Error::CommonError(CommonError::InternalError(
GarageError::Timeout GarageError::Timeout
| GarageError::RemoteError(_) | GarageError::RemoteError(_)
| GarageError::Quorum(_, _, _, _), | GarageError::Quorum(_, _, _, _),
) => "ServiceUnavailable", )) => "ServiceUnavailable",
Error::InternalError(_) | Error::Hyper(_) | Error::Http(_) => "InternalError", Error::CommonError(
CommonError::InternalError(_) | CommonError::Hyper(_) | CommonError::Http(_),
) => "InternalError",
_ => "InvalidRequest", _ => "InvalidRequest",
} }
} }
pub fn internal_error<M: ToString>(msg: M) -> Self {
Self::CommonError(CommonError::InternalError(GarageError::Message(
msg.to_string(),
)))
}
pub fn bad_request<M: ToString>(msg: M) -> Self { pub fn bad_request<M: ToString>(msg: M) -> Self {
Self::BadRequest(msg.to_string()) Self::CommonError(CommonError::BadRequest(msg.to_string()))
} }
} }
@ -176,19 +182,12 @@ impl ApiError for Error {
/// Get the HTTP status code that best represents the meaning of the error for the client /// Get the HTTP status code that best represents the meaning of the error for the client
fn http_status_code(&self) -> StatusCode { fn http_status_code(&self) -> StatusCode {
match self { match self {
Error::CommonError(c) => c.http_status_code(),
Error::NoSuchKey | Error::NoSuchBucket | Error::NoSuchUpload => StatusCode::NOT_FOUND, Error::NoSuchKey | Error::NoSuchBucket | Error::NoSuchUpload => StatusCode::NOT_FOUND,
Error::BucketNotEmpty | Error::BucketAlreadyExists => StatusCode::CONFLICT, Error::BucketNotEmpty | Error::BucketAlreadyExists => StatusCode::CONFLICT,
Error::PreconditionFailed => StatusCode::PRECONDITION_FAILED, Error::PreconditionFailed => StatusCode::PRECONDITION_FAILED,
Error::Forbidden(_) => StatusCode::FORBIDDEN, Error::Forbidden(_) => StatusCode::FORBIDDEN,
Error::NotAcceptable(_) => StatusCode::NOT_ACCEPTABLE, Error::NotAcceptable(_) => StatusCode::NOT_ACCEPTABLE,
Error::InternalError(
GarageError::Timeout
| GarageError::RemoteError(_)
| GarageError::Quorum(_, _, _, _),
) => StatusCode::SERVICE_UNAVAILABLE,
Error::InternalError(_) | Error::Hyper(_) | Error::Http(_) => {
StatusCode::INTERNAL_SERVER_ERROR
}
Error::InvalidRange(_) => StatusCode::RANGE_NOT_SATISFIABLE, Error::InvalidRange(_) => StatusCode::RANGE_NOT_SATISFIABLE,
Error::NotImplemented(_) => StatusCode::NOT_IMPLEMENTED, Error::NotImplemented(_) => StatusCode::NOT_IMPLEMENTED,
_ => StatusCode::BAD_REQUEST, _ => StatusCode::BAD_REQUEST,
@ -230,67 +229,3 @@ impl ApiError for Error {
})) }))
} }
} }
/// Trait to map error to the Bad Request error code
pub trait OkOrBadRequest {
type S;
fn ok_or_bad_request<M: AsRef<str>>(self, reason: M) -> Result<Self::S, Error>;
}
impl<T, E> OkOrBadRequest for Result<T, E>
where
E: std::fmt::Display,
{
type S = T;
fn ok_or_bad_request<M: AsRef<str>>(self, reason: M) -> Result<T, Error> {
match self {
Ok(x) => Ok(x),
Err(e) => Err(Error::BadRequest(format!("{}: {}", reason.as_ref(), e))),
}
}
}
impl<T> OkOrBadRequest for Option<T> {
type S = T;
fn ok_or_bad_request<M: AsRef<str>>(self, reason: M) -> Result<T, Error> {
match self {
Some(x) => Ok(x),
None => Err(Error::BadRequest(reason.as_ref().to_string())),
}
}
}
/// Trait to map an error to an Internal Error code
pub trait OkOrInternalError {
type S;
fn ok_or_internal_error<M: AsRef<str>>(self, reason: M) -> Result<Self::S, Error>;
}
impl<T, E> OkOrInternalError for Result<T, E>
where
E: std::fmt::Display,
{
type S = T;
fn ok_or_internal_error<M: AsRef<str>>(self, reason: M) -> Result<T, Error> {
match self {
Ok(x) => Ok(x),
Err(e) => Err(Error::InternalError(GarageError::Message(format!(
"{}: {}",
reason.as_ref(),
e
)))),
}
}
}
impl<T> OkOrInternalError for Option<T> {
type S = T;
fn ok_or_internal_error<M: AsRef<str>>(self, reason: M) -> Result<T, Error> {
match self {
Some(x) => Ok(x),
None => Err(Error::InternalError(GarageError::Message(
reason.as_ref().to_string(),
))),
}
}
}

View file

@ -52,7 +52,7 @@ pub fn authority_to_host(authority: &str) -> Result<String, Error> {
let mut iter = authority.chars().enumerate(); let mut iter = authority.chars().enumerate();
let (_, first_char) = iter let (_, first_char) = iter
.next() .next()
.ok_or_else(|| Error::BadRequest("Authority is empty".to_string()))?; .ok_or_else(|| Error::bad_request("Authority is empty".to_string()))?;
let split = match first_char { let split = match first_char {
'[' => { '[' => {
@ -60,7 +60,7 @@ pub fn authority_to_host(authority: &str) -> Result<String, Error> {
match iter.next() { match iter.next() {
Some((_, ']')) => iter.next(), Some((_, ']')) => iter.next(),
_ => { _ => {
return Err(Error::BadRequest(format!( return Err(Error::bad_request(format!(
"Authority {} has an illegal format", "Authority {} has an illegal format",
authority authority
))) )))
@ -73,7 +73,7 @@ pub fn authority_to_host(authority: &str) -> Result<String, Error> {
let authority = match split { let authority = match split {
Some((i, ':')) => Ok(&authority[..i]), Some((i, ':')) => Ok(&authority[..i]),
None => Ok(authority), None => Ok(authority),
Some((_, _)) => Err(Error::BadRequest(format!( Some((_, _)) => Err(Error::bad_request(format!(
"Authority {} has an illegal format", "Authority {} has an illegal format",
authority authority
))), ))),
@ -134,7 +134,7 @@ pub fn parse_bucket_key<'a>(
None => (path, None), None => (path, None),
}; };
if bucket.is_empty() { if bucket.is_empty() {
return Err(Error::BadRequest("No bucket specified".to_string())); return Err(Error::bad_request("No bucket specified"));
} }
Ok((bucket, key)) Ok((bucket, key))
} }

View file

@ -88,7 +88,7 @@ async fn handle_read_batch_query(
let (items, more, next_start) = if query.single_item { let (items, more, next_start) = if query.single_item {
if query.prefix.is_some() || query.end.is_some() || query.limit.is_some() || query.reverse { if query.prefix.is_some() || query.end.is_some() || query.limit.is_some() || query.reverse {
return Err(Error::BadRequest("Batch query parameters 'prefix', 'end', 'limit' and 'reverse' must not be set when singleItem is true.".into())); return Err(Error::bad_request("Batch query parameters 'prefix', 'end', 'limit' and 'reverse' must not be set when singleItem is true."));
} }
let sk = query let sk = query
.start .start
@ -183,7 +183,7 @@ async fn handle_delete_batch_query(
let deleted_items = if query.single_item { let deleted_items = if query.single_item {
if query.prefix.is_some() || query.end.is_some() { if query.prefix.is_some() || query.end.is_some() {
return Err(Error::BadRequest("Batch query parameters 'prefix' and 'end' must not be set when singleItem is true.".into())); return Err(Error::bad_request("Batch query parameters 'prefix' and 'end' must not be set when singleItem is true."));
} }
let sk = query let sk = query
.start .start

View file

@ -31,7 +31,7 @@ where
(None, Some(s)) => (Some(s.clone()), false), (None, Some(s)) => (Some(s.clone()), false),
(Some(p), Some(s)) => { (Some(p), Some(s)) => {
if !s.starts_with(p) { if !s.starts_with(p) {
return Err(Error::BadRequest(format!( return Err(Error::bad_request(format!(
"Start key '{}' does not start with prefix '{}'", "Start key '{}' does not start with prefix '{}'",
s, p s, p
))); )));

View file

@ -62,7 +62,7 @@ impl Endpoint {
.unwrap_or((path.to_owned(), "")); .unwrap_or((path.to_owned(), ""));
if bucket.is_empty() { if bucket.is_empty() {
return Err(Error::BadRequest("Missing bucket name".to_owned())); return Err(Error::bad_request("Missing bucket name".to_owned()));
} }
if *req.method() == Method::OPTIONS { if *req.method() == Method::OPTIONS {
@ -83,7 +83,7 @@ impl Endpoint {
Method::PUT => Self::from_put(partition_key, &mut query)?, Method::PUT => Self::from_put(partition_key, &mut query)?,
Method::DELETE => Self::from_delete(partition_key, &mut query)?, Method::DELETE => Self::from_delete(partition_key, &mut query)?,
_ if req.method() == method_search => Self::from_search(partition_key, &mut query)?, _ if req.method() == method_search => Self::from_search(partition_key, &mut query)?,
_ => return Err(Error::BadRequest("Unknown method".to_owned())), _ => return Err(Error::bad_request("Unknown method".to_owned())),
}; };
if let Some(message) = query.nonempty_message() { if let Some(message) = query.nonempty_message() {

View file

@ -310,7 +310,7 @@ impl ApiHandler for S3ApiServer {
) )
.await .await
} else { } else {
Err(Error::BadRequest(format!( Err(Error::bad_request(format!(
"Invalid endpoint: list-type={}", "Invalid endpoint: list-type={}",
list_type list_type
))) )))

View file

@ -130,7 +130,7 @@ pub async fn handle_create_bucket(
if let Some(location_constraint) = cmd { if let Some(location_constraint) = cmd {
if location_constraint != garage.config.s3_api.s3_region { if location_constraint != garage.config.s3_api.s3_region {
return Err(Error::BadRequest(format!( return Err(Error::bad_request(format!(
"Cannot satisfy location constraint `{}`: buckets can only be created in region `{}`", "Cannot satisfy location constraint `{}`: buckets can only be created in region `{}`",
location_constraint, location_constraint,
garage.config.s3_api.s3_region garage.config.s3_api.s3_region
@ -163,7 +163,7 @@ pub async fn handle_create_bucket(
} else { } else {
// Create the bucket! // Create the bucket!
if !is_valid_bucket_name(&bucket_name) { if !is_valid_bucket_name(&bucket_name) {
return Err(Error::BadRequest(format!( return Err(Error::bad_request(format!(
"{}: {}", "{}: {}",
bucket_name, INVALID_BUCKET_NAME_MESSAGE bucket_name, INVALID_BUCKET_NAME_MESSAGE
))); )));

View file

@ -201,8 +201,8 @@ pub async fn handle_upload_part_copy(
let mut ranges = http_range::HttpRange::parse(range_str, source_version_meta.size) let mut ranges = http_range::HttpRange::parse(range_str, source_version_meta.size)
.map_err(|e| (e, source_version_meta.size))?; .map_err(|e| (e, source_version_meta.size))?;
if ranges.len() != 1 { if ranges.len() != 1 {
return Err(Error::BadRequest( return Err(Error::bad_request(
"Invalid x-amz-copy-source-range header: exactly 1 range must be given".into(), "Invalid x-amz-copy-source-range header: exactly 1 range must be given",
)); ));
} else { } else {
ranges.pop().unwrap() ranges.pop().unwrap()
@ -230,8 +230,8 @@ pub async fn handle_upload_part_copy(
// This is only for small files, we don't bother handling this. // This is only for small files, we don't bother handling this.
// (in AWS UploadPartCopy works for parts at least 5MB which // (in AWS UploadPartCopy works for parts at least 5MB which
// is never the case of an inline object) // is never the case of an inline object)
return Err(Error::BadRequest( return Err(Error::bad_request(
"Source object is too small (minimum part size is 5Mb)".into(), "Source object is too small (minimum part size is 5Mb)",
)); ));
} }
ObjectVersionData::FirstBlock(_meta, _first_block_hash) => (), ObjectVersionData::FirstBlock(_meta, _first_block_hash) => (),
@ -250,7 +250,7 @@ pub async fn handle_upload_part_copy(
// Check this part number hasn't yet been uploaded // Check this part number hasn't yet been uploaded
if let Some(dv) = dest_version { if let Some(dv) = dest_version {
if dv.has_part_number(part_number) { if dv.has_part_number(part_number) {
return Err(Error::BadRequest(format!( return Err(Error::bad_request(format!(
"Part number {} has already been uploaded", "Part number {} has already been uploaded",
part_number part_number
))); )));
@ -536,8 +536,8 @@ impl CopyPreconditionHeaders {
(None, None, None, Some(ims)) => v_date > *ims, (None, None, None, Some(ims)) => v_date > *ims,
(None, None, None, None) => true, (None, None, None, None) => true,
_ => { _ => {
return Err(Error::BadRequest( return Err(Error::bad_request(
"Invalid combination of x-amz-copy-source-if-xxxxx headers".into(), "Invalid combination of x-amz-copy-source-if-xxxxx headers",
)) ))
} }
}; };

View file

@ -210,8 +210,8 @@ pub async fn handle_get(
match (part_number, parse_range_header(req, last_v_meta.size)?) { match (part_number, parse_range_header(req, last_v_meta.size)?) {
(Some(_), Some(_)) => { (Some(_), Some(_)) => {
return Err(Error::BadRequest( return Err(Error::bad_request(
"Cannot specify both partNumber and Range header".into(), "Cannot specify both partNumber and Range header",
)); ));
} }
(Some(pn), None) => { (Some(pn), None) => {
@ -302,9 +302,9 @@ async fn handle_get_range(
let body: Body = Body::from(bytes[begin as usize..end as usize].to_vec()); let body: Body = Body::from(bytes[begin as usize..end as usize].to_vec());
Ok(resp_builder.body(body)?) Ok(resp_builder.body(body)?)
} else { } else {
None.ok_or_internal_error( Err(Error::internal_error(
"Requested range not present in inline bytes when it should have been", "Requested range not present in inline bytes when it should have been",
) ))
} }
} }
ObjectVersionData::FirstBlock(_meta, _first_block_hash) => { ObjectVersionData::FirstBlock(_meta, _first_block_hash) => {

View file

@ -588,7 +588,7 @@ impl ListObjectsQuery {
"]" => Ok(RangeBegin::AfterKey { "]" => Ok(RangeBegin::AfterKey {
key: String::from_utf8(base64::decode(token[1..].as_bytes())?)?, key: String::from_utf8(base64::decode(token[1..].as_bytes())?)?,
}), }),
_ => Err(Error::BadRequest("Invalid continuation token".to_string())), _ => Err(Error::bad_request("Invalid continuation token".to_string())),
}, },
// StartAfter has defined semantics in the spec: // StartAfter has defined semantics in the spec:

View file

@ -48,7 +48,7 @@ pub async fn handle_post_object(
let field = if let Some(field) = multipart.next_field().await? { let field = if let Some(field) = multipart.next_field().await? {
field field
} else { } else {
return Err(Error::BadRequest( return Err(Error::bad_request(
"Request did not contain a file".to_owned(), "Request did not contain a file".to_owned(),
)); ));
}; };
@ -66,14 +66,14 @@ pub async fn handle_post_object(
"tag" => (/* tag need to be reencoded, but we don't support them yet anyway */), "tag" => (/* tag need to be reencoded, but we don't support them yet anyway */),
"acl" => { "acl" => {
if params.insert("x-amz-acl", content).is_some() { if params.insert("x-amz-acl", content).is_some() {
return Err(Error::BadRequest( return Err(Error::bad_request(
"Field 'acl' provided more than one time".to_string(), "Field 'acl' provided more than one time".to_string(),
)); ));
} }
} }
_ => { _ => {
if params.insert(&name, content).is_some() { if params.insert(&name, content).is_some() {
return Err(Error::BadRequest(format!( return Err(Error::bad_request(format!(
"Field '{}' provided more than one time", "Field '{}' provided more than one time",
name name
))); )));
@ -145,7 +145,7 @@ pub async fn handle_post_object(
.ok_or_bad_request("Invalid expiration date")? .ok_or_bad_request("Invalid expiration date")?
.into(); .into();
if Utc::now() - expiration > Duration::zero() { if Utc::now() - expiration > Duration::zero() {
return Err(Error::BadRequest( return Err(Error::bad_request(
"Expiration date is in the paste".to_string(), "Expiration date is in the paste".to_string(),
)); ));
} }
@ -159,7 +159,7 @@ pub async fn handle_post_object(
"policy" | "x-amz-signature" => (), // this is always accepted, as it's required to validate other fields "policy" | "x-amz-signature" => (), // this is always accepted, as it's required to validate other fields
"content-type" => { "content-type" => {
let conds = conditions.params.remove("content-type").ok_or_else(|| { let conds = conditions.params.remove("content-type").ok_or_else(|| {
Error::BadRequest(format!("Key '{}' is not allowed in policy", param_key)) Error::bad_request(format!("Key '{}' is not allowed in policy", param_key))
})?; })?;
for cond in conds { for cond in conds {
let ok = match cond { let ok = match cond {
@ -169,7 +169,7 @@ pub async fn handle_post_object(
} }
}; };
if !ok { if !ok {
return Err(Error::BadRequest(format!( return Err(Error::bad_request(format!(
"Key '{}' has value not allowed in policy", "Key '{}' has value not allowed in policy",
param_key param_key
))); )));
@ -178,7 +178,7 @@ pub async fn handle_post_object(
} }
"key" => { "key" => {
let conds = conditions.params.remove("key").ok_or_else(|| { let conds = conditions.params.remove("key").ok_or_else(|| {
Error::BadRequest(format!("Key '{}' is not allowed in policy", param_key)) Error::bad_request(format!("Key '{}' is not allowed in policy", param_key))
})?; })?;
for cond in conds { for cond in conds {
let ok = match cond { let ok = match cond {
@ -186,7 +186,7 @@ pub async fn handle_post_object(
Operation::StartsWith(s) => key.starts_with(&s), Operation::StartsWith(s) => key.starts_with(&s),
}; };
if !ok { if !ok {
return Err(Error::BadRequest(format!( return Err(Error::bad_request(format!(
"Key '{}' has value not allowed in policy", "Key '{}' has value not allowed in policy",
param_key param_key
))); )));
@ -201,7 +201,7 @@ pub async fn handle_post_object(
continue; continue;
} }
let conds = conditions.params.remove(&param_key).ok_or_else(|| { let conds = conditions.params.remove(&param_key).ok_or_else(|| {
Error::BadRequest(format!("Key '{}' is not allowed in policy", param_key)) Error::bad_request(format!("Key '{}' is not allowed in policy", param_key))
})?; })?;
for cond in conds { for cond in conds {
let ok = match cond { let ok = match cond {
@ -209,7 +209,7 @@ pub async fn handle_post_object(
Operation::StartsWith(s) => value.to_str()?.starts_with(s.as_str()), Operation::StartsWith(s) => value.to_str()?.starts_with(s.as_str()),
}; };
if !ok { if !ok {
return Err(Error::BadRequest(format!( return Err(Error::bad_request(format!(
"Key '{}' has value not allowed in policy", "Key '{}' has value not allowed in policy",
param_key param_key
))); )));
@ -220,7 +220,7 @@ pub async fn handle_post_object(
} }
if let Some((param_key, _)) = conditions.params.iter().next() { if let Some((param_key, _)) = conditions.params.iter().next() {
return Err(Error::BadRequest(format!( return Err(Error::bad_request(format!(
"Key '{}' is required in policy, but no value was provided", "Key '{}' is required in policy, but no value was provided",
param_key param_key
))); )));
@ -326,7 +326,7 @@ impl Policy {
match condition { match condition {
PolicyCondition::Equal(map) => { PolicyCondition::Equal(map) => {
if map.len() != 1 { if map.len() != 1 {
return Err(Error::BadRequest("Invalid policy item".to_owned())); return Err(Error::bad_request("Invalid policy item".to_owned()));
} }
let (mut k, v) = map.into_iter().next().expect("size was verified"); let (mut k, v) = map.into_iter().next().expect("size was verified");
k.make_ascii_lowercase(); k.make_ascii_lowercase();
@ -334,7 +334,7 @@ impl Policy {
} }
PolicyCondition::OtherOp([cond, mut key, value]) => { PolicyCondition::OtherOp([cond, mut key, value]) => {
if key.remove(0) != '$' { if key.remove(0) != '$' {
return Err(Error::BadRequest("Invalid policy item".to_owned())); return Err(Error::bad_request("Invalid policy item".to_owned()));
} }
key.make_ascii_lowercase(); key.make_ascii_lowercase();
match cond.as_str() { match cond.as_str() {
@ -347,7 +347,7 @@ impl Policy {
.or_default() .or_default()
.push(Operation::StartsWith(value)); .push(Operation::StartsWith(value));
} }
_ => return Err(Error::BadRequest("Invalid policy item".to_owned())), _ => return Err(Error::bad_request("Invalid policy item".to_owned())),
} }
} }
PolicyCondition::SizeRange(key, min, max) => { PolicyCondition::SizeRange(key, min, max) => {
@ -355,7 +355,7 @@ impl Policy {
length.0 = length.0.max(min); length.0 = length.0.max(min);
length.1 = length.1.min(max); length.1 = length.1.min(max);
} else { } else {
return Err(Error::BadRequest("Invalid policy item".to_owned())); return Err(Error::bad_request("Invalid policy item".to_owned()));
} }
} }
} }
@ -420,14 +420,14 @@ where
self.read += bytes.len() as u64; self.read += bytes.len() as u64;
// optimization to fail early when we know before the end it's too long // optimization to fail early when we know before the end it's too long
if self.length.end() < &self.read { if self.length.end() < &self.read {
return Poll::Ready(Some(Err(Error::BadRequest( return Poll::Ready(Some(Err(Error::bad_request(
"File size does not match policy".to_owned(), "File size does not match policy".to_owned(),
)))); ))));
} }
} }
Poll::Ready(None) => { Poll::Ready(None) => {
if !self.length.contains(&self.read) { if !self.length.contains(&self.read) {
return Poll::Ready(Some(Err(Error::BadRequest( return Poll::Ready(Some(Err(Error::bad_request(
"File size does not match policy".to_owned(), "File size does not match policy".to_owned(),
)))); ))));
} }

View file

@ -183,7 +183,7 @@ fn ensure_checksum_matches(
) -> Result<(), Error> { ) -> Result<(), Error> {
if let Some(expected_sha256) = content_sha256 { if let Some(expected_sha256) = content_sha256 {
if expected_sha256 != data_sha256sum { if expected_sha256 != data_sha256sum {
return Err(Error::BadRequest( return Err(Error::bad_request(
"Unable to validate x-amz-content-sha256".to_string(), "Unable to validate x-amz-content-sha256".to_string(),
)); ));
} else { } else {
@ -192,7 +192,7 @@ fn ensure_checksum_matches(
} }
if let Some(expected_md5) = content_md5 { if let Some(expected_md5) = content_md5 {
if expected_md5.trim_matches('"') != base64::encode(data_md5sum) { if expected_md5.trim_matches('"') != base64::encode(data_md5sum) {
return Err(Error::BadRequest( return Err(Error::bad_request(
"Unable to validate content-md5".to_string(), "Unable to validate content-md5".to_string(),
)); ));
} else { } else {
@ -428,7 +428,7 @@ pub async fn handle_put_part(
// Check part hasn't already been uploaded // Check part hasn't already been uploaded
if let Some(v) = version { if let Some(v) = version {
if v.has_part_number(part_number) { if v.has_part_number(part_number) {
return Err(Error::BadRequest(format!( return Err(Error::bad_request(format!(
"Part number {} has already been uploaded", "Part number {} has already been uploaded",
part_number part_number
))); )));
@ -513,7 +513,7 @@ pub async fn handle_complete_multipart_upload(
let version = version.ok_or(Error::NoSuchKey)?; let version = version.ok_or(Error::NoSuchKey)?;
if version.blocks.is_empty() { if version.blocks.is_empty() {
return Err(Error::BadRequest("No data was uploaded".to_string())); return Err(Error::bad_request("No data was uploaded".to_string()));
} }
let headers = match object_version.state { let headers = match object_version.state {
@ -574,8 +574,8 @@ pub async fn handle_complete_multipart_upload(
.map(|x| x.part_number) .map(|x| x.part_number)
.eq(block_parts.into_iter()); .eq(block_parts.into_iter());
if !same_parts { if !same_parts {
return Err(Error::BadRequest( return Err(Error::bad_request(
"Part numbers in block list and part list do not match. This can happen if a part was partially uploaded. Please abort the multipart upload and try again.".into(), "Part numbers in block list and part list do not match. This can happen if a part was partially uploaded. Please abort the multipart upload and try again."
)); ));
} }

View file

@ -342,7 +342,7 @@ impl Endpoint {
Method::POST => Self::from_post(key, &mut query)?, Method::POST => Self::from_post(key, &mut query)?,
Method::PUT => Self::from_put(key, &mut query, req.headers())?, Method::PUT => Self::from_put(key, &mut query, req.headers())?,
Method::DELETE => Self::from_delete(key, &mut query)?, Method::DELETE => Self::from_delete(key, &mut query)?,
_ => return Err(Error::BadRequest("Unknown method".to_owned())), _ => return Err(Error::bad_request("Unknown method".to_owned())),
}; };
if let Some(message) = query.nonempty_message() { if let Some(message) = query.nonempty_message() {

View file

@ -176,7 +176,7 @@ impl WebsiteConfiguration {
|| self.index_document.is_some() || self.index_document.is_some()
|| self.routing_rules.is_some()) || self.routing_rules.is_some())
{ {
return Err(Error::BadRequest( return Err(Error::bad_request(
"Bad XML: can't have RedirectAllRequestsTo and other fields".to_owned(), "Bad XML: can't have RedirectAllRequestsTo and other fields".to_owned(),
)); ));
} }
@ -222,7 +222,7 @@ impl WebsiteConfiguration {
impl Key { impl Key {
pub fn validate(&self) -> Result<(), Error> { pub fn validate(&self) -> Result<(), Error> {
if self.key.0.is_empty() { if self.key.0.is_empty() {
Err(Error::BadRequest( Err(Error::bad_request(
"Bad XML: error document specified but empty".to_owned(), "Bad XML: error document specified but empty".to_owned(),
)) ))
} else { } else {
@ -234,7 +234,7 @@ impl Key {
impl Suffix { impl Suffix {
pub fn validate(&self) -> Result<(), Error> { pub fn validate(&self) -> Result<(), Error> {
if self.suffix.0.is_empty() | self.suffix.0.contains('/') { if self.suffix.0.is_empty() | self.suffix.0.contains('/') {
Err(Error::BadRequest( Err(Error::bad_request(
"Bad XML: index document is empty or contains /".to_owned(), "Bad XML: index document is empty or contains /".to_owned(),
)) ))
} else { } else {
@ -247,7 +247,7 @@ impl Target {
pub fn validate(&self) -> Result<(), Error> { pub fn validate(&self) -> Result<(), Error> {
if let Some(ref protocol) = self.protocol { if let Some(ref protocol) = self.protocol {
if protocol.0 != "http" && protocol.0 != "https" { if protocol.0 != "http" && protocol.0 != "https" {
return Err(Error::BadRequest("Bad XML: invalid protocol".to_owned())); return Err(Error::bad_request("Bad XML: invalid protocol".to_owned()));
} }
} }
Ok(()) Ok(())
@ -269,19 +269,19 @@ impl Redirect {
pub fn validate(&self, has_prefix: bool) -> Result<(), Error> { pub fn validate(&self, has_prefix: bool) -> Result<(), Error> {
if self.replace_prefix.is_some() { if self.replace_prefix.is_some() {
if self.replace_full.is_some() { if self.replace_full.is_some() {
return Err(Error::BadRequest( return Err(Error::bad_request(
"Bad XML: both ReplaceKeyPrefixWith and ReplaceKeyWith are set".to_owned(), "Bad XML: both ReplaceKeyPrefixWith and ReplaceKeyWith are set".to_owned(),
)); ));
} }
if !has_prefix { if !has_prefix {
return Err(Error::BadRequest( return Err(Error::bad_request(
"Bad XML: ReplaceKeyPrefixWith is set, but KeyPrefixEquals isn't".to_owned(), "Bad XML: ReplaceKeyPrefixWith is set, but KeyPrefixEquals isn't".to_owned(),
)); ));
} }
} }
if let Some(ref protocol) = self.protocol { if let Some(ref protocol) = self.protocol {
if protocol.0 != "http" && protocol.0 != "https" { if protocol.0 != "http" && protocol.0 != "https" {
return Err(Error::BadRequest("Bad XML: invalid protocol".to_owned())); return Err(Error::bad_request("Bad XML: invalid protocol".to_owned()));
} }
} }
// TODO there are probably more invalide cases, but which ones? // TODO there are probably more invalide cases, but which ones?

View file

@ -16,7 +16,7 @@ type HmacSha256 = Hmac<Sha256>;
pub fn verify_signed_content(expected_sha256: Hash, body: &[u8]) -> Result<(), Error> { pub fn verify_signed_content(expected_sha256: Hash, body: &[u8]) -> Result<(), Error> {
if expected_sha256 != sha256sum(body) { if expected_sha256 != sha256sum(body) {
return Err(Error::BadRequest( return Err(Error::bad_request(
"Request content hash does not match signed hash".to_string(), "Request content hash does not match signed hash".to_string(),
)); ));
} }

View file

@ -105,7 +105,7 @@ fn parse_authorization(
let (auth_kind, rest) = authorization.split_at(first_space); let (auth_kind, rest) = authorization.split_at(first_space);
if auth_kind != "AWS4-HMAC-SHA256" { if auth_kind != "AWS4-HMAC-SHA256" {
return Err(Error::BadRequest("Unsupported authorization method".into())); return Err(Error::bad_request("Unsupported authorization method"));
} }
let mut auth_params = HashMap::new(); let mut auth_params = HashMap::new();
@ -129,10 +129,11 @@ fn parse_authorization(
let date = headers let date = headers
.get("x-amz-date") .get("x-amz-date")
.ok_or_bad_request("Missing X-Amz-Date field") .ok_or_bad_request("Missing X-Amz-Date field")
.map_err(Error::from)
.and_then(|d| parse_date(d))?; .and_then(|d| parse_date(d))?;
if Utc::now() - date > Duration::hours(24) { if Utc::now() - date > Duration::hours(24) {
return Err(Error::BadRequest("Date is too old".to_string())); return Err(Error::bad_request("Date is too old".to_string()));
} }
let auth = Authorization { let auth = Authorization {
@ -156,7 +157,7 @@ fn parse_query_authorization(
headers: &HashMap<String, String>, headers: &HashMap<String, String>,
) -> Result<Authorization, Error> { ) -> Result<Authorization, Error> {
if algorithm != "AWS4-HMAC-SHA256" { if algorithm != "AWS4-HMAC-SHA256" {
return Err(Error::BadRequest( return Err(Error::bad_request(
"Unsupported authorization method".to_string(), "Unsupported authorization method".to_string(),
)); ));
} }
@ -179,10 +180,10 @@ fn parse_query_authorization(
.get("x-amz-expires") .get("x-amz-expires")
.ok_or_bad_request("X-Amz-Expires not found in query parameters")? .ok_or_bad_request("X-Amz-Expires not found in query parameters")?
.parse() .parse()
.map_err(|_| Error::BadRequest("X-Amz-Expires is not a number".to_string()))?; .map_err(|_| Error::bad_request("X-Amz-Expires is not a number".to_string()))?;
if duration > 7 * 24 * 3600 { if duration > 7 * 24 * 3600 {
return Err(Error::BadRequest( return Err(Error::bad_request(
"X-Amz-Exprires may not exceed a week".to_string(), "X-Amz-Exprires may not exceed a week".to_string(),
)); ));
} }
@ -190,10 +191,11 @@ fn parse_query_authorization(
let date = headers let date = headers
.get("x-amz-date") .get("x-amz-date")
.ok_or_bad_request("Missing X-Amz-Date field") .ok_or_bad_request("Missing X-Amz-Date field")
.map_err(Error::from)
.and_then(|d| parse_date(d))?; .and_then(|d| parse_date(d))?;
if Utc::now() - date > Duration::seconds(duration) { if Utc::now() - date > Duration::seconds(duration) {
return Err(Error::BadRequest("Date is too old".to_string())); return Err(Error::bad_request("Date is too old".to_string()));
} }
Ok(Authorization { Ok(Authorization {

View file

@ -87,7 +87,7 @@ fn compute_streaming_payload_signature(
let mut hmac = signing_hmac.clone(); let mut hmac = signing_hmac.clone();
hmac.update(string_to_sign.as_bytes()); hmac.update(string_to_sign.as_bytes());
Hash::try_from(&hmac.finalize().into_bytes()).ok_or_internal_error("Invalid signature") Ok(Hash::try_from(&hmac.finalize().into_bytes()).ok_or_internal_error("Invalid signature")?)
} }
mod payload { mod payload {
@ -163,10 +163,10 @@ impl From<SignedPayloadStreamError> for Error {
match err { match err {
SignedPayloadStreamError::Stream(e) => e, SignedPayloadStreamError::Stream(e) => e,
SignedPayloadStreamError::InvalidSignature => { SignedPayloadStreamError::InvalidSignature => {
Error::BadRequest("Invalid payload signature".into()) Error::bad_request("Invalid payload signature")
} }
SignedPayloadStreamError::Message(e) => { SignedPayloadStreamError::Message(e) => {
Error::BadRequest(format!("Chunk format error: {}", e)) Error::bad_request(format!("Chunk format error: {}", e))
} }
} }
} }

View file

@ -3,50 +3,39 @@ use hyper::header::HeaderValue;
use hyper::{HeaderMap, StatusCode}; use hyper::{HeaderMap, StatusCode};
use garage_api::generic_server::ApiError; use garage_api::generic_server::ApiError;
use garage_util::error::Error as GarageError;
/// Errors of this crate /// Errors of this crate
#[derive(Debug, Error)] #[derive(Debug, Error)]
pub enum Error { pub enum Error {
/// An error received from the API crate /// An error received from the API crate
#[error(display = "API error: {}", _0)] #[error(display = "API error: {}", _0)]
ApiError(#[error(source)] garage_api::Error), ApiError(garage_api::Error),
// Category: internal error
/// Error internal to garage
#[error(display = "Internal error: {}", _0)]
InternalError(#[error(source)] GarageError),
/// The file does not exist /// The file does not exist
#[error(display = "Not found")] #[error(display = "Not found")]
NotFound, NotFound,
/// The request contained an invalid UTF-8 sequence in its path or in other parameters
#[error(display = "Invalid UTF-8: {}", _0)]
InvalidUtf8(#[error(source)] std::str::Utf8Error),
/// The client send a header with invalid value
#[error(display = "Invalid header value: {}", _0)]
InvalidHeader(#[error(source)] hyper::header::ToStrError),
/// The client sent a request without host, or with unsupported method /// The client sent a request without host, or with unsupported method
#[error(display = "Bad request: {}", _0)] #[error(display = "Bad request: {}", _0)]
BadRequest(String), BadRequest(String),
} }
impl<T> From<T> for Error
where
garage_api::Error: From<T>,
{
fn from(err: T) -> Self {
Error::ApiError(garage_api::Error::from(err))
}
}
impl Error { impl Error {
/// Transform errors into http status code /// Transform errors into http status code
pub fn http_status_code(&self) -> StatusCode { pub fn http_status_code(&self) -> StatusCode {
match self { match self {
Error::NotFound => StatusCode::NOT_FOUND, Error::NotFound => StatusCode::NOT_FOUND,
Error::ApiError(e) => e.http_status_code(), Error::ApiError(e) => e.http_status_code(),
Error::InternalError( Error::BadRequest(_) => StatusCode::BAD_REQUEST,
GarageError::Timeout
| GarageError::RemoteError(_)
| GarageError::Quorum(_, _, _, _),
) => StatusCode::SERVICE_UNAVAILABLE,
Error::InternalError(_) => StatusCode::INTERNAL_SERVER_ERROR,
_ => StatusCode::BAD_REQUEST,
} }
} }

View file

@ -207,7 +207,7 @@ async fn serve_file(garage: Arc<Garage>, req: &Request<Body>) -> Result<Response
Method::OPTIONS => handle_options_for_bucket(req, &bucket), Method::OPTIONS => handle_options_for_bucket(req, &bucket),
Method::HEAD => handle_head(garage.clone(), req, bucket_id, &key, None).await, Method::HEAD => handle_head(garage.clone(), req, bucket_id, &key, None).await,
Method::GET => handle_get(garage.clone(), req, bucket_id, &key, None).await, Method::GET => handle_get(garage.clone(), req, bucket_id, &key, None).await,
_ => Err(ApiError::BadRequest("HTTP method not supported".into())), _ => Err(ApiError::bad_request("HTTP method not supported")),
} }
.map_err(Error::from); .map_err(Error::from);
@ -290,9 +290,7 @@ fn path_to_key<'a>(path: &'a str, index: &str) -> Result<Cow<'a, str>, Error> {
let path_utf8 = percent_encoding::percent_decode_str(path).decode_utf8()?; let path_utf8 = percent_encoding::percent_decode_str(path).decode_utf8()?;
if !path_utf8.starts_with('/') { if !path_utf8.starts_with('/') {
return Err(Error::BadRequest( return Err(Error::BadRequest("Path must start with a / (slash)".into()));
"Path must start with a / (slash)".to_string(),
));
} }
match path_utf8.chars().last() { match path_utf8.chars().last() {