Return 400 bad request for non implemented API #161

New issue

Closed

opened 2021-11-24 09:41:06 +00:00 by quentin · 1 comment

quentin commented

2021-11-24 09:41:06 +00:00

Owner

In the doc we say:

All APIs that are not mentionned are not implemented and will return a 400 bad request.

But in fact, in many cases we are returning a 200 OK or 404 Not Found with confusing data.

For example, currently we have this logic for request on keys:

Method::GET => {
  Ok(handle_get(garage, &req, bucket, key).await?)
}

Once we will have implemented ListPart, wewill have the following logic:

Method::GET => {
  if params.contains_key("uploadid") {
    // ListParts
    Ok(handle_list_part(garage, &req, bucket, key).await?)
  } else {
    // GetObject query
    Ok(handle_get(garage, &req, bucket, key).await?)
  }
}

So currently, we wrongly forward ListPart to GetObject without any idea on how it will handle the request, and this pattern is true for most of the non-implemented endpoints.

Instead, if we want proper 400 bad requests, we should list all the S3 endpoints and add an error entry. For ListPart, we would add:

Method::GET => {
  if params.contains_key("uploadid") {
    // ListParts
    Err(Error::BadRequest("ListPart is not implemented".to_string()))  
  } else {
    // GetObject query
    Ok(handle_get(garage, &req, bucket, key).await?)
  }
}

Ideally, we would also update the doc with the full list of S3 endpoints.
I think to this page: https://garagehq.deuxfleurs.fr/reference_manual/s3_compatibility.html

How to quickly generate a list of all S3 endpoints:

Go to https://docs.aws.amazon.com/AmazonS3/latest/API/API_Operations_Amazon_Simple_Storage_Service.html
Open your developper console (F12)
Adapt the following one liner

Array.from(document.querySelectorAll("li.listitem > p:nth-child(1) > a")).map(e => ` - [${e.innerText}](${e.href})`).join("\n")

The result is 96 endpoints (for now, we support ~16):

