[
https://issues.apache.org/jira/browse/HDDS-17600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18135885#comment-18135885
]
Ritesh Shukla commented on HDDS-17600:
--------------------------------------
Replaced the summary and description. Testing against AWS S3 on 2026-10-31
showed that S3 accepts some bodies the first fix refused: it reads
{{<Key>tmp/<a/>secret</Key>}} as {{tmp/secret}}. It also refuses
{{<Key><a>dir/</a> </Key>}}, which the first fix let through, as the PR review
pointed out. The fix in apache/ozone#11860 now reads values the way S3 does,
and malformed request bodies answer {{MalformedXML}}.
----
🤖 Generated with [Claude Code|https://claude.com/claude-code]
> S3G: a value in request XML that holds a child element is read as only the
> text after it
> ----------------------------------------------------------------------------------------
>
> Key: HDDS-17600
> URL: https://issues.apache.org/jira/browse/HDDS-17600
> Project: Apache Ozone
> Issue Type: Bug
> Components: S3
> Reporter: Ritesh Shukla
> Assignee: Ritesh Shukla
> Priority: Major
> Labels: pull-request-available
>
> h3. Problem
> S3 request bodies carry values as XML text: the key in a DeleteObjects
> request, a tag's key and value, a part's ETag. The S3 Gateway reads these
> bodies with JAXB, which keeps only the text after an element's last child
> element. So this DeleteObjects body:
> {code:xml}
> <Delete><Object><Key>tmp/<a/>secret</Key></Object></Delete>
> {code}
> is read as the key {{secret}}, and the gateway deletes {{secret}}. AWS S3
> reads the same body as the key {{tmp/secret}}.
> h3. What AWS S3 does
> Each body below was sent to AWS S3 on 2026-10-31.
> ||Body||AWS S3||Ozone master||
> |{{<Key>tmp/<a/>secret</Key>}}|deletes {{tmp/secret}}|deletes {{secret}}|
> |{{<Key>tmp/x<a> </a>y</Key>}}|deletes {{tmp/xy}}|deletes {{y}}|
> |{{<Key><a>dir/</a> </Key>}}|400 {{MalformedXML}}|deletes the key {{" "}}
> (one space)|
> |{{<Value><a>protected</a></Value>}} in a tag|400 {{MalformedXML}}|stores an
> empty value|
> |{{<Object>junk<Key>tmp/a</Key></Object>}}|400 {{MalformedXML}}|deletes
> {{tmp/a}}|
> S3's rule, from those runs:
> * Inside an element read as one value, a child element that holds nothing but
> XML whitespace (space, tab, CR, LF) is ignored, and the text on both sides is
> joined. Comments, CDATA sections and processing instructions are joined the
> same way.
> * A child element holding any other text, at any depth, makes the body
> {{MalformedXML}}. That includes other whitespace characters such as U+3000
> and a no-break space.
> * Text directly inside an element that holds other elements, such as
> {{Object}}, makes the body {{MalformedXML}}. XML whitespace there is
> indentation.
> h3. Impact
> The gateway can act on a value the request does not contain: delete a
> different key, or store a different tag value or none. The caller still needs
> permission for what the gateway actually does, because OM authorizes every
> key it changes (read from code). Clients that build XML with a library escape
> {{<}} in values (checked in the AWS SDK for Java v1, 1.12.788), so these
> bodies come from hand-written XML or from software that builds XML without
> escaping.
> h3. Cause
> {{MessageUnmarshaller}} passes the parser's events through
> {{XmlNamespaceFilter}} into JAXB's {{UnmarshallerHandler}}. Nothing between
> the parser and JAXB handles a child element inside a value, and JAXB keeps
> only the text after the last child.
> h3. Fix (apache/ozone#11860)
> * {{XmlElementModel}} works out, once per request class, which elements JAXB
> reads as one value and which hold other elements, for each parent element,
> from the class's field annotations. It fails on JAXB mappings it does not
> follow, so a new field cannot get past the filter unnoticed.
> * {{XmlNamespaceFilter}} uses it to read values the way S3 does: inside a
> value, a child element holding only XML whitespace is not passed to JAXB, so
> the text around it is read as one value; a child element holding other text
> makes the body malformed. Text directly inside an element that holds other
> elements makes the body malformed.
> * Every failure in the shared XML reader now answers 400 {{MalformedXML}},
> S3's code for a malformed body, instead of 400 {{InvalidRequest}}. This
> changes DeleteObjects, CompleteMultipartUpload and PutBucketAcl; tagging and
> lifecycle already answered {{MalformedXML}}. For PutBucketAcl, AWS documents
> a separate code, {{MalformedACLError}}; which one S3 returns for a malformed
> ACL body was not tested.
> * Not changed on purpose: elements a request class does not define are still
> ignored. S3 refuses them, but the gateway cannot tell S3 fields it does not
> model, such as newer SDKs' checksum elements, from junk, and ignoring them
> never changes the values it reads.
> Affected request bodies: DeleteObjects, CompleteMultipartUpload,
> PutObjectTagging, PutBucketTagging, PutBucketAcl and
> PutBucketLifecycleConfiguration. Lifecycle keeps its own allowlist, which
> already refuses any element inside a value.
> h3. Testing
> * Unit: the parameterized cases in {{TestMultiDeleteRequestUnmarshaller}},
> {{TestCompleteMultipartUploadRequestUnmarshaller}} and
> {{TestObjectTaggingPut}} are the bodies sent to S3, each with S3's answer.
> {{TestBucketAcl}} and {{TestPutBucketLifecycleConfigurationUnmarshaller}}
> cover ACL and lifecycle bodies. {{TestXmlElementModel}} pins the
> value/container tree of all five request classes.
> * Integration:
> {{AbstractS3SDKV1Tests#testPresignedUrlDeleteObjectsReadsKeyAsS3Does}} sends
> {{<Key>P/tmp/<a/>P/secret</Key>}} and {{<Key><a>P/dir/</a> </Key>}} through a
> presigned DeleteObjects URL. On master's code it fails: the request returns
> 200 and {{P/tmp/P/secret}} is still there, because the gateway deleted
> {{P/secret}}. With the fix it passes.
> Related differences found while testing S3, each its own ticket: HDDS-17629
> ({{PartNumber}} and {{Quiet}}), HDDS-17630 (PUT with an empty {{uploadId}}),
> HDDS-17631 (an element given twice), HDDS-17632 (prefixed elements).
> ----
> 🤖 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]
