Ritesh Shukla created HDDS-17631:
------------------------------------
Summary: S3G: request XML that repeats a single-valued element is
accepted and read as its last occurrence
Key: HDDS-17631
URL: https://issues.apache.org/jira/browse/HDDS-17631
Project: Apache Ozone
Issue Type: Bug
Components: S3
Reporter: Ritesh Shukla
Assignee: Ritesh Shukla
h3. Problem
An S3 request body names each setting once. In a multi-object delete each
{{Object}} has one {{Key}}. A tag has one {{Key}} and one {{Value}}. A
multipart {{Part}} has one {{PartNumber}} and one {{ETag}}. An ACL {{Grant}}
has one {{Permission}}, and a lifecycle {{Rule}} has one {{Prefix}}, {{Status}}
and {{Expiration}}. Only lists repeat: {{Object}}, {{Part}}, {{Tag}} inside
{{TagSet}} or {{And}}, {{Grant}}, {{Rule}} and {{Transition}}.
When a body gives a single-valued element twice, the S3 Gateway accepts it and
silently uses the last one. AWS S3 refused every such body it was sent with 400
{{MalformedXML}} and changed nothing (tested against AWS S3 on 2026-10-31).
For example, this DeleteObjects body names two keys for one object:
{code:xml}
<Delete><Object><Key>tmp/a</Key><Key>secret</Key></Object></Delete>
{code}
The gateway deletes {{secret}}. S3 answers 400 {{MalformedXML}} and deletes
nothing. A standard XML reader asked for the object's key returns {{tmp/a}}:
Python's ElementTree {{find("Key")}} and the DOM's
{{getElementsByTagName("Key").item(0)}} both do. So one request means different
things to different readers, and the gateway picks one without saying so.
The same happens in every request that has an XML body:
||Given twice in the body||Gateway on master||AWS S3, 2026-10-31||
|DeleteObjects {{Key}}: {{tmp/a}}, then {{secret}}|deletes {{secret}}|400
{{MalformedXML}}, nothing deleted|
|DeleteObjects {{Quiet}}: {{true}}, then {{false}}|uses {{false}}|400
{{MalformedXML}}, in either order|
|PutObjectTagging {{Value}}: {{a}}, then {{b}}|stores {{b}}|400
{{MalformedXML}}, no tag stored|
|PutObjectTagging {{Key}}: {{k1}}, then {{k2}}|stores {{k2}}|400
{{MalformedXML}}, no tag stored|
|CompleteMultipartUpload {{PartNumber}}: {{12}}, then {{1}}|completes with part
1|400 {{MalformedXML}}|
|CompleteMultipartUpload {{ETag}}: part 12's, then part 1's|uses part 1's|400
{{MalformedXML}}|
|{{<Key>tmp/a</Key><Key xmlns="urn:other">secret</Key>}}|deletes {{secret}}|400
{{MalformedXML}}|
|{{<s3:Key>tmp/a</s3:Key><Key>secret</Key>}}, with {{s3}} bound to the S3
namespace|deletes {{secret}}|400 {{MalformedXML}}|
|{{<Key>tmp/a</Key><x:Key>secret</x:Key>}}, with {{x}} bound to another
namespace|deletes {{tmp/a}}|400 {{MalformedXML}}|
|PutBucketAcl {{Permission}}: {{READ}}, then {{FULL_CONTROL}}|grants
{{FULL_CONTROL}}|not tested|
|PutBucketLifecycleConfiguration {{Prefix}}: {{safe/}}, then
{{secret/}}|expires objects under {{secret/}}|not tested|
|PutBucketLifecycleConfiguration {{Days}}: {{3650}}, then {{1}}|expires objects
after 1 day|not tested|
S3 ignores a change of default namespace and reads an element whose prefix is
bound to the S3 namespace as the plain element, so the first two namespace rows
hold two {{Key}} elements as far as S3 is concerned. An element whose prefix is
bound to another namespace is refused by S3 even on its own (tested), so the
third row is refused either way. ACL and lifecycle bodies were not sent to S3,
to leave the test bucket's settings alone. PutBucketTagging reads the same
model as PutObjectTagging; only PutObjectTagging was sent to S3.
A repeated container behaves the same way on the gateway. A second {{TagSet}},
{{Owner}}, {{AccessControlList}}, {{Grantee}}, {{Expiration}} or {{Filter}}, or
a second {{Tag}} directly under a lifecycle {{Filter}}, silently replaces the
first.
How the gateway column was produced: each body was run on master (6b78ebd246)
through the gateway's own unmarshaller classes, in a standalone harness with
jaxb-runtime 2.3.9 and Hadoop's secure SAX parser factory. That shows which
value the gateway reads. The action that follows (delete, store, grant, install
the rule) is read from the handler code, not run against a live gateway.
h3. Impact
* No common client sends these bodies. The AWS SDK for Java 1.x and the AWS CLI
write each single-valued element once (checked by serializing all six requests
offline). Hand-built XML can repeat them.
* One SDK path does repeat a container. The AWS SDK for Java 1.x accepts
several tag sets in a bucket tagging configuration and writes one {{TagSet}}
element for each. Today the gateway keeps only the last set's tags; with the
fix it refuses the request. S3's answer to that body was not tested.
* Ozone's own permission checks run on the value the gateway acts on, so they
are not bypassed. This is inferred from the code, not run.
* The harm is the silent choice. Anything else that reads the same body and
takes the first value sees a different request from the one Ozone runs. That
includes an audit trail, a policy check, or a person reviewing a lifecycle
rule. They would see another key deleted, {{READ}} where {{FULL_CONTROL}} is
granted, or a 3650-day expiry where Ozone expires data after 1 day.
h3. Cause
{{MessageUnmarshaller.readFrom}} (MessageUnmarshaller.java:74-86) streams the
body through {{XmlNamespaceFilter}} into JAXB. JAXB stores a single-valued
field each time its element appears, so the last occurrence wins, and a
repeated child object replaces the earlier one whole. Nothing counts
occurrences. {{XmlNamespaceFilter}} (XmlNamespaceFilter.java:43-53) only forces
the namespace. The lifecycle filter in
{{PutBucketLifecycleConfigurationUnmarshaller}} (lines 35-47 and 66-75) checks
which children are allowed, not how many times each appears.
The affected request models are {{MultiDeleteRequest}},
{{CompleteMultipartUploadRequest}}, {{S3Tagging}} (object and bucket tagging),
{{S3BucketAcl}} and {{S3LifecycleConfiguration}}.
h3. Proposed fix
Answer as S3 does: refuse a body in which a single-valued element appears more
than once in the same parent, with 400 {{MalformedXML}}, before anything is
changed.
* Take "single-valued" from the request model classes, not from a hand list. A
{{List}} field may repeat; every other bound field may appear at most once. For
every element Ozone reads, this matches S3's published API model.
* Count per parent model class, not by element name. A lifecycle {{Tag}} is
single under {{Filter}} but a list under {{And}}. A rule by name alone would
either refuse the {{And}} bodies that the SDKs send, or let two {{Tag}}
elements under {{Filter}} through.
* Name elements as S3 does when counting: an element in another default
namespace, or with a prefix bound to the S3 namespace, counts as the plain
element (tested 2026-10-31). Prefix handling itself is tracked separately.
* Keep accepting elements that Ozone does not model, as today. Newer SDKs send
some of them, for example {{ChecksumCRC32}} in a {{Part}}.
* Answer {{MalformedXML}} for all six requests. Today a parse failure answers
{{InvalidRequest}} for DeleteObjects, CompleteMultipartUpload and PutBucketAcl.
The new refusal gets its own mapping, so other parse errors keep their current
code.
* S3's answer for repeated elements in PutBucketAcl and
PutBucketLifecycleConfiguration bodies is not tested yet. The fix refuses them
the same way. If a later test shows that S3 reads one of the values instead,
the gateway should read that same value for that request.
Keeping the first occurrence instead is not better. S3 refuses these bodies,
and whichever occurrence the gateway picks, some other reader picks the other
one.
This check and HDDS-17600 (elements inside a value field) can share one walk
over the request model, as long as that walk tracks the model class of each
open element rather than only its name.
h3. Testing
* Unit, in s3gateway: one parameterized test per request type. It covers every
row of the table, plus {{VersionId}}, {{TagSet}}, {{Owner}},
{{AccessControlList}}, {{Grantee}}, {{Status}}, {{ID}}, {{Expiration}},
{{Filter}}, {{Tag}} and {{Prefix}} under {{Filter}}, {{Prefix}} under {{And}},
and {{DaysAfterInitiation}} given twice. Each must answer {{MalformedXML}} and
change nothing. On master each case fails, because the body is accepted with
the last value.
* Unit, bodies that must stay accepted: several {{Object}}, {{Part}}, {{Tag}},
{{Grant}}, {{Rule}} and {{Transition}} elements; an {{And}} with two {{Tag}}
elements; {{Quiet}} between two {{Object}} elements; an unmodelled
{{ChecksumCRC32}} in a {{Part}}.
* Existing tests: a prototype of this check was run standalone over every
request body in the s3gateway unit tests, the S3 SDK integration tests, the
smoketests and the test resources. That is 153 bodies, with the parameterized
lifecycle cases expanded. It changed no result and no error message.
* Integration: in {{AbstractS3SDKV2Tests}}, a presigned CompleteMultipartUpload
whose {{Part}} has {{PartNumber}} twice answers 400 {{MalformedXML}}, and the
upload can still be completed afterwards.
h3. Not covered here
* S3 also refuses an element that its schema does not define. For example,
{{<Object><Wrap><Key>tmp/a</Key></Wrap><Key>secret</Key></Object>}} gets 400
{{MalformedXML}} from S3 (tested 2026-10-31). The gateway ignores {{Wrap}} and
deletes {{secret}}. Matching that needs S3's list of elements for each parent,
which the request models do not hold, so it is a separate change.
* For the same reason, a repeated element that S3 defines but Ozone does not
read, such as {{ETag}} inside a delete {{Object}}, stays accepted and ignored.
----
🤖 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]
