Skip to content

defense-in-depth check for <Error> in CompleteMultipartUpload respons…#622

Open
cloud4t0r wants to merge 1 commit into
awslabs:mainfrom
cloud4t0r:main
Open

defense-in-depth check for <Error> in CompleteMultipartUpload respons…#622
cloud4t0r wants to merge 1 commit into
awslabs:mainfrom
cloud4t0r:main

Conversation

@cloud4t0r
Copy link
Copy Markdown

Issue #, if available:

The issue stems from a documented edge case by AWS: when you perform a CompleteMultipartUpload, S3 may return an HTTP 200 OK with an XML body containing an instead of the expected . This typically occurs during internal S3 overloads (InternalError). Without this patch, the SDK treated the 200 as a success and continued, even though the multipart upload had actually failed.

this patch address this todo in the original code :
/**
* TODO: The body of the response can be ERROR, check Error specified in body part from
* https://docs.aws.amazon.com/AmazonS3/latest/API/API_CompleteMultipartUpload.html#AmazonS3-CompleteMultipartUpload-response-CompleteMultipartUploadOutput
* We need to handle this case.
* TODO: the checksum returned within the response of complete multipart upload need to be exposed?
*/

Description of changes:
The patch adds three protections in the AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_COMPLETE_MULTIPART_UPLOAD handler:

Detection of <Error><Code> in the body: Even if error_code == AWS_ERROR_SUCCESS (HTTP 200), we parse the XML body to look for an <Error><Code>. If found, we map the S3 error to a CRT error code (or AWS_ERROR_S3_NON_RECOVERABLE_ASYNC_ERROR if unknown). This is an extra safety net in addition to the generic mechanism in s3_meta_request.c, which already performs this check beforehand but might miss it in some timing cases (body arriving after send_request_finish).

Verification of the ETag presence: On a real success, the body must contain a <CompleteMultipartUploadResult> with an <ETag>. If we get a 200 but no ETag (empty body, truncated body, or unexpected content), it must be an issue → we force AWS_ERROR_S3_NON_RECOVERABLE_ASYNC_ERROR.

Conditional invocation of the headers_callback: Before the patch, the user callback (headers_callback) was always invoked on a 200, even if the body contained an error. Now it is only called if error_code is still AWS_ERROR_SUCCESS after the two checks above. This prevents notifying the caller of a "success" that is not actually one.

In summary: this is defense‑in‑depth so that the calling code never treats a multipart upload as successful if S3 has returned an error disguised as a 200 OK.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant