[
https://issues.apache.org/jira/browse/HDDS-17630?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18135628#comment-18135628
]
Ritesh Shukla commented on HDDS-17630:
--------------------------------------
Updated the description after a second test against AWS S3 on 2026-10-31. S3
answers 405 MethodNotAllowed to a PUT with any {{uploadId}} and no
{{partNumber}}, not only an empty one, so the proposed fix now covers both. It
refuses any {{partNumber}} value ({{0}}, {{abc}}, empty) without an upload ID
with the same 400, also while an upload is in progress on the key. {{HEAD
?uploadId=}} and {{GET ?partNumber=1&uploadId=}} get 405 on S3; that difference
is noted and left out of this fix.
----
🤖 Generated with [Claude Code|https://claude.com/claude-code]
> S3G: a part upload with an empty or missing uploadId overwrites the object
> instead of failing
> ---------------------------------------------------------------------------------------------
>
> Key: HDDS-17630
> URL: https://issues.apache.org/jira/browse/HDDS-17630
> Project: Apache Ozone
> Issue Type: Bug
> Components: S3
> Reporter: Ritesh Shukla
> Assignee: Ritesh Shukla
> Priority: Major
>
> h3. Problem
> An S3 {{PUT}} on an object key means one of two things:
> * {{PUT /bucket/key?partNumber=N&uploadId=<id>}} uploads one part of a
> multipart upload (UploadPart). Nothing under the key changes until the upload
> is completed.
> * {{PUT /bucket/key}} writes the object (PutObject).
> When {{uploadId}} is empty, or {{partNumber}} comes with no {{uploadId}} at
> all, the S3 Gateway handles the request as a PutObject, or as a CopyObject
> when {{x-amz-copy-source}} is set. The body (or the copy source) replaces the
> committed object, and the client gets {{200 OK}}, just as for a part upload:
> {code}
> PUT /bucket/key body "original object" -> 200, key
> holds "original object"
> PUT /bucket/key?partNumber=1&uploadId= body "part 1" -> 200, key
> now holds "part 1"
> {code}
> AWS S3 refuses each of these requests and leaves the object alone (tested
> against AWS S3 on 2026-10-31). The Ozone column was run in the S3 Gateway
> unit-test harness on master:
> || Request || AWS S3 || S3 Gateway on master ||
> | {{PUT ?partNumber=1&uploadId=}} | 400 InvalidArgument, object kept | 200,
> object replaced by the body |
> | {{PUT ?partNumber=1&uploadId}} (no {{=}}) | 400 InvalidArgument, object
> kept | same as above: the gateway's JAX-RS layer decodes a bare {{uploadId}}
> to the same empty value (checked with Jersey's query decoder) |
> | {{PUT ?partNumber=1}} (no {{uploadId}}) | 400 InvalidArgument, object kept
> | 200, object replaced by the body |
> | {{PUT ?partNumber=1&uploadId=}} with {{x-amz-copy-source}} (UploadPartCopy)
> | 400 InvalidArgument, object kept | 200, object replaced by a full copy of
> the source; {{x-amz-copy-source-range}} is ignored |
> | {{PUT ?uploadId=}} (no {{partNumber}}) | 405 MethodNotAllowed, object kept
> | 200, object replaced by the body |
> | {{PUT ?partNumber=}} or {{?partNumber=0}} (no {{uploadId}}), or
> {{?partNumber=&uploadId=}} | 400 InvalidArgument, object kept
> ({{?partNumber=abc}} too) | 200, object replaced by the body |
> | {{PUT ?partNumber=1&uploadId=}} while an upload is in progress on the key |
> 400 InvalidArgument, object kept, no part added | 200, object replaced by the
> body |
> | {{PUT ?uploadId=<non-empty ID>}} (no {{partNumber}}), unknown or real
> upload | 405 MethodNotAllowed, object kept, no part added | unknown ID: 404
> NoSuchUpload; real upload: OM refuses part number 0 with 400 (read from code)
> |
> S3's InvalidArgument message is "This operation does not accept partNumber
> without uploadId", for any value of {{partNumber}}. With an empty body, the
> gateway truncates the object to 0 bytes.
> Other handlers that read {{uploadId}}:
> * {{POST ?uploadId=}} (CompleteMultipartUpload) and {{GET ?uploadId=}}
> (ListParts): S3 and the gateway both answer 404 NoSuchUpload. On the gateway
> side this was run against the test client; that OM refuses an empty upload ID
> was read from code.
> * {{DELETE ?uploadId=}}: S3 answers 404 NoSuchUpload; the gateway deletes the
> object. That is HDDS-17599.
> * {{HEAD ?uploadId=}} and {{GET ?partNumber=1&uploadId=}}: S3 answers 405
> MethodNotAllowed; the gateway answers 200 (HeadObject) and 404 NoSuchUpload.
> Both are read-only and are left out of this fix.
> h3. Impact
> * Only a caller who may already write the key can do this. The operation that
> actually runs is authorized as itself: PutObject, or GetObject on the source
> and PutObject on the destination for the copy form. No permission boundary is
> crossed (read from code).
> * What is lost: the object's data (run), and with it the object's metadata
> and tags (read from code). The response is a success, so the client cannot
> tell. A client that uploads several parts under the same empty ID ends up
> with only the last part as the object (inferred).
> * How a client gets here: an application bug that passes an empty upload ID,
> for example after losing the CreateMultipartUpload response. Stock clients
> send it as-is. The AWS SDK for Java v2 (2.55.6) and v1 (1.12.788), and the
> AWS CLI v2 (2.35.10), all put {{uploadId=}} on the wire for an empty ID,
> including for UploadPartCopy; both SDKs refuse a null ID before sending (run
> against a local capture server).
> h3. Cause
> {{ObjectEndpoint#handlePutRequest}} treats a request as a part upload only
> when {{uploadID != null && !uploadID.equals("")}} (ObjectEndpoint.java:214 on
> master). Every other request becomes a PutObject, or a CopyObject when
> {{x-amz-copy-source}} is set (ObjectEndpoint.java:236-243). That path never
> reads {{partNumber}}.
> The empty-string test dates from the first UploadPart code (HDDS-902). The
> parameter was then declared with {{@DefaultValue("")}}, so a missing
> {{uploadId}} arrived as an empty string, and comparing with {{""}} was how
> "absent" was written. HDDS-14209 switched to reading query parameters
> directly, where a missing {{uploadId}} is {{null}}, but kept the comparison.
> So a present but empty {{uploadId}} still counts as absent.
> h3. Proposed fix
> Answer as S3 does (tested against AWS S3 on 2026-10-31), before any OM call:
> * {{partNumber}} present and {{uploadId}} missing or empty: 400
> {{InvalidArgument}} with S3's message, "This operation does not accept
> partNumber without uploadId". This also covers UploadPartCopy.
> * {{uploadId}} present, empty or not, and no {{partNumber}}: 405
> {{MethodNotAllowed}}, as S3 answers both. {{S3ErrorTable}} gains a
> {{METHOD_NOT_ALLOWED}} entry with S3's code and message.
> * Everything else is unchanged: a non-empty {{uploadId}} with a
> {{partNumber}} is still a part upload, and a PUT with neither parameter is
> still a PutObject.
> Both checks go at the top of {{handlePutRequest}}. A refused request is
> audited and counted as a failed PutObject, and the key is never opened. The
> part-upload branch then only needs {{uploadID != null}}.
> The obvious one-line fix, dropping {{!uploadID.equals("")}} so the empty ID
> reaches OM, is worse. OM would answer 404 NoSuchUpload, not S3's 400. {{PUT
> ?partNumber=1}} with no {{uploadId}} would still overwrite the object. And
> UploadPartCopy would read the source key before failing. HDDS-17599 answers
> DELETE with NoSuchUpload because that is what S3 does for DELETE; S3 answers
> PUT differently, so this fix does too.
> h3. Testing
> * Unit, {{TestPartUpload}}: one parameterized case for each of
> {{?partNumber=1&uploadId=}}, {{?partNumber=1}} and {{?uploadId=}}. Each puts
> a key, sends the PUT, checks S3's error code and message, and checks that the
> key's content and size are unchanged. It runs with and without data stream.
> On master all six runs fail: the PUT succeeds and the key is replaced. To add
> with the fix: {{?partNumber=}}, {{?partNumber=0}} and {{?partNumber=abc}}
> without an upload ID (400), and {{?uploadId=<non-empty>}} without a part
> number (405).
> * Unit, {{TestMultipartUploadWithCopy}}: the same for UploadPartCopy with
> {{?partNumber=1&uploadId=}}. On master the destination is replaced by the
> source.
> * {{EndpointTestUtils#put}} stops adding {{partNumber=0}} to plain PUTs. A
> real PutObject carries no {{partNumber}}, and the new check would refuse it.
> * All s3gateway unit tests pass with the change.
> * Integration (to add with the fix): a case in {{AbstractS3SDKV2Tests}} calls
> {{uploadPart}} and {{uploadPartCopy}} with {{uploadId("")}}, which the SDK
> sends as {{uploadId=}}. It expects 400 InvalidArgument and the same ETag from
> {{headObject}} afterwards.
> ----
> 🤖 Generated with [Claude Code|https://claude.com/claude-code]
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
