Siyao Meng created HDDS-17273:
---------------------------------
Summary: S3 PUT or UploadPart that fails with an Error after the
body is copied still commits, skipping the Content-MD5 and x-amz-content-sha256
checks
Key: HDDS-17273
URL: https://issues.apache.org/jira/browse/HDDS-17273
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments: MC-5-commit-s3-write-only-after-transfer-complete.patch,
TestBugMC5ErrorAfterBodyCopyCommits.java
h3. Mechanism
The S3 write paths commit the key in {{close()}} of a try-with-resources
{{S3ObjectWriteGuard}}. The guard's pre-commit blocks the commit only if
{{copyFrom}} recorded an {{IOException}} or {{RuntimeException}}, or if the
length does not match. The Content-MD5 and x-amz-content-sha256 checks, the
ETag and, for parts, {{requirePartETag}} are added only after {{copyFrom}}
returns.
So anything thrown after the last body byte was copied leaves the commit open:
an {{Error}} from {{SignedChunksInputStream.verifyComplete}} (the final chunk
check inside {{copyFrom}}), or any throwable in the endpoint code that follows
(digest, ETag, Content-MD5 header lookup, hook registration). {{close()}} then
commits the content without the digest checks, and without an ETag if the
throwable came before the ETag was put. An Error during the copy loop itself is
blocked by the length check. In the current code no {{RuntimeException}} can
occur in that window, so the realistic trigger is {{OutOfMemoryError}}.
The same pattern is in PutObject, UploadPart, UploadPartCopy and CopyObject in
{{ObjectEndpoint}} and in the datastream put, copy and part upload in
{{ObjectEndpointStreaming}}.
h3. Trigger
# A client PUTs an object (or part) whose body does not match its Content-MD5
or x-amz-content-sha256.
# The gateway hits an {{OutOfMemoryError}} after the whole body has been read
and before the digest checks are added.
The reproduction injects the Error through the mocked request; a real out of
memory condition at that point was not produced.
h3. Impact
The client gets an error, but content that its own digest would have rejected
replaces the previous object (or becomes the part), in the signed chunked case
without an ETag. A retry with the correct body overwrites it, except with
{{If-None-Match: *}}. With a body that matches its digest the effect is an
error response for a write that was committed, in some paths without an ETag
(the signed chunked PUT, and UploadPart, where the Content-MD5 lookup comes
before the ETag is set). Committing unchecked content needs two independent
faults at once, so it is unlikely.
h3. Reproduction
[^TestBugMC5ErrorAfterBodyCopyCommits.java] ({{hadoop-ozone/s3gateway}}) runs
the real {{ObjectEndpoint}} and {{S3ObjectWriteGuard}} against the gateway's
{{OzoneClientStub}}. Every request carries a digest that does not match the
body. Without an Error such a request is rejected and the previous object
stays. With an {{OutOfMemoryError}} from the Content-MD5 header lookup (done
after the copy), or from the request body when the final chunk of a signed
chunked upload is read, the PUT replaces the previous object with the unchecked
content, the signed PUT leaves an object without ETag, and the UploadPart
commits the part. On the unmodified source all four tests pass, which means the
defect is present. An UploadPart without ETag against a real OM follows from
the same code but was not run.
h3. Patch
[^MC-5-commit-s3-write-only-after-transfer-complete.patch], against
b10a7d427bdf83807a7e48a1be507d3b02d5a503. It also applies to master at
e0de543cbe9.
The guard commits only after the request called the new
{{S3ObjectWriteGuard.markTransferComplete}}, which each of the seven write
blocks does as its last statement, after all checks are added. Its existing
pre-commit refuses the commit otherwise, so any throwable before that point
aborts the write. Successful requests and the existing error responses are
unchanged.
The patch applies together with the one for HDDS-17272 without conflict.
Whichever of the two lands second needs {{guard.markTransferComplete();}}
before {{guard.close();}} in
{{TestS3ObjectWriteGuard.encryptedWriteCommitsAllContent}}, the test HDDS-17272
adds.
Covered by {{testPutObjectNotCommittedOnErrorAfterBodyCopy}} in the existing
{{TestObjectPut}}, {{testPartUploadNotCommittedOnErrorAfterBodyCopy}} in
{{TestPartUpload}} (with and without datastream) and two contract tests in
{{TestS3ObjectWriteGuard}}. Without the change the endpoint tests fail because
the key or part is committed. With it all s3gateway unit tests (1623) and the
AWS SDK suites {{TestS3SDK}} and {{TestS3SDKWithRatisStreaming}} on a mini
cluster (490) pass, and checkstyle is clean. With the patch the three failure
cases of the reproduction fail, because nothing is committed.
Found by TLA+ model checking and code review of the S3 gateway object put and
key commit paths under HDDS-15926, on commit
b10a7d427bdf83807a7e48a1be507d3b02d5a503. Checked against HDDS issues and
apache/ozone pull requests for duplicates before filing. The attached patch is
a proposal for review. Generated with Specula (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
