HDDS-10777. S3 Gateway error when parsing XML concurrently (#6609)

apache · May 8, 2024 · ff78dc8 · ff78dc8
1 parent bab281c
commit ff78dc8
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 9 deletions.
diff --git a/.../java/org/apache/hadoop/ozone/s3/endpoint/CompleteMultipartUploadRequestUnmarshaller.java b/.../java/org/apache/hadoop/ozone/s3/endpoint/CompleteMultipartUploadRequestUnmarshaller.java
@@ -44,14 +44,13 @@ public class CompleteMultipartUploadRequestUnmarshaller
     implements MessageBodyReader<CompleteMultipartUploadRequest> {
 
   private final JAXBContext context;
-  private final XMLReader xmlReader;
+  private final SAXParserFactory saxParserFactory;
 
   public CompleteMultipartUploadRequestUnmarshaller() {
     try {
       context = JAXBContext.newInstance(CompleteMultipartUploadRequest.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 " +
           "CompleteMultipartUploadRequest parser", ex);
@@ -70,6 +69,7 @@ public CompleteMultipartUploadRequest readFrom(
       MultivaluedMap<String, String> multivaluedMap,
       InputStream inputStream) throws IOException, WebApplicationException {
     try {
+      XMLReader xmlReader = saxParserFactory.newSAXParser().getXMLReader();
       UnmarshallerHandler unmarshallerHandler =
           context.createUnmarshaller().getUnmarshallerHandler();
       XmlNamespaceFilter filter =

diff --git a/...way/src/main/java/org/apache/hadoop/ozone/s3/endpoint/MultiDeleteRequestUnmarshaller.java b/...way/src/main/java/org/apache/hadoop/ozone/s3/endpoint/MultiDeleteRequestUnmarshaller.java
@@ -43,14 +43,13 @@ public class MultiDeleteRequestUnmarshaller
     implements MessageBodyReader<MultiDeleteRequest> {
 
   private final JAXBContext context;
-  private final XMLReader xmlReader;
+  private final SAXParserFactory saxParserFactory;
 
   public MultiDeleteRequestUnmarshaller() {
     try {
       context = JAXBContext.newInstance(MultiDeleteRequest.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't instantiate MultiDeleteRequest parser",
           ex);
@@ -68,6 +67,7 @@ public MultiDeleteRequest readFrom(Class<MultiDeleteRequest> type,
       Type genericType, Annotation[] annotations, MediaType mediaType,
       MultivaluedMap<String, String> httpHeaders, InputStream entityStream) {
     try {
+      XMLReader xmlReader = saxParserFactory.newSAXParser().getXMLReader();
       UnmarshallerHandler unmarshallerHandler =
           context.createUnmarshaller().getUnmarshallerHandler();
 

diff --git a/...ay/src/main/java/org/apache/hadoop/ozone/s3/endpoint/PutBucketAclRequestUnmarshaller.java b/...ay/src/main/java/org/apache/hadoop/ozone/s3/endpoint/PutBucketAclRequestUnmarshaller.java
@@ -44,14 +44,13 @@ public class PutBucketAclRequestUnmarshaller
     implements MessageBodyReader<S3BucketAcl> {
 
   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<String, String> multivaluedMap,
       InputStream inputStream) throws IOException, WebApplicationException {
     try {
+      XMLReader xmlReader = saxParserFactory.newSAXParser().getXMLReader();
       UnmarshallerHandler unmarshallerHandler =
           context.createUnmarshaller().getUnmarshallerHandler();
       XmlNamespaceFilter filter =

diff --git a/...a/org/apache/hadoop/ozone/s3/endpoint/TestCompleteMultipartUploadRequestUnmarshaller.java b/...a/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.assertNotNull;
 
 /**
  * Class tests Unmarshall logic of {@link CompleteMultipartUploadRequest}.
@@ -75,6 +79,7 @@ public void fromStreamWithoutNamespace() throws IOException {
   }
 
   private void checkContent(CompleteMultipartUploadRequest request) {
+    assertNotNull(request);
     assertEquals(2, request.getPartList().size());
 
     List<CompleteMultipartUploadRequest.Part> 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 = ("<CompleteMultipartUpload>" + "<Part><ETag>" + part1 +
+        "</ETag><PartNumber>1</PartNumber" + "></Part><Part><ETag>" +
+        part2 + "</ETag><PartNumber>2" +
+        "</PartNumber></Part></CompleteMultipartUpload>").getBytes(
+        UTF_8);
+
+    List<CompletableFuture<CompleteMultipartUploadRequest>> futures =
+        new ArrayList<>();
+    for (int i = 0; i < 40; i++) {
+      futures.add(CompletableFuture.supplyAsync(() -> {
+        try {
+          //GIVEN
+          ByteArrayInputStream inputBody = new ByteArrayInputStream(bytes);
+          //WHEN
+          return unmarshall(unmarshaller, inputBody);
+        } catch (IOException e) {
+          return null;
+        }
+      }));
+    }
+
+    for (CompletableFuture<CompleteMultipartUploadRequest> 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);
+  }
 }