[
https://issues.apache.org/jira/browse/HDDS-17600?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ritesh Shukla updated HDDS-17600:
---------------------------------
Description:
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]
was:
h3. Problem
The S3 Gateway unmarshals request bodies (DeleteObjects,
CompleteMultipartUpload, PutBucketLifecycleConfiguration) with JAXB. When an
element that should hold only text also holds a child element, JAXB keeps only
the text after the last child and drops the rest. The gateway then acts on a
value the request does not literally contain.
For a multi-object delete:
{code:xml}
<Delete><Object><Key>dir/<a/>file</Key></Object></Delete>
{code}
is read as key {{file}}, not {{dir/file}}, and the gateway deletes {{file}}.
Likewise {{<Key><a>dir/</a>file</Key>}} is read as {{file}}.
No S3 request body has mixed content, so these bodies are malformed and should
be rejected rather than silently reinterpreted.
h3. Cause
{{MessageUnmarshaller}} feeds the SAX stream through {{XmlNamespaceFilter}}
into JAXB's {{UnmarshallerHandler}}, and nothing rejects an element holding
both text and child elements. (HDDS-14857 already made the parser reject
DOCTYPE declarations; this is a separate gap.)
h3. Proposed fix
In {{XmlNamespaceFilter}}, which every request unmarshaller uses, track for
each open element whether it has seen non-whitespace text and whether it has
seen a child element, and reject the request if it has both. Whitespace between
elements stays allowed. A rejected body takes the existing parse-error path
({{InvalidRequest}}).
h3. Testing
* Unit: {{TestMultiDeleteRequestUnmarshaller}} checks that both mixed-content
shapes above are rejected (they fail before the fix: the body is accepted),
that a key containing an escaped {{<}} is still read as one key, and that an
indented body still parses. All s3gateway unit tests pass with the change.
* Acceptance: a new case in {{s3/objectmultidelete.robot}} sends
{{<Key>dir/<a/>PREFIX-mixed</Key>}} through a presigned DeleteObjects URL
({{presigned_url_helper.py}} gains the helper; the AWS CLI escapes {{<}} in
keys). On the {{ozone}} compose cluster, on master it returns {{200 OK}} with
{{<Deleted><Key>PREFIX-mixed</Key></Deleted>}}, deleting a key the request did
not name; with the fix it is rejected and the object is kept.
h3. Note
Multi-delete parse errors map to {{InvalidRequest}}, while the
CompleteMultipartUpload unmarshaller maps them to {{MalformedXML}}, which is
AWS's code for a malformed body. Aligning that is left out of this change.
----
🤖 Generated with [Claude Code|https://claude.com/claude-code]
Summary: S3G: a value in request XML that holds a child element is read
as only the text after it (was: S3G: request XML with mixed content is
accepted and read as only its trailing text)
> 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]
