kerneltime commented on code in PR #11860:
URL: https://github.com/apache/ozone/pull/11860#discussion_r4432701350
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestMultiDeleteRequestUnmarshaller.java:
##########
@@ -66,6 +71,39 @@ public void fromStreamWithoutNamespace() throws IOException {
assertEquals(3, multiDeleteRequest.getObjects().size());
}
+ @Test
+ public void keyWithEscapedMarkupIsOneKey() throws IOException {
+ MultiDeleteRequest request =
unmarshall(body("<Delete><Object><Key>tmp/<x></Key></Object></Delete>"));
+ assertEquals("tmp/<x>", request.getObjects().get(0).getKey());
+ }
+
+ @Test
+ public void indentedBodyIsAccepted() throws IOException {
+ MultiDeleteRequest request = unmarshall(body("<Delete>\n <Object>\n
<Key>tmp/a</Key>\n </Object>\n</Delete>"));
+ assertEquals("tmp/a", request.getObjects().get(0).getKey());
+ }
+
+ @Test
+ public void elementAfterTextInKeyIsRejected() {
Review Comment:
Thanks, this question led to a bigger change. I sent `tmp/<a/>` and the
other shapes to AWS S3: it reads `<Key>tmp/<a/></Key>` as `tmp/` and
`<Key>tmp/<a/>secret</Key>` as `tmp/secret`, so refusing them was wrong. In
260b4a6565 the cases are parameterized lists of the bodies S3 accepted, with
the key it read, and the bodies it refused, each named after its case in that
S3 run; `tmp/<a/>` is D04.
##########
hadoop-ozone/dist/src/main/smoketest/s3/objectmultidelete.robot:
##########
@@ -47,3 +48,18 @@ Delete file with multi delete
Should contain ${result}
${PREFIX}/multidelete/key=value/f3
Should contain ${result} STANDARD
Should not contain ${result}
REDUCED_REDUNDANCY
+
+Multi delete rejects a key with mixed content
Review Comment:
Done in 260b4a6565:
`AbstractS3SDKV1Tests#testPresignedUrlDeleteObjectsReadsKeyAsS3Does` sends both
shapes through a presigned DeleteObjects URL, and the robot case and the Python
helper are gone. With master's filter code it fails (the request returns 200
and deletes `P/secret` instead of `P/tmp/P/secret`); with this change it passes.
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/XmlNamespaceFilter.java:
##########
@@ -43,12 +52,42 @@ public XmlNamespaceFilter(String namespace) {
@Override
public void startElement(String uri, String localName, String qName,
Attributes atts) throws SAXException {
+ boolean[] parent = content.peek();
+ if (parent != null) {
+ if (parent[0]) {
+ throw new SAXException("Mixed content is not allowed: element " +
qName + " follows text");
+ }
+ parent[1] = true;
+ }
+ content.push(new boolean[2]);
super.startElement(namespace, localName, qName, atts);
}
@Override
public void endElement(String uri, String localName, String qName)
throws SAXException {
+ content.pop();
super.endElement(namespace, localName, qName);
}
+
+ @Override
+ public void characters(char[] ch, int start, int length) throws SAXException
{
+ boolean[] current = content.peek();
+ if (current != null && !isWhitespace(ch, start, length)) {
Review Comment:
No, that was a gap, thanks for catching it. S3 refuses that body with
MalformedXML, because the element inside the key holds text. 260b4a6565 follows
S3's rule: inside a value, a child element holding anything but XML whitespace
makes the body malformed, wherever the text sits, and a text-free child element
is ignored, so the text around it is read as one value. `<Key><a>dir/</a>
</Key>` is case D05 in the tests and now gets 400 MalformedXML.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
