From e5acbd67a5f11a6d1c909bcf1b61ef4a5357e0a2 Mon Sep 17 00:00:00 2001 From: guohao1 Date: Tue, 30 Apr 2024 14:58:45 +0800 Subject: [PATCH 1/3] HDDS-10777. S3 gateway error when parsing xml in concurrent execution --- ...eteMultipartUploadRequestUnmarshaller.java | 16 ++----- .../MultiDeleteRequestUnmarshaller.java | 16 ++----- ...eteMultipartUploadRequestUnmarshaller.java | 44 +++++++++++++++++++ 3 files changed, 52 insertions(+), 24 deletions(-) diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/CompleteMultipartUploadRequestUnmarshaller.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/CompleteMultipartUploadRequestUnmarshaller.java index 17f7f575a6ca..f18738312341 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/CompleteMultipartUploadRequestUnmarshaller.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/CompleteMultipartUploadRequestUnmarshaller.java @@ -43,19 +43,7 @@ public class CompleteMultipartUploadRequestUnmarshaller implements MessageBodyReader { - private final JAXBContext context; - private final XMLReader xmlReader; - public CompleteMultipartUploadRequestUnmarshaller() { - try { - context = JAXBContext.newInstance(CompleteMultipartUploadRequest.class); - SAXParserFactory saxParserFactory = SAXParserFactory.newInstance(); - saxParserFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - xmlReader = saxParserFactory.newSAXParser().getXMLReader(); - } catch (Exception ex) { - throw new AssertionError("Can not instantiate " + - "CompleteMultipartUploadRequest parser", ex); - } } @Override public boolean isReadable(Class aClass, Type type, @@ -70,6 +58,10 @@ public CompleteMultipartUploadRequest readFrom( MultivaluedMap multivaluedMap, InputStream inputStream) throws IOException, WebApplicationException { try { + JAXBContext context = JAXBContext.newInstance(CompleteMultipartUploadRequest.class); + SAXParserFactory saxParserFactory = SAXParserFactory.newInstance(); + saxParserFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + XMLReader xmlReader = saxParserFactory.newSAXParser().getXMLReader(); UnmarshallerHandler unmarshallerHandler = context.createUnmarshaller().getUnmarshallerHandler(); XmlNamespaceFilter filter = diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/MultiDeleteRequestUnmarshaller.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/MultiDeleteRequestUnmarshaller.java index f5745a8fc102..38c4bdb82566 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/MultiDeleteRequestUnmarshaller.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/MultiDeleteRequestUnmarshaller.java @@ -42,19 +42,7 @@ public class MultiDeleteRequestUnmarshaller implements MessageBodyReader { - private final JAXBContext context; - private final XMLReader xmlReader; - public MultiDeleteRequestUnmarshaller() { - try { - context = JAXBContext.newInstance(MultiDeleteRequest.class); - SAXParserFactory saxParserFactory = SAXParserFactory.newInstance(); - saxParserFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - xmlReader = saxParserFactory.newSAXParser().getXMLReader(); - } catch (Exception ex) { - throw new AssertionError("Can't instantiate MultiDeleteRequest parser", - ex); - } } @Override @@ -68,6 +56,10 @@ public MultiDeleteRequest readFrom(Class type, Type genericType, Annotation[] annotations, MediaType mediaType, MultivaluedMap httpHeaders, InputStream entityStream) { try { + JAXBContext context = JAXBContext.newInstance(MultiDeleteRequest.class); + SAXParserFactory saxParserFactory = SAXParserFactory.newInstance(); + saxParserFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + XMLReader xmlReader = saxParserFactory.newSAXParser().getXMLReader(); UnmarshallerHandler unmarshallerHandler = context.createUnmarshaller().getUnmarshallerHandler(); diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestCompleteMultipartUploadRequestUnmarshaller.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestCompleteMultipartUploadRequestUnmarshaller.java index cd0fbfed4e65..7c0e14db0086 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestCompleteMultipartUploadRequestUnmarshaller.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestCompleteMultipartUploadRequestUnmarshaller.java @@ -20,12 +20,16 @@ import java.io.ByteArrayInputStream; import java.io.IOException; +import java.util.ArrayList; import java.util.List; import java.util.UUID; +import java.util.concurrent.CompletableFuture; + import org.junit.jupiter.api.Test; import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; /** * Class tests Unmarshall logic of {@link CompleteMultipartUploadRequest}. @@ -75,6 +79,7 @@ public void fromStreamWithoutNamespace() throws IOException { } private void checkContent(CompleteMultipartUploadRequest request) { + assertNotEquals(request, null); assertEquals(2, request.getPartList().size()); List parts = @@ -89,4 +94,43 @@ private CompleteMultipartUploadRequest unmarshall( return new CompleteMultipartUploadRequestUnmarshaller() .readFrom(null, null, null, null, null, inputBody); } + + @Test + public void concurrentParse() { + CompleteMultipartUploadRequestUnmarshaller unmarshaller = + new CompleteMultipartUploadRequestUnmarshaller(); + byte[] bytes = ("" + "" + part1 + + "1" + + part2 + "2" + + "").getBytes( + UTF_8); + + List> futures = + new ArrayList<>(); + for (int i = 0; i < 20; i++) { + futures.add(CompletableFuture.supplyAsync(() -> { + try { + //GIVEN + ByteArrayInputStream inputBody = new ByteArrayInputStream(bytes); + //WHEN + return unmarshall(unmarshaller, inputBody); + } catch (IOException e) { + return null; + } + })); + } + + for (CompletableFuture future : futures) { + CompleteMultipartUploadRequest request = future.join(); + //THEN + checkContent(request); + } + } + + private CompleteMultipartUploadRequest unmarshall( + CompleteMultipartUploadRequestUnmarshaller unmarshaller, + ByteArrayInputStream inputBody) throws IOException { + return unmarshaller + .readFrom(null, null, null, null, null, inputBody); + } } From 0d539802943df22fb96372924eb21a51ae386ff0 Mon Sep 17 00:00:00 2001 From: guohao1 Date: Tue, 30 Apr 2024 18:20:13 +0800 Subject: [PATCH 2/3] test --- .../TestCompleteMultipartUploadRequestUnmarshaller.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestCompleteMultipartUploadRequestUnmarshaller.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestCompleteMultipartUploadRequestUnmarshaller.java index 7c0e14db0086..deeb5f4077ec 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestCompleteMultipartUploadRequestUnmarshaller.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestCompleteMultipartUploadRequestUnmarshaller.java @@ -29,7 +29,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; /** * Class tests Unmarshall logic of {@link CompleteMultipartUploadRequest}. @@ -79,7 +79,7 @@ public void fromStreamWithoutNamespace() throws IOException { } private void checkContent(CompleteMultipartUploadRequest request) { - assertNotEquals(request, null); + assertNotNull(request); assertEquals(2, request.getPartList().size()); List parts = From d4041bbb0c0b8074aa6c51f68d8b7a63f30dd57d Mon Sep 17 00:00:00 2001 From: guohao1 Date: Wed, 8 May 2024 15:00:38 +0800 Subject: [PATCH 3/3] HDDS-10777. S3 gateway error when parsing xml in concurrent execution --- ...CompleteMultipartUploadRequestUnmarshaller.java | 14 +++++++++++--- .../endpoint/MultiDeleteRequestUnmarshaller.java | 14 +++++++++++--- .../endpoint/PutBucketAclRequestUnmarshaller.java | 6 +++--- ...CompleteMultipartUploadRequestUnmarshaller.java | 2 +- 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/CompleteMultipartUploadRequestUnmarshaller.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/CompleteMultipartUploadRequestUnmarshaller.java index f18738312341..cdaaa228ecd7 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/CompleteMultipartUploadRequestUnmarshaller.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/CompleteMultipartUploadRequestUnmarshaller.java @@ -43,7 +43,18 @@ public class CompleteMultipartUploadRequestUnmarshaller implements MessageBodyReader { + private final JAXBContext context; + private final SAXParserFactory saxParserFactory; + public CompleteMultipartUploadRequestUnmarshaller() { + try { + context = JAXBContext.newInstance(CompleteMultipartUploadRequest.class); + saxParserFactory = SAXParserFactory.newInstance(); + saxParserFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + } catch (Exception ex) { + throw new AssertionError("Can not instantiate " + + "CompleteMultipartUploadRequest parser", ex); + } } @Override public boolean isReadable(Class aClass, Type type, @@ -58,9 +69,6 @@ public CompleteMultipartUploadRequest readFrom( MultivaluedMap multivaluedMap, InputStream inputStream) throws IOException, WebApplicationException { try { - JAXBContext context = JAXBContext.newInstance(CompleteMultipartUploadRequest.class); - SAXParserFactory saxParserFactory = SAXParserFactory.newInstance(); - saxParserFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); XMLReader xmlReader = saxParserFactory.newSAXParser().getXMLReader(); UnmarshallerHandler unmarshallerHandler = context.createUnmarshaller().getUnmarshallerHandler(); diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/MultiDeleteRequestUnmarshaller.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/MultiDeleteRequestUnmarshaller.java index 38c4bdb82566..0c34c08091aa 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/MultiDeleteRequestUnmarshaller.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/MultiDeleteRequestUnmarshaller.java @@ -42,7 +42,18 @@ public class MultiDeleteRequestUnmarshaller implements MessageBodyReader { + private final JAXBContext context; + private final SAXParserFactory saxParserFactory; + public MultiDeleteRequestUnmarshaller() { + try { + context = JAXBContext.newInstance(MultiDeleteRequest.class); + saxParserFactory = SAXParserFactory.newInstance(); + saxParserFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + } catch (Exception ex) { + throw new AssertionError("Can't instantiate MultiDeleteRequest parser", + ex); + } } @Override @@ -56,9 +67,6 @@ public MultiDeleteRequest readFrom(Class type, Type genericType, Annotation[] annotations, MediaType mediaType, MultivaluedMap httpHeaders, InputStream entityStream) { try { - JAXBContext context = JAXBContext.newInstance(MultiDeleteRequest.class); - SAXParserFactory saxParserFactory = SAXParserFactory.newInstance(); - saxParserFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); XMLReader xmlReader = saxParserFactory.newSAXParser().getXMLReader(); UnmarshallerHandler unmarshallerHandler = context.createUnmarshaller().getUnmarshallerHandler(); diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/PutBucketAclRequestUnmarshaller.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/PutBucketAclRequestUnmarshaller.java index 3ca2e47c469e..3fa6149815ea 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/PutBucketAclRequestUnmarshaller.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/PutBucketAclRequestUnmarshaller.java @@ -44,14 +44,13 @@ public class PutBucketAclRequestUnmarshaller implements MessageBodyReader { private final JAXBContext context; - private final XMLReader xmlReader; + private final SAXParserFactory saxParserFactory; public PutBucketAclRequestUnmarshaller() { try { context = JAXBContext.newInstance(S3BucketAcl.class); - SAXParserFactory saxParserFactory = SAXParserFactory.newInstance(); + saxParserFactory = SAXParserFactory.newInstance(); saxParserFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - xmlReader = saxParserFactory.newSAXParser().getXMLReader(); } catch (Exception ex) { throw new AssertionError("Can not instantiate " + "PutBucketAclRequest parser", ex); @@ -70,6 +69,7 @@ public S3BucketAcl readFrom( MultivaluedMap multivaluedMap, InputStream inputStream) throws IOException, WebApplicationException { try { + XMLReader xmlReader = saxParserFactory.newSAXParser().getXMLReader(); UnmarshallerHandler unmarshallerHandler = context.createUnmarshaller().getUnmarshallerHandler(); XmlNamespaceFilter filter = diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestCompleteMultipartUploadRequestUnmarshaller.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestCompleteMultipartUploadRequestUnmarshaller.java index deeb5f4077ec..1872c440da31 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestCompleteMultipartUploadRequestUnmarshaller.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestCompleteMultipartUploadRequestUnmarshaller.java @@ -107,7 +107,7 @@ public void concurrentParse() { List> futures = new ArrayList<>(); - for (int i = 0; i < 20; i++) { + for (int i = 0; i < 40; i++) { futures.add(CompletableFuture.supplyAsync(() -> { try { //GIVEN