From 168a90dfb5489d465d64f066f375e5d06bc1f08c Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Wed, 5 Jan 2022 17:07:36 +0100 Subject: Fix some error codes --- src/api/api_server.rs | 3 ++- src/api/error.rs | 30 ++++++++++++++++++++++++++---- src/api/s3_bucket.rs | 10 ++-------- src/api/s3_copy.rs | 8 ++++---- src/api/s3_delete.rs | 4 ++-- src/api/s3_get.rs | 14 +++++++------- src/api/s3_put.rs | 16 ++++++++-------- src/api/s3_website.rs | 4 ++-- 8 files changed, 53 insertions(+), 36 deletions(-) diff --git a/src/api/api_server.rs b/src/api/api_server.rs index 41ae6857..41aa0046 100644 --- a/src/api/api_server.rs +++ b/src/api/api_server.rs @@ -107,6 +107,7 @@ async fn handler_inner(garage: Arc, req: Request) -> Result StatusCode { match self { - Error::NotFound => StatusCode::NOT_FOUND, + Error::NoSuchKey | Error::NoSuchBucket | Error::NoSuchUpload => StatusCode::NOT_FOUND, + Error::BucketNotEmpty | Error::BucketAlreadyExists => StatusCode::CONFLICT, Error::Forbidden(_) => StatusCode::FORBIDDEN, Error::InternalError( GarageError::Timeout @@ -115,9 +132,14 @@ impl Error { pub fn aws_code(&self) -> &'static str { match self { - Error::NotFound => "NoSuchKey", + Error::NoSuchKey => "NoSuchKey", + Error::NoSuchBucket => "NoSuchBucket", + Error::NoSuchUpload => "NoSuchUpload", + Error::BucketAlreadyExists => "BucketAlreadyExists", + Error::BucketNotEmpty => "BucketNotEmpty", Error::Forbidden(_) => "AccessDenied", Error::AuthorizationHeaderMalformed(_) => "AuthorizationHeaderMalformed", + Error::NotImplemented(_) => "NotImplemented", Error::InternalError( GarageError::Timeout | GarageError::RemoteError(_) diff --git a/src/api/s3_bucket.rs b/src/api/s3_bucket.rs index ea7e0b8c..fb3e982d 100644 --- a/src/api/s3_bucket.rs +++ b/src/api/s3_bucket.rs @@ -153,10 +153,7 @@ pub async fn handle_create_bucket( // otherwise return a forbidden error. let kp = api_key.bucket_permissions(&bucket_id); if !(kp.allow_write || kp.allow_owner) { - return Err(Error::Forbidden(format!( - "Key {} does not have write or owner permissions on bucket {}", - api_key.key_id, bucket_name - ))); + return Err(Error::BucketAlreadyExists); } } else { // Create the bucket! @@ -231,10 +228,7 @@ pub async fn handle_delete_bucket( .get_range(&bucket_id, None, Some(DeletedFilter::NotDeleted), 10) .await?; if !objects.is_empty() { - return Err(Error::BadRequest(format!( - "Bucket {} is not empty", - bucket_name - ))); + return Err(Error::BucketNotEmpty); } // --- done checking, now commit --- diff --git a/src/api/s3_copy.rs b/src/api/s3_copy.rs index 4ede8230..7952dae8 100644 --- a/src/api/s3_copy.rs +++ b/src/api/s3_copy.rs @@ -27,14 +27,14 @@ pub async fn handle_copy( .object_table .get(&source_bucket_id, &source_key.to_string()) .await? - .ok_or(Error::NotFound)?; + .ok_or(Error::NoSuchKey)?; let source_last_v = source_object .versions() .iter() .rev() .find(|v| v.is_complete()) - .ok_or(Error::NotFound)?; + .ok_or(Error::NoSuchKey)?; let source_last_state = match &source_last_v.state { ObjectVersionState::Complete(x) => x, @@ -47,7 +47,7 @@ pub async fn handle_copy( // Implement x-amz-metadata-directive: REPLACE let old_meta = match source_last_state { ObjectVersionData::DeleteMarker => { - return Err(Error::NotFound); + return Err(Error::NoSuchKey); } ObjectVersionData::Inline(meta, _bytes) => meta, ObjectVersionData::FirstBlock(meta, _fbh) => meta, @@ -88,7 +88,7 @@ pub async fn handle_copy( .version_table .get(&source_last_v.uuid, &EmptyKey) .await?; - let source_version = source_version.ok_or(Error::NotFound)?; + let source_version = source_version.ok_or(Error::NoSuchKey)?; // Write an "uploading" marker in Object table // This holds a reference to the object in the Version table diff --git a/src/api/s3_delete.rs b/src/api/s3_delete.rs index 1976139b..9e267490 100644 --- a/src/api/s3_delete.rs +++ b/src/api/s3_delete.rs @@ -21,7 +21,7 @@ async fn handle_delete_internal( .object_table .get(&bucket_id, &key.to_string()) .await? - .ok_or(Error::NotFound)?; // No need to delete + .ok_or(Error::NoSuchKey)?; // No need to delete let interesting_versions = object.versions().iter().filter(|v| { !matches!( @@ -40,7 +40,7 @@ async fn handle_delete_internal( timestamp = std::cmp::max(timestamp, v.timestamp + 1); } - let deleted_version = version_to_delete.ok_or(Error::NotFound)?; + let deleted_version = version_to_delete.ok_or(Error::NoSuchKey)?; let version_uuid = gen_uuid(); diff --git a/src/api/s3_get.rs b/src/api/s3_get.rs index 269a3fa8..67ab2b59 100644 --- a/src/api/s3_get.rs +++ b/src/api/s3_get.rs @@ -92,14 +92,14 @@ pub async fn handle_head( .object_table .get(&bucket_id, &key.to_string()) .await? - .ok_or(Error::NotFound)?; + .ok_or(Error::NoSuchKey)?; let version = object .versions() .iter() .rev() .find(|v| v.is_data()) - .ok_or(Error::NotFound)?; + .ok_or(Error::NoSuchKey)?; let version_meta = match &version.state { ObjectVersionState::Complete(ObjectVersionData::Inline(meta, _)) => meta, @@ -131,21 +131,21 @@ pub async fn handle_get( .object_table .get(&bucket_id, &key.to_string()) .await? - .ok_or(Error::NotFound)?; + .ok_or(Error::NoSuchKey)?; let last_v = object .versions() .iter() .rev() .find(|v| v.is_complete()) - .ok_or(Error::NotFound)?; + .ok_or(Error::NoSuchKey)?; let last_v_data = match &last_v.state { ObjectVersionState::Complete(x) => x, _ => unreachable!(), }; let last_v_meta = match last_v_data { - ObjectVersionData::DeleteMarker => return Err(Error::NotFound), + ObjectVersionData::DeleteMarker => return Err(Error::NoSuchKey), ObjectVersionData::Inline(meta, _) => meta, ObjectVersionData::FirstBlock(meta, _) => meta, }; @@ -196,7 +196,7 @@ pub async fn handle_get( let get_next_blocks = garage.version_table.get(&last_v.uuid, &EmptyKey); let (first_block, version) = futures::try_join!(read_first_block, get_next_blocks)?; - let version = version.ok_or(Error::NotFound)?; + let version = version.ok_or(Error::NoSuchKey)?; let mut blocks = version .blocks @@ -261,7 +261,7 @@ async fn handle_get_range( let version = garage.version_table.get(&version.uuid, &EmptyKey).await?; let version = match version { Some(v) => v, - None => return Err(Error::NotFound), + None => return Err(Error::NoSuchKey), }; // We will store here the list of blocks that have an intersection with the requested diff --git a/src/api/s3_put.rs b/src/api/s3_put.rs index 152e59b4..bb92c252 100644 --- a/src/api/s3_put.rs +++ b/src/api/s3_put.rs @@ -382,7 +382,7 @@ pub async fn handle_put_part( .iter() .any(|v| v.uuid == version_uuid && v.is_uploading()) { - return Err(Error::NotFound); + return Err(Error::NoSuchUpload); } // Copy block to store @@ -449,15 +449,15 @@ pub async fn handle_complete_multipart_upload( garage.version_table.get(&version_uuid, &EmptyKey), )?; - let object = object.ok_or_else(|| Error::BadRequest("Object not found".to_string()))?; + let object = object.ok_or(Error::NoSuchKey)?; let mut object_version = object .versions() .iter() .find(|v| v.uuid == version_uuid && v.is_uploading()) .cloned() - .ok_or_else(|| Error::BadRequest("Version not found".to_string()))?; + .ok_or(Error::NoSuchUpload)?; - let version = version.ok_or_else(|| Error::BadRequest("Version not found".to_string()))?; + let version = version.ok_or(Error::NoSuchKey)?; if version.blocks.is_empty() { return Err(Error::BadRequest("No data was uploaded".to_string())); } @@ -538,14 +538,14 @@ pub async fn handle_abort_multipart_upload( .object_table .get(&bucket_id, &key.to_string()) .await?; - let object = object.ok_or_else(|| Error::BadRequest("Object not found".to_string()))?; + let object = object.ok_or(Error::NoSuchKey)?; let object_version = object .versions() .iter() .find(|v| v.uuid == version_uuid && v.is_uploading()); let mut object_version = match object_version { - None => return Err(Error::NotFound), + None => return Err(Error::NoSuchUpload), Some(x) => x.clone(), }; @@ -611,9 +611,9 @@ pub(crate) fn get_headers(req: &Request) -> Result Result { - let id_bin = hex::decode(id).ok_or_bad_request("Invalid upload ID")?; + let id_bin = hex::decode(id).map_err(|_| Error::NoSuchUpload)?; if id_bin.len() != 32 { - return None.ok_or_bad_request("Invalid upload ID"); + return Err(Error::NoSuchUpload); } let mut uuid = [0u8; 32]; uuid.copy_from_slice(&id_bin[..]); diff --git a/src/api/s3_website.rs b/src/api/s3_website.rs index e141e449..85d7c261 100644 --- a/src/api/s3_website.rs +++ b/src/api/s3_website.rs @@ -22,7 +22,7 @@ pub async fn handle_delete_website( .bucket_table .get(&EmptyKey, &bucket_id) .await? - .ok_or(Error::NotFound)?; + .ok_or(Error::NoSuchBucket)?; if let crdt::Deletable::Present(param) = &mut bucket.state { param.website_config.update(None); @@ -50,7 +50,7 @@ pub async fn handle_put_website( .bucket_table .get(&EmptyKey, &bucket_id) .await? - .ok_or(Error::NotFound)?; + .ok_or(Error::NoSuchBucket)?; let conf: WebsiteConfiguration = from_reader(&body as &[u8])?; conf.validate()?; -- cgit v1.2.3