In the doc we say: > All APIs that are not mentionned are not implemented and will return a 400 bad request. But in fact, in many cases we are returning a `200 OK` or `404 Not Found` with confusing data. For example, currently we have this logic for request on keys: ```rust Method::GET => { Ok(handle_get(garage, &req, bucket, key).await?) } ``` Once we will have implemented `ListPart`, wewill have the following logic: ```rust Method::GET => { if params.contains_key("uploadid") { // ListParts Ok(handle_list_part(garage, &req, bucket, key).await?) } else { // GetObject query Ok(handle_get(garage, &req, bucket, key).await?) } } ``` So currently, we wrongly forward ListPart to GetObject without any idea on how it will handle the request, and this pattern is true for most of the non-implemented endpoints. Instead, if we want proper 400 bad requests, we should list all the S3 endpoints and add an error entry. For `ListPart`, we would add: ```rust Method::GET => { if params.contains_key("uploadid") { // ListParts Err(Error::BadRequest("ListPart is not implemented".to_string())) } else { // GetObject query Ok(handle_get(garage, &req, bucket, key).await?) } } ``` --- Ideally, we would also update the doc with the full list of S3 endpoints. I think to this page: https://garagehq.deuxfleurs.fr/reference_manual/s3_compatibility.html --- How to quickly generate a list of all S3 endpoints: - Go to https://docs.aws.amazon.com/AmazonS3/latest/API/API_Operations_Amazon_Simple_Storage_Service.html - Open your developper console (F12) - Adapt the following one liner ```javascript Array.from(document.querySelectorAll("li.listitem > p:nth-child(1) > a")).map(e => ` - [${e.innerText}](${e.href})`).join("\n") ``` The result is 96 endpoints (for now, we support ~16): - [AbortMultipartUpload](https://docs.aws.amazon.com/AmazonS3/latest/API/API_AbortMultipartUpload.html) - [CompleteMultipartUpload](https://docs.aws.amazon.com/AmazonS3/latest/API/API_CompleteMultipartUpload.html) - [CopyObject](https://docs.aws.amazon.com/AmazonS3/latest/API/API_CopyObject.html) - [CreateBucket](https://docs.aws.amazon.com/AmazonS3/latest/API/API_CreateBucket.html) - [CreateMultipartUpload](https://docs.aws.amazon.com/AmazonS3/latest/API/API_CreateMultipartUpload.html) - [DeleteBucket](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteBucket.html) - [DeleteBucketAnalyticsConfiguration](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteBucketAnalyticsConfiguration.html) - [DeleteBucketCors](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteBucketCors.html) - [DeleteBucketEncryption](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteBucketEncryption.html) - [DeleteBucketIntelligentTieringConfiguration](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteBucketIntelligentTieringConfiguration.html) - [DeleteBucketInventoryConfiguration](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteBucketInventoryConfiguration.html) - [DeleteBucketLifecycle](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteBucketLifecycle.html) - [DeleteBucketMetricsConfiguration](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteBucketMetricsConfiguration.html) - [DeleteBucketOwnershipControls](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteBucketOwnershipControls.html) - [DeleteBucketPolicy](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteBucketPolicy.html) - [DeleteBucketReplication](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteBucketReplication.html) - [DeleteBucketTagging](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteBucketTagging.html) - [DeleteBucketWebsite](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteBucketWebsite.html) - [DeleteObject](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObject.html) - [DeleteObjects](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObjects.html) - [DeleteObjectTagging](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObjectTagging.html) - [DeletePublicAccessBlock](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeletePublicAccessBlock.html) - [GetBucketAccelerateConfiguration](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAccelerateConfiguration.html) - [GetBucketAcl](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html) - [GetBucketAnalyticsConfiguration](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAnalyticsConfiguration.html) - [GetBucketCors](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketCors.html) - [GetBucketEncryption](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketEncryption.html) - [GetBucketIntelligentTieringConfiguration](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketIntelligentTieringConfiguration.html) - [GetBucketInventoryConfiguration](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketInventoryConfiguration.html) - [GetBucketLifecycle](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketLifecycle.html) - [GetBucketLifecycleConfiguration](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketLifecycleConfiguration.html) - [GetBucketLocation](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketLocation.html) - [GetBucketLogging](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketLogging.html) - [GetBucketMetricsConfiguration](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketMetricsConfiguration.html) - [GetBucketNotification](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketNotification.html) - [GetBucketNotificationConfiguration](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketNotificationConfiguration.html) - [GetBucketOwnershipControls](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketOwnershipControls.html) - [GetBucketPolicy](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketPolicy.html) - [GetBucketPolicyStatus](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketPolicyStatus.html) - [GetBucketReplication](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketReplication.html) - [GetBucketRequestPayment](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketRequestPayment.html) - [GetBucketTagging](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketTagging.html) - [GetBucketVersioning](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketVersioning.html) - [GetBucketWebsite](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketWebsite.html) - [GetObject](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetObject.html) - [GetObjectAcl](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetObjectAcl.html) - [GetObjectLegalHold](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetObjectLegalHold.html) - [GetObjectLockConfiguration](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetObjectLockConfiguration.html) - [GetObjectRetention](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetObjectRetention.html) - [GetObjectTagging](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetObjectTagging.html) - [GetObjectTorrent](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetObjectTorrent.html) - [GetPublicAccessBlock](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetPublicAccessBlock.html) - [HeadBucket](https://docs.aws.amazon.com/AmazonS3/latest/API/API_HeadBucket.html) - [HeadObject](https://docs.aws.amazon.com/AmazonS3/latest/API/API_HeadObject.html) - [ListBucketAnalyticsConfigurations](https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListBucketAnalyticsConfigurations.html) - [ListBucketIntelligentTieringConfigurations](https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListBucketIntelligentTieringConfigurations.html) - [ListBucketInventoryConfigurations](https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListBucketInventoryConfigurations.html) - [ListBucketMetricsConfigurations](https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListBucketMetricsConfigurations.html) - [ListBuckets](https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListBuckets.html) - [ListMultipartUploads](https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListMultipartUploads.html) - [ListObjects](https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjects.html) - [ListObjectsV2](https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjectsV2.html) - [ListObjectVersions](https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjectVersions.html) - [ListParts](https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListParts.html) - [PutBucketAccelerateConfiguration](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAccelerateConfiguration.html) - [PutBucketAcl](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAcl.html) - [PutBucketAnalyticsConfiguration](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAnalyticsConfiguration.html) - [PutBucketCors](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketCors.html) - [PutBucketEncryption](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketEncryption.html) - [PutBucketIntelligentTieringConfiguration](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketIntelligentTieringConfiguration.html) - [PutBucketInventoryConfiguration](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketInventoryConfiguration.html) - [PutBucketLifecycle](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketLifecycle.html) - [PutBucketLifecycleConfiguration](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketLifecycleConfiguration.html) - [PutBucketLogging](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketLogging.html) - [PutBucketMetricsConfiguration](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketMetricsConfiguration.html) - [PutBucketNotification](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketNotification.html) - [PutBucketNotificationConfiguration](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketNotificationConfiguration.html) - [PutBucketOwnershipControls](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketOwnershipControls.html) - [PutBucketPolicy](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketPolicy.html) - [PutBucketReplication](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketReplication.html) - [PutBucketRequestPayment](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketRequestPayment.html) - [PutBucketTagging](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketTagging.html) - [PutBucketVersioning](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketVersioning.html) - [PutBucketWebsite](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketWebsite.html) - [PutObject](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutObject.html) - [PutObjectAcl](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutObjectAcl.html) - [PutObjectLegalHold](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutObjectLegalHold.html) - [PutObjectLockConfiguration](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutObjectLockConfiguration.html) - [PutObjectRetention](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutObjectRetention.html) - [PutObjectTagging](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutObjectTagging.html) - [PutPublicAccessBlock](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutPublicAccessBlock.html) - [RestoreObject](https://docs.aws.amazon.com/AmazonS3/latest/API/API_RestoreObject.html) - [SelectObjectContent](https://docs.aws.amazon.com/AmazonS3/latest/API/API_SelectObjectContent.html) - [UploadPart](https://docs.aws.amazon.com/AmazonS3/latest/API/API_UploadPart.html) - [UploadPartCopy](https://docs.aws.amazon.com/AmazonS3/latest/API/API_UploadPartCopy.html) - [WriteGetObjectResponse](https://docs.aws.amazon.com/AmazonS3/latest/API/API_WriteGetObjectResponse.html)

