[
https://issues.apache.org/jira/browse/HDDS-17629?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18135893#comment-18135893
]
Ritesh Shukla commented on HDDS-17629:
--------------------------------------
Corrected the description after re-checking each claim on master ef63bacef7
with unit tests and a mini-cluster run. Since HDDS-17554, two entries that
convert to the same part number, such as {{1}} and {{4294967297}}, are refused
with {{InvalidPartOrder}} instead of being merged. The table's outcomes and the
HTTP 500 after an unrecognised {{Quiet}} value are now measured rather than
read from code, and the tests under Testing are written and fail on master as
described. The proposed fix now names the exact whitespace set and checks each
part in the loop that builds the part map, after the bucket lookup.
----
🤖 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. The ETag
> matches part 12, so the upload is completed and the object holds {{b}} (both
> measured on master).
> ||{{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 column is measured on master ef63bacef7: the readings with the
> gateway's own unmarshaller in unit tests, and the outcome (completion, or
> OM's answer) on a mini cluster through presigned URLs.
> 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 (measured), 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 refused
> with 400 {{InvalidPartOrder}} before OM is called, by the duplicate check
> HDDS-17554 added (measured). S3 refuses {{4294967297}} itself with
> {{MalformedXML}}.
> * Quiet. The keys are deleted, but the caller gets HTTP 500 with the web
> server's HTML error page instead of the delete result, because no exception
> mapper handles the NullPointerException (measured on a mini cluster). The S3
> Gateway writes no audit entry for the request (measured in a unit test); OM
> still audits the delete (from code). A retry deletes nothing and fails the
> same way (measured).
> * Who sends these forms. Not the AWS Java SDKs or the AWS CLI. The SDKs write
> part numbers with Java's {{toString}} and booleans as lowercase {{true}} or
> {{false}} (bytecode of SDK v1 1.12.788 and SDK v2 2.55.6, the versions Ozone
> tests with). The AWS CLI v2 (2.35.10) writes integers with Python's {{str}}
> and booleans as lowercase {{true}} or {{false}} (source of its bundled
> botocore); boto3 uses the same serializer (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 ef63bacef7.
> * {{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 that a number is valid. {{ObjectEndpoint.java:878-882}} puts
> each number into a map keyed by part number, refuses a number seen twice with
> {{InvalidPartOrder}} (HDDS-17554), 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 around the number, and nothing else: space, tab, CR and
> LF. Do not use {{String.trim()}} or {{String.strip()}}; both drop more (an
> XML 1.1 body can carry a vertical tab, and {{strip()}} also drops U+3000).
> Read the rest as {{Integer.parseInt}} does: 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.
> * 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".
> * An element holding only XML whitespace is read like an empty element (not
> tested against S3).
> * Numbers from 10,001 to 2147483647 stay as today: OM answers
> {{InvalidPart}}, as S3 does.
> Check every part in the loop of {{ObjectEndpoint.completeMultipartUpload}}
> that builds the part map, before OM is called. A number that does not parse
> is {{MalformedXML}}, a number below 1 is {{InvalidArgument}}, and a number
> seen twice stays {{InvalidPartOrder}} (HDDS-17554). That loop runs after the
> bucket lookup. S3 checks the upload before it reads the parts: an unknown
> upload with {{1 2}} gets 404 {{NoSuchUpload}} (tested 2026-10-31). The
> gateway learns that the upload is unknown only from OM's answer, so for an
> unknown upload with a malformed part number it answers {{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 read it with
> {{Boolean.parseBoolean}}, which never fails. The NullPointerException cannot
> happen, and {{BucketEndpoint}} needs no change. 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 tests below are written and fail on master ef63bacef7 for the reasons
> given (run 2026-10-31). A prototype of the fix makes them pass.
> * 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. Two cases S3 was not sent pin the whitespace set: U+3000
> before the number, and a vertical tab before it in an XML 1.1 body, are both
> refused.
> * 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}}: upload part 1 ({{a}}) and part 12
> ({{b}}), then complete through a presigned URL with {{<PartNumber>1
> 2</PartNumber>}} and part 12's ETag, then with
> {{<PartNumber>4294967297</PartNumber>}} and part 1's ETag. On master the
> first returns 200 and the object holds {{b}}. After the fix both return 400
> {{MalformedXML}}, no object is created, and a valid completion with part 1
> still works. A second test sends DeleteObjects with {{<Quiet>True</Quiet>}}
> through a presigned URL: on master it returns 500 after deleting the object;
> after the fix it returns 200 with no {{Deleted}} entries.
> ----
> 🤖 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]
