S3 headIncompletePartForUpload function error exclusions seem insufficient

[jimydavis]

In s3store.go, there's a number of errors that are ignored as below. I presume this is for fresh new uploads that have no incomplete parts:

func (store S3Store) headIncompletePartForUpload(ctx context.Context, uploadId string) (int64, error) {

...

  if err != nil {
    if isAwsError[*types.NoSuchKey](err) || isAwsError[*types.NotFound](err) || isAwsErrorCode(err, "AccessDenied") {
      err = nil
    }
    return 0, err
  }

However, yesterday on testing a fresh upload (no parts uploaded yet, completely fresh), I received

status=500 body="ERR_INTERNAL_SERVER_ERROR: operation error S3: HeadObject, https response error StatusCode: 403, RequestID: xxxxxxx, HostID: uploadID+multipartID, api error Forbidden: Forbidden\n"

In my debugger, the error looked like this:

Do we have to exclude Forbidden for fresh uploads without incomplete parts too or am I missing something?

Upon reading the AWS error codes documentation, Forbidden is not an error code. Only AccessDenied is. But as you can see from my debugger screenshot, I am indeed getting the Forbidden code from what presumably must be from AWS's servers and not within any of the client code.

Lastly, many people should encounter this error as incomplete parts are not always going to be there and when you do a HEAD on it, its going to come back empty.

Thank you.

edit: this seems to affect getIncompletePartForUpload too.

[Acconut]

Do you have allowed all of these actions?

https://github.com/tus/tusd/blob/5e9be5a18d3672b12d8fe7fd80bc26baae582472/pkg/s3store/s3store.go#L9-L13

I don't know the exact details but depending on the permissions AWS S3 either returns a 404 Not Found or 403 Forbidden error for non-existing objects. It could also be that we are missing a permission in the above documentation.

The current code only handles the 404 case. We should also handle 403 better while also checking the documentation for missing permissions.

[jimydavis]

Yes I have all these permissions. Everything else works fine except when it tries to HEAD or GET .part files which don't exist yet for a fresh upload.

In fact after extensive testing, I have no idea when and when a .part file will appear. Sometimes an actual .part file appears in the S3 console browser. Other times, it goes through the shadow multi-part system which does not show up on the S3 GUI console. In both cases, I have seen the upload complete successfully.

Adding || isAwsErrorCode(err, "Forbidden") just makes the error go away. Full code below:

if err != nil && (isAwsError[*types.NoSuchKey](err) || isAwsError[*types.NotFound](err) || isAwsErrorCode(err, "AccessDenied")) || isAwsErrorCode(err, "Forbidden") {
  return nil, nil
}

In the AWS error code documentation: https://docs.aws.amazon.com/AmazonS3/latest/API/ErrorResponses.html , you can see Forbidden is not an error code. It is just a HTTP status.

[jimydavis]

@Acconut Hello! Wondering if you thought this has any merit for an addition of the Forbidden error type? Thanks! Congrats on launching 2.0.0 🥳

[Acconut]

In fact after extensive testing, I have no idea when and when a .part file will appear. Sometimes an actual .part file appears in the S3 console browser.

The .part files are sometimes created when an upload is paused in the middle of the transfer. S3 has a minimum part size for its multipart uploads and if the amount of remaining data is smaller than this minimum part size, we store it in a .part file, which will be consumed when the upload is resumed.

Wondering if you thought this has any merit for an addition of the Forbidden error type?

Yes, that would be a sensible change. Feel free to open a PR for it. And thanks for the kind words :)

[jimydavis]

Created a pull request here: https://github.com/tus/tusd/pull/1019 - if anyone has friends at AWS, it would help to know why AWS returns "Forbidden" as an Error Code when the documentation does not mention it: https://docs.aws.amazon.com/AmazonS3/latest/API/ErrorResponses.html

[Acconut]

Thank you for the PR, @jimydavis. I hope this resolve the issue. If not, please let me know and we will reopen this.

it would help to know why AWS returns "Forbidden" as an Error Code when the documentation does not mention it

One reason could be that Forbidden is not an S3-specific error but a general IAM error and thus not included in this list.