From 04d643a19d5a7fbb97ae42fb01912197aacad2c6 Mon Sep 17 00:00:00 2001 From: Dmytro Nosan Date: Fri, 2 May 2025 16:17:31 +0300 Subject: [PATCH] SimpleXsdSchema is not thread-safe Before this commit, SimpleXsdSchema had a reference to an instance of `org.w3c.dom.Element`, which is not thread safe. This causes issues when multiple clients are requesting the schema file simultaneously. This commit updates SimpleXsdSchema to always create a new `ResourceSource` from an XSD Resource whenever getSource is requested. Signed-off-by: Dmytro Nosan --- .../xml/xsd/SimpleXsdSchema.java | 39 +++-- .../xml/xsd/AbstractXsdSchemaTests.java | 150 +++++++++--------- .../xml/xsd/SimpleXsdSchemaTests.java | 4 +- 3 files changed, 101 insertions(+), 92 deletions(-) diff --git a/spring-xml/src/main/java/org/springframework/xml/xsd/SimpleXsdSchema.java b/spring-xml/src/main/java/org/springframework/xml/xsd/SimpleXsdSchema.java index 0450f2c00..62b46cc79 100644 --- a/spring-xml/src/main/java/org/springframework/xml/xsd/SimpleXsdSchema.java +++ b/spring-xml/src/main/java/org/springframework/xml/xsd/SimpleXsdSchema.java @@ -23,7 +23,6 @@ import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; import javax.xml.transform.Source; -import javax.xml.transform.dom.DOMSource; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -34,6 +33,7 @@ import org.springframework.util.Assert; import org.springframework.xml.DocumentBuilderFactoryUtils; import org.springframework.xml.sax.SaxUtils; +import org.springframework.xml.transform.ResourceSource; import org.springframework.xml.validation.XmlValidator; import org.springframework.xml.validation.XmlValidatorFactory; @@ -50,7 +50,7 @@ */ public class SimpleXsdSchema implements XsdSchema, InitializingBean { - private static DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactoryUtils.newInstance(); + private static final DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactoryUtils.newInstance(); private static final String SCHEMA_NAMESPACE = "http://www.w3.org/2001/XMLSchema"; @@ -58,7 +58,7 @@ public class SimpleXsdSchema implements XsdSchema, InitializingBean { private Resource xsdResource; - private Element schemaElement; + private String targetNamespace; static { documentBuilderFactory.setNamespaceAware(true); @@ -95,20 +95,25 @@ public void setXsd(Resource xsdResource) { @Override public String getTargetNamespace() { - - Assert.notNull(this.schemaElement, - "schemaElement must not be null! Did you run afterPropertiesSet() or register this as a Spring bean?"); - - return this.schemaElement.getAttribute("targetNamespace"); + Assert.state(this.targetNamespace != null, + () -> "'targetNamespace' cannot be accessed before afterPropertiesSet() has been called on this instance"); + return this.targetNamespace; } @Override public Source getSource() { - return new DOMSource(this.schemaElement); + Assert.state(this.xsdResource != null, () -> "Source cannot be created from a null XSD resource"); + try { + return new ResourceSource(this.xsdResource); + } + catch (IOException ex) { + throw new XsdSchemaException(ex.getMessage(), ex); + } } @Override public XmlValidator createValidator() { + Assert.state(this.xsdResource != null, () -> "XmlValidator cannot be created from a null XSD resource"); try { return XmlValidatorFactory.createValidator(this.xsdResource, XmlValidatorFactory.SCHEMA_W3C_XML); } @@ -127,13 +132,15 @@ public void afterPropertiesSet() throws ParserConfigurationException, IOExceptio private void loadSchema(DocumentBuilder documentBuilder) throws SAXException, IOException { Document schemaDocument = documentBuilder.parse(SaxUtils.createInputSource(this.xsdResource)); - this.schemaElement = schemaDocument.getDocumentElement(); - Assert.isTrue(SCHEMA_NAME.getLocalPart().equals(this.schemaElement.getLocalName()), this.xsdResource - + " has invalid root element : [" + this.schemaElement.getLocalName() + "] instead of [schema]"); - Assert.isTrue(SCHEMA_NAME.getNamespaceURI().equals(this.schemaElement.getNamespaceURI()), - this.xsdResource + " has invalid root element: [" + this.schemaElement.getNamespaceURI() - + "] instead of [" + SCHEMA_NAME.getNamespaceURI() + "]"); - Assert.hasText(getTargetNamespace(), this.xsdResource + " has no targetNamespace"); + Element schemaElement = schemaDocument.getDocumentElement(); + Assert.isTrue(SCHEMA_NAME.getLocalPart().equals(schemaElement.getLocalName()), this.xsdResource + + " has invalid root element : [" + schemaElement.getLocalName() + "] instead of [schema]"); + Assert.isTrue(SCHEMA_NAME.getNamespaceURI().equals(schemaElement.getNamespaceURI()), + this.xsdResource + " has invalid root element: [" + schemaElement.getNamespaceURI() + "] instead of [" + + SCHEMA_NAME.getNamespaceURI() + "]"); + String targetNamespace = schemaElement.getAttribute("targetNamespace"); + Assert.hasText(targetNamespace, this.xsdResource + " has no targetNamespace"); + this.targetNamespace = targetNamespace; } public String toString() { diff --git a/spring-xml/src/test/java/org/springframework/xml/xsd/AbstractXsdSchemaTests.java b/spring-xml/src/test/java/org/springframework/xml/xsd/AbstractXsdSchemaTests.java index 940d16852..560ada389 100644 --- a/spring-xml/src/test/java/org/springframework/xml/xsd/AbstractXsdSchemaTests.java +++ b/spring-xml/src/test/java/org/springframework/xml/xsd/AbstractXsdSchemaTests.java @@ -16,13 +16,17 @@ package org.springframework.xml.xsd; -import javax.xml.parsers.DocumentBuilder; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; + import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.transform.Transformer; import javax.xml.transform.TransformerFactory; import javax.xml.transform.dom.DOMResult; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.w3c.dom.Document; import org.xmlunit.assertj.XmlAssert; @@ -38,99 +42,97 @@ public abstract class AbstractXsdSchemaTests { - private DocumentBuilder documentBuilder; - - protected Transformer transformer; - - @BeforeEach - public final void setUp() throws Exception { - - DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactoryUtils.newInstance(); - documentBuilderFactory.setNamespaceAware(true); - this.documentBuilder = documentBuilderFactory.newDocumentBuilder(); - TransformerFactory transformerFactory = TransformerFactoryUtils.newInstance(); - this.transformer = transformerFactory.newTransformer(); - } - @Test void testSingle() throws Exception { - - Resource resource = new ClassPathResource("single.xsd", AbstractXsdSchemaTests.class); - XsdSchema single = createSchema(resource); - - assertThat(single.getTargetNamespace()).isEqualTo("http://www.springframework.org/spring-ws/single/schema"); - - resource = new ClassPathResource("single.xsd", AbstractXsdSchemaTests.class); - Document expected = this.documentBuilder.parse(SaxUtils.createInputSource(resource)); - DOMResult domResult = new DOMResult(); - this.transformer.transform(single.getSource(), domResult); - Document result = (Document) domResult.getNode(); - - XmlAssert.assertThat(result).and(expected).ignoreWhitespace().areIdentical(); + Resource xsdResource = new ClassPathResource("single.xsd", AbstractXsdSchemaTests.class); + XsdSchema xsdSchema = createSchema(xsdResource); + assertThat(xsdSchema.getTargetNamespace()).isEqualTo("http://www.springframework.org/spring-ws/single/schema"); + Document actual = createDocument(xsdSchema); + Document expected = createDocument(xsdResource); + XmlAssert.assertThat(actual).and(expected).ignoreWhitespace().areIdentical(); } @Test void testIncludes() throws Exception { - - Resource resource = new ClassPathResource("including.xsd", AbstractXsdSchemaTests.class); - XsdSchema including = createSchema(resource); - - assertThat(including.getTargetNamespace()).isEqualTo("http://www.springframework.org/spring-ws/include/schema"); - - resource = new ClassPathResource("including.xsd", AbstractXsdSchemaTests.class); - Document expected = this.documentBuilder.parse(SaxUtils.createInputSource(resource)); - DOMResult domResult = new DOMResult(); - this.transformer.transform(including.getSource(), domResult); - Document result = (Document) domResult.getNode(); - - XmlAssert.assertThat(result).and(expected).ignoreWhitespace().areIdentical(); + Resource xsdResource = new ClassPathResource("including.xsd", AbstractXsdSchemaTests.class); + XsdSchema xsdSchema = createSchema(xsdResource); + assertThat(xsdSchema.getTargetNamespace()).isEqualTo("http://www.springframework.org/spring-ws/include/schema"); + Document expected = createDocument(xsdResource); + Document actual = createDocument(xsdSchema); + XmlAssert.assertThat(actual).and(expected).ignoreWhitespace().areIdentical(); } @Test void testImports() throws Exception { - - Resource resource = new ClassPathResource("importing.xsd", AbstractXsdSchemaTests.class); - XsdSchema importing = createSchema(resource); - - assertThat(importing.getTargetNamespace()) + Resource xsdResource = new ClassPathResource("importing.xsd", AbstractXsdSchemaTests.class); + XsdSchema xsdSchema = createSchema(xsdResource); + assertThat(xsdSchema.getTargetNamespace()) .isEqualTo("http://www.springframework.org/spring-ws/importing/schema"); - - resource = new ClassPathResource("importing.xsd", AbstractXsdSchemaTests.class); - Document expected = this.documentBuilder.parse(SaxUtils.createInputSource(resource)); - DOMResult domResult = new DOMResult(); - this.transformer.transform(importing.getSource(), domResult); - Document result = (Document) domResult.getNode(); - - XmlAssert.assertThat(result).and(expected).ignoreWhitespace().areIdentical(); + Document expected = createDocument(xsdResource); + Document actual = createDocument(xsdSchema); + XmlAssert.assertThat(actual).and(expected).ignoreWhitespace().areIdentical(); } @Test void testXmlNamespace() throws Exception { - - Resource resource = new ClassPathResource("xmlNamespace.xsd", AbstractXsdSchemaTests.class); - XsdSchema importing = createSchema(resource); - - assertThat(importing.getTargetNamespace()).isEqualTo("http://www.springframework.org/spring-ws/xmlNamespace"); - - resource = new ClassPathResource("xmlNamespace.xsd", AbstractXsdSchemaTests.class); - Document expected = this.documentBuilder.parse(SaxUtils.createInputSource(resource)); - DOMResult domResult = new DOMResult(); - this.transformer.transform(importing.getSource(), domResult); - Document result = (Document) domResult.getNode(); - - XmlAssert.assertThat(result).and(expected).ignoreWhitespace().areIdentical(); + Resource xsdResource = new ClassPathResource("xmlNamespace.xsd", AbstractXsdSchemaTests.class); + XsdSchema xsdSchema = createSchema(xsdResource); + assertThat(xsdSchema.getTargetNamespace()).isEqualTo("http://www.springframework.org/spring-ws/xmlNamespace"); + Document expected = createDocument(xsdResource); + Document actual = createDocument(xsdSchema); + XmlAssert.assertThat(actual).and(expected).ignoreWhitespace().areIdentical(); } @Test void testCreateValidator() throws Exception { - - Resource resource = new ClassPathResource("single.xsd", AbstractXsdSchemaTests.class); - XsdSchema single = createSchema(resource); - XmlValidator validator = single.createValidator(); - + Resource xsdResource = new ClassPathResource("single.xsd", AbstractXsdSchemaTests.class); + XsdSchema xsdSchema = createSchema(xsdResource); + XmlValidator validator = xsdSchema.createValidator(); assertThat(validator).isNotNull(); } + @Test + void testLoadXsdSchemaConcurrently() throws Exception { + ClassPathResource xsdResource = new ClassPathResource("single.xsd", AbstractXsdSchemaTests.class); + XsdSchema xsdSchema = createSchema(xsdResource); + int numberOfThreads = 4; + CountDownLatch startSignal = new CountDownLatch(1); + CountDownLatch readyToStart = new CountDownLatch(numberOfThreads); + ExecutorService executorService = Executors.newFixedThreadPool(numberOfThreads); + List> documents = new ArrayList<>(); + try { + for (int i = 0; i < numberOfThreads; i++) { + documents.add(executorService.submit(() -> { + readyToStart.countDown(); + startSignal.await(); + return createDocument(xsdSchema); + })); + } + readyToStart.await(); + startSignal.countDown(); + Document expected = createDocument(xsdResource); + assertThat(documents).hasSize(numberOfThreads) + .extracting(Future::get) + .allSatisfy((actual) -> XmlAssert.assertThat(actual).and(expected).ignoreWhitespace()); + } + finally { + executorService.shutdownNow(); + } + } + protected abstract XsdSchema createSchema(Resource resource) throws Exception; + private Document createDocument(Resource resource) throws Exception { + DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactoryUtils.newInstance(); + documentBuilderFactory.setNamespaceAware(true); + return documentBuilderFactory.newDocumentBuilder().parse(SaxUtils.createInputSource(resource)); + } + + private Document createDocument(XsdSchema schema) throws Exception { + DOMResult domResult = new DOMResult(); + TransformerFactory transformerFactory = TransformerFactoryUtils.newInstance(); + transformerFactory.newTransformer().transform(schema.getSource(), domResult); + return (Document) domResult.getNode(); + } + } diff --git a/spring-xml/src/test/java/org/springframework/xml/xsd/SimpleXsdSchemaTests.java b/spring-xml/src/test/java/org/springframework/xml/xsd/SimpleXsdSchemaTests.java index 29213278c..768304331 100644 --- a/spring-xml/src/test/java/org/springframework/xml/xsd/SimpleXsdSchemaTests.java +++ b/spring-xml/src/test/java/org/springframework/xml/xsd/SimpleXsdSchemaTests.java @@ -20,7 +20,7 @@ import org.springframework.core.io.Resource; -import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; class SimpleXsdSchemaTests extends AbstractXsdSchemaTests { @@ -35,7 +35,7 @@ protected XsdSchema createSchema(Resource resource) throws Exception { @Test void testBareXsdSchema() { - assertThatIllegalArgumentException().isThrownBy(() -> new SimpleXsdSchema().toString()); + assertThatIllegalStateException().isThrownBy(() -> new SimpleXsdSchema().toString()); } }