[
https://issues.apache.org/jira/browse/HDDS-17629?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18135627#comment-18135627
]
Ritesh Shukla commented on HDDS-17629:
--------------------------------------
Updated the description after a second test against AWS S3 on 2026-10-31. S3
never refuses a {{Quiet}} value: {{true}} in any letter case means quiet, and
any other text means not quiet, so the proposed fix now reads {{Quiet}} that
way instead of refusing unusual values. For {{PartNumber}}, S3 reads an empty
element as 0 ({{InvalidArgument}}), accepts Unicode decimal digits, and refuses
{{2147483648}} and a {{Part}} with no {{PartNumber}} with {{MalformedXML}}; the
table and the proposed fix now follow those answers.
----
🤖 Generated with [Claude Code|https://claude.com/claude-code]
> S3G: PartNumber and Quiet in request XML are not read as S3 reads them;
> PartNumber 4294967297 is read as 1
> ----------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17629
> URL: https://issues.apache.org/jira/browse/HDDS-17629
> Project: Apache Ozone
> Issue Type: Bug
> Components: S3
> Reporter: Ritesh Shukla
> Assignee: Ritesh Shukla
> Priority: Minor
>
> h3. Problem
> CompleteMultipartUpload joins uploaded parts into one object. The request
> names each part by {{PartNumber}} and {{ETag}}. S3 reads {{PartNumber}}
> strictly. Tested against AWS S3 on 2026-10-31, it refuses {{1 2}}, {{1+2}}
> and {{4294967297}} with 400 {{MalformedXML}}, and {{-1}} with 400
> {{InvalidArgument}} ("PartNumber must be >= 1"). The S3 Gateway reads the
> same texts as 12, 12, 1 and -1, and acts on those numbers.
> Example. An upload has part 1 holding {{a}} and part 12 holding {{b}}. The
> client sends part 12's ETag with a part number of {{1 2}}:
> {code:xml}
> <CompleteMultipartUpload xmlns="http://s3.amazonaws.com/doc/2006-03-01/";>
> <Part><PartNumber>1
> 2</PartNumber><ETag>"92eb5ffee6ae2fec3ad71c777531578f"</ETag></Part>
> </CompleteMultipartUpload>
> {code}
> S3 refuses it with 400 {{MalformedXML}}. The gateway reads part 12
> (measured). The ETag matches part 12, so the upload is completed and the
> object holds {{b}} (from code).
> ||{{PartNumber}} text||AWS S3 (tested 2026-10-31)||Ozone today||
> |{{1}}|200, completed with part 1|same|
> |{{01}}, {{+1}}, {{1}} with spaces around it|200, read as part 1|same|
> |{{1 2}}, with part 12's ETag|400 {{MalformedXML}}|reads 12, completes with
> part 12|
> |{{1+2}}, with part 12's ETag|400 {{MalformedXML}}|reads 12, completes with
> part 12|
> |{{4294967297}}, with part 1's ETag|400 {{MalformedXML}}|reads 1, completes
> with part 1|
> |{{-1}}|400 {{InvalidArgument}}, "PartNumber must be >= 1"|reads -1, OM
> answers {{InvalidPart}}|
> |{{0}}|400 {{InvalidArgument}}, "PartNumber must be >= 1"|reads 0, OM answers
> {{InvalidPart}}|
> |{{1+}}, {{1-}}|400 {{MalformedXML}}|reads 1 (completes) and -1
> ({{InvalidPart}})|
> |{{abc}}, {{1.0}}, a no-break space before {{1}}|400 {{MalformedXML}}|reads
> 0, OM answers {{InvalidPart}}|
> |{{2147483648}}|400 {{MalformedXML}}|reads -2147483648, OM answers
> {{InvalidPart}}|
> |{{10001}}, {{2147483647}}|400 {{InvalidPart}} (no such part)|same|
> |an empty element, {{<PartNumber/>}}|400 {{InvalidArgument}}, "PartNumber
> must be >= 1"|reads 0, OM answers {{InvalidPart}}|
> |no {{PartNumber}} element|400 {{MalformedXML}}|reads 0, OM answers
> {{InvalidPart}}|
> |tab and newline around {{1}}|200, read as part 1|same|
> |Arabic-Indic digit one (U+0661)|200, read as part 1|reads 0, OM answers
> {{InvalidPart}}|
> The Ozone readings are measured with master's request classes and namespace
> filter, Hadoop's secure SAX parser and jaxb-runtime 2.3.9, following the
> steps of {{MessageUnmarshaller.readFrom}}. What Ozone does after the reading
> (completing, or OM's answer) follows from the code and was not run.
> DeleteObjects' {{Quiet}} flag goes through the same conversion, with a worse
> result. S3 never refuses a {{Quiet}} value (tested 2026-10-31): {{true}} in
> any letter case means quiet, and any other text means not quiet, including
> {{1}}, {{true}} with spaces around it, {{False}}, {{yes}} and an empty
> element. The gateway reads {{true}}, {{1}} and {{true}} with spaces as quiet,
> and {{false}}, {{0}} and {{2}} as not quiet. Other text, such as {{True}},
> {{TRUE}}, {{False}}, {{yes}}, an empty element or {{<Quiet/>}}, is read as
> nothing (null), which was measured. The gateway reads {{Quiet}} only after OM
> has deleted the keys. So such a request deletes them and then fails with a
> NullPointerException, where S3 answers 200.
> h3. Impact
> * Part numbers. OM uses a part only if the request's ETag matches that part's
> stored ETag ({{S3MultipartUploadCompleteRequest.java:677-692}}, from code).
> So a misread number can only pick a part whose ETag the request names. The
> result is an object that a valid request with the same ETags could also
> build, and the same permissions are needed. What goes wrong: Ozone completes
> uploads that S3 refuses. {{-1}} gets {{InvalidPart}} instead of S3's
> {{InvalidArgument}}. Text such as {{abc}} is read as part 0, so the request
> fails with {{InvalidPart}} because of a part it never named. Two entries that
> convert to the same number, such as {{1}} and {{4294967297}}, are merged into
> one part before OM sees them (from code). HDDS-17554 tracks literal duplicate
> part numbers.
> * Quiet. The keys are deleted, but the caller gets an error instead of the
> delete result. From code, not run: the error is an HTTP 500 because no
> exception mapper handles it, the S3 Gateway audit log has no entry for the
> request (OM still audits the delete), and a retry fails the same way.
> * Who sends these forms. Not the AWS Java SDKs. Both write part numbers with
> Java's {{toString}} and booleans as lowercase {{true}} or {{false}}, checked
> in the bytecode of SDK v1 1.12.788 and SDK v2 2.55.11. The AWS CLI and boto3
> were not checked. Hand-written XML can: a script that prints a boolean the
> way Python or .NET do writes {{True}}.
> * Not affected: lifecycle. Its {{Days}} and {{DaysAfterInitiation}} are
> already read as text and parsed strictly since HDDS-16393. {{1 2}}, {{1+2}},
> {{7+}} and overflow are refused there (measured). Every other request field
> is text.
> h3. Cause
> Gateway files are in
> {{hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint}};
> {{S3MultipartUploadCompleteRequest.java}} is in
> {{hadoop-ozone/ozone-manager}}. Line numbers are for master at 6b78ebd246.
> * {{CompleteMultipartUploadRequest.java:55-56}} binds {{PartNumber}} to an
> {{int}}. {{MultiDeleteRequest.java:36-37}} binds {{Quiet}} to a {{Boolean}}.
> JAXB fills them with its built-in converters ({{DatatypeConverterImpl}} in
> jaxb-runtime 2.3.9). The integer converter skips spaces and plus signs
> anywhere in the text, takes a minus sign anywhere as the sign, and wraps
> around on overflow. The boolean converter returns null for text it does not
> recognise.
> * {{MessageUnmarshaller.java:76-77}} keeps JAXB's default error handling,
> which ignores conversion errors. Text such as {{abc}} leaves {{PartNumber}}
> at its default, 0.
> * Nothing checks the values afterwards. {{ObjectEndpoint.java:872-874}} puts
> each number into a map keyed by part number and sends the map to OM.
> {{BucketEndpoint.java:414}} unboxes {{Quiet}} after the delete call at line
> 407.
> h3. Proposed fix
> Answer as S3 does. Bind both values as text and parse them strictly before
> anything is acted on. Lifecycle already does this for {{Days}} since
> HDDS-16393 ({{S3LifecycleConfiguration.java:154-157}} and {{243-253}}). The
> public getters and setters keep their types.
> {{PartNumber}}, following S3's answers (tested 2026-10-31):
> * Drop XML whitespace (space, tab, CR, LF) around the number and read the
> rest as a 32-bit decimal integer: one optional sign, then digits. S3 reads
> {{01}}, {{+1}} and the Arabic-Indic digit one as part 1, so leading zeros and
> Unicode decimal digits are accepted, as Java's {{Integer.parseInt}} accepts
> them.
> * Refuse anything else with 400 {{MalformedXML}}: whitespace or a sign inside
> the number, a decimal point, letters, a no-break space, a number too large
> for 32 bits ({{2147483648}}, {{4294967297}}), and a {{Part}} with no
> {{PartNumber}} element.
> * Read an empty {{PartNumber}} element as 0, as S3 does, and refuse any
> number below 1 with 400 {{InvalidArgument}} and S3's message, "PartNumber
> must be >= 1".
> * Numbers from 10,001 to 2147483647 stay as today: OM answers
> {{InvalidPart}}, as S3 does.
> Check every part in {{ObjectEndpoint.completeMultipartUpload}}, next to the
> existing empty-part-list check, before OM is called. S3 checks that the
> upload exists before it reads the parts, so for an unknown upload with a
> malformed part number S3 answers {{NoSuchUpload}} where the gateway will
> answer {{MalformedXML}}. That order difference is left as is.
> {{Quiet}}: read it as S3 does (tested 2026-10-31). The request is quiet if
> the text is {{true}} in any letter case, and not quiet for any other text; a
> {{Quiet}} value is never refused. Bind it as text and work out the flag
> before any key is deleted, so the NullPointerException cannot happen. This
> changes {{1}} and {{true}} with spaces around it from quiet to not quiet, as
> on S3.
> Why not the obvious alternatives (each measured):
> * Make JAXB fail on conversion errors. The converters raise no error for {{1
> 2}} or {{4294967297}}; they return 12 and 1. A strict JAXB handler would also
> refuse every element Ozone does not model, such as {{ChecksumCRC32}}, which
> S3's API defines for a {{Part}}.
> * A JAXB type adapter that throws. JAXB's default handler swallows the
> exception and leaves the field null.
> * A range check on the converted number. By then {{1 2}} and {{4294967297}}
> are already 12 and 1.
> h3. Testing
> The before-and-after results below are what the code predicts. The tests have
> not been written or run yet.
> * Unit, in {{TestCompleteMultipartUploadRequestUnmarshaller}} and
> {{TestMultipartUploadComplete}}: each form S3 was tested with gets S3's
> answer, and a refused request is not passed on to OM. Before the fix, {{1 2}}
> with part 12's ETag completes with part 12, and {{4294967297}} with part 1's
> ETag completes with part 1. {{01}}, {{+1}} and {{1}} with spaces around it
> still complete.
> * Unit, in {{TestMultiDeleteRequestUnmarshaller}} and
> {{TestObjectMultiDelete}}: each {{Quiet}} value S3 was tested with gets S3's
> answer. {{<Quiet>True</Quiet>}} deletes the key and answers in quiet mode;
> {{<Quiet>1</Quiet>}} and {{<Quiet>yes</Quiet>}} delete it and answer in
> verbose mode. Before the fix, {{True}} and {{yes}} delete the key and then
> throw a NullPointerException, and {{1}} answers in quiet mode. A request
> without {{Quiet}} still answers in verbose mode.
> * Unit: a check over the five request classes that fails if any field is
> bound to something other than text, so a future numeric or boolean field
> cannot bring this back.
> * Integration, in {{AbstractS3SDKV1Tests}}: complete an upload through a
> presigned URL with {{<PartNumber>4294967297</PartNumber>}}. The suite already
> sends a hand-written completion body this way. On master this returns 200.
> After the fix it returns 400 {{MalformedXML}} and no object is created.
> ----
> 🤖 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]