👍 2

quentin added the

labels 2021-11-24 09:41:06 +00:00

trinity-1686a commented

2021-11-27 22:38:13 +00:00

Owner

I believe WriteGetObjectResponse should be excluded from the list.

Every endpoints except for WriteGetObjectResponse and ListBuckets uses the following header:
Host: Bucket.s3.amazonaws.com (but mentionning Bucket.s3.<Region>.amazonaws.com instead in examples)

ListBuckets does not use this header because it is the only endpoint not related to a specific bucket.

WriteGetObjectResponse on the other hand, use this header:
Host: <RequestRoute>.s3-object-lambda.<Region>.amazonaws.com (notice "s3-object-lambda" instead of "s3")
I'm not sure to understand exactly what this endpoint does (post-processing Get requests if my understanding is right), but it seems more related to Amazon lambdas and their integration with S3, than S3 itself.

I also fail to see any difference between GetBucketNotification and GetBucketNotificationConfiguration, which makes me thing the former is not deprecated as much as it has been renamed to the latter, and could be excluded too.
Same goes for PutBucketNotification and PutBucketNotificationConfiguration, GetBucketLifecycle and GetBucketLifecycleConfiguration, and PutBucketLifecycle and PutBucketLifecycleConfiguration.

I believe `WriteGetObjectResponse` should be excluded from the list. Every endpoints except for `WriteGetObjectResponse` and `ListBuckets` uses the following header: <code>Host: **_Bucket_**.s3.amazonaws.com</code> (but mentionning <code>**_Bucket_**.s3.**_\<Region\>_**.amazonaws.com</code> instead in examples) `ListBuckets` does not use this header because it is the _only_ endpoint not related to a specific bucket. `WriteGetObjectResponse` on the other hand, use this header: <code>Host: **_\<RequestRoute\>_**.s3-object-lambda.**_\<Region\>_**.amazonaws.com</code> (notice "s3-object-lambda" instead of "s3") I'm not sure to understand exactly what this endpoint does (post-processing Get requests if my understanding is right), but it seems more related to Amazon lambdas and their integration with S3, than S3 itself. I also fail to see any difference between `GetBucketNotification` and `GetBucketNotificationConfiguration`, which makes me thing the former is not deprecated as much as it has been renamed to the latter, and could be excluded too. Same goes for `PutBucketNotification` and `PutBucketNotificationConfiguration`, `GetBucketLifecycle` and `GetBucketLifecycleConfiguration`, and `PutBucketLifecycle` and `PutBucketLifecycleConfiguration`.

👍 1