chihsuan commented on code in PR #11860:
URL: https://github.com/apache/ozone/pull/11860#discussion_r4430448356
##########
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:
Would it make sense to add `tmp/<a/>` here? It's rejected only by the
element-after-text check, which the current cases don't isolate. Small nit too,
the two rejection tests could share one `@ValueSource` if you like.
##########
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:
Is it intended that `<Key><a>dir/</a> </Key>` still passes and is read as a
single space key?
##########
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:
Similar to #11859, could this move into `AbstractS3SDKV1Tests` as a
presigned URL test, like [this
one](https://github.com/apache/ozone/blob/28c85143e511e99a175ae414cd255f2e8e5082bd/hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/awssdk/v1/AbstractS3SDKV1Tests.java#L2653-L2681)?
That would drop the Python helper, and the AWS compat run wouldn't pick it up
either.
--
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]
