Skip to content

Commit 04d643a

Browse files
nosannosan
committed
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 <[email protected]>
1 parent e036303 commit 04d643a

File tree

3 files changed

+101
-92
lines changed

3 files changed

+101
-92
lines changed

‎spring-xml/src/main/java/org/springframework/xml/xsd/SimpleXsdSchema.java‎

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import javax.xml.parsers.DocumentBuilderFactory;
2424
import javax.xml.parsers.ParserConfigurationException;
2525
import javax.xml.transform.Source;
26-
import javax.xml.transform.dom.DOMSource;
2726

2827
import org.w3c.dom.Document;
2928
import org.w3c.dom.Element;
@@ -34,6 +33,7 @@
3433
import org.springframework.util.Assert;
3534
import org.springframework.xml.DocumentBuilderFactoryUtils;
3635
import org.springframework.xml.sax.SaxUtils;
36+
import org.springframework.xml.transform.ResourceSource;
3737
import org.springframework.xml.validation.XmlValidator;
3838
import org.springframework.xml.validation.XmlValidatorFactory;
3939

@@ -50,15 +50,15 @@
5050
*/
5151
public class SimpleXsdSchema implements XsdSchema, InitializingBean {
5252

53-
private static DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactoryUtils.newInstance();
53+
private static final DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactoryUtils.newInstance();
5454

5555
private static final String SCHEMA_NAMESPACE = "http://www.w3.org/2001/XMLSchema";
5656

5757
private static final QName SCHEMA_NAME = new QName(SCHEMA_NAMESPACE, "schema", "xsd");
5858

5959
private Resource xsdResource;
6060

61-
private Element schemaElement;
61+
private String targetNamespace;
6262

6363
static {
6464
documentBuilderFactory.setNamespaceAware(true);
@@ -95,20 +95,25 @@ public void setXsd(Resource xsdResource) {
9595

9696
@Override
9797
public String getTargetNamespace() {
98-
99-
Assert.notNull(this.schemaElement,
100-
"schemaElement must not be null! Did you run afterPropertiesSet() or register this as a Spring bean?");
101-
102-
return this.schemaElement.getAttribute("targetNamespace");
98+
Assert.state(this.targetNamespace != null,
99+
() -> "'targetNamespace' cannot be accessed before afterPropertiesSet() has been called on this instance");
100+
return this.targetNamespace;
103101
}
104102

105103
@Override
106104
public Source getSource() {
107-
return new DOMSource(this.schemaElement);
105+
Assert.state(this.xsdResource != null, () -> "Source cannot be created from a null XSD resource");
106+
try {
107+
return new ResourceSource(this.xsdResource);
108+
}
109+
catch (IOException ex) {
110+
throw new XsdSchemaException(ex.getMessage(), ex);
111+
}
108112
}
109113

110114
@Override
111115
public XmlValidator createValidator() {
116+
Assert.state(this.xsdResource != null, () -> "XmlValidator cannot be created from a null XSD resource");
112117
try {
113118
return XmlValidatorFactory.createValidator(this.xsdResource, XmlValidatorFactory.SCHEMA_W3C_XML);
114119
}
@@ -127,13 +132,15 @@ public void afterPropertiesSet() throws ParserConfigurationException, IOExceptio
127132

128133
private void loadSchema(DocumentBuilder documentBuilder) throws SAXException, IOException {
129134
Document schemaDocument = documentBuilder.parse(SaxUtils.createInputSource(this.xsdResource));
130-
this.schemaElement = schemaDocument.getDocumentElement();
131-
Assert.isTrue(SCHEMA_NAME.getLocalPart().equals(this.schemaElement.getLocalName()), this.xsdResource
132-
+ " has invalid root element : [" + this.schemaElement.getLocalName() + "] instead of [schema]");
133-
Assert.isTrue(SCHEMA_NAME.getNamespaceURI().equals(this.schemaElement.getNamespaceURI()),
134-
this.xsdResource + " has invalid root element: [" + this.schemaElement.getNamespaceURI()
135-
+ "] instead of [" + SCHEMA_NAME.getNamespaceURI() + "]");
136-
Assert.hasText(getTargetNamespace(), this.xsdResource + " has no targetNamespace");
135+
Element schemaElement = schemaDocument.getDocumentElement();
136+
Assert.isTrue(SCHEMA_NAME.getLocalPart().equals(schemaElement.getLocalName()), this.xsdResource
137+
+ " has invalid root element : [" + schemaElement.getLocalName() + "] instead of [schema]");
138+
Assert.isTrue(SCHEMA_NAME.getNamespaceURI().equals(schemaElement.getNamespaceURI()),
139+
this.xsdResource + " has invalid root element: [" + schemaElement.getNamespaceURI() + "] instead of ["
140+
+ SCHEMA_NAME.getNamespaceURI() + "]");
141+
String targetNamespace = schemaElement.getAttribute("targetNamespace");
142+
Assert.hasText(targetNamespace, this.xsdResource + " has no targetNamespace");
143+
this.targetNamespace = targetNamespace;
137144
}
138145

139146
public String toString() {

‎spring-xml/src/test/java/org/springframework/xml/xsd/AbstractXsdSchemaTests.java‎

Lines changed: 76 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,17 @@
1616

1717
package org.springframework.xml.xsd;
1818

19-
import javax.xml.parsers.DocumentBuilder;
19+
import java.util.ArrayList;
20+
import java.util.List;
21+
import java.util.concurrent.CountDownLatch;
22+
import java.util.concurrent.ExecutorService;
23+
import java.util.concurrent.Executors;
24+
import java.util.concurrent.Future;
25+
2026
import javax.xml.parsers.DocumentBuilderFactory;
21-
import javax.xml.transform.Transformer;
2227
import javax.xml.transform.TransformerFactory;
2328
import javax.xml.transform.dom.DOMResult;
2429

25-
import org.junit.jupiter.api.BeforeEach;
2630
import org.junit.jupiter.api.Test;
2731
import org.w3c.dom.Document;
2832
import org.xmlunit.assertj.XmlAssert;
@@ -38,99 +42,97 @@
3842

3943
public abstract class AbstractXsdSchemaTests {
4044

41-
private DocumentBuilder documentBuilder;
42-
43-
protected Transformer transformer;
44-
45-
@BeforeEach
46-
public final void setUp() throws Exception {
47-
48-
DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactoryUtils.newInstance();
49-
documentBuilderFactory.setNamespaceAware(true);
50-
this.documentBuilder = documentBuilderFactory.newDocumentBuilder();
51-
TransformerFactory transformerFactory = TransformerFactoryUtils.newInstance();
52-
this.transformer = transformerFactory.newTransformer();
53-
}
54-
5545
@Test
5646
void testSingle() throws Exception {
57-
58-
Resource resource = new ClassPathResource("single.xsd", AbstractXsdSchemaTests.class);
59-
XsdSchema single = createSchema(resource);
60-
61-
assertThat(single.getTargetNamespace()).isEqualTo("http://www.springframework.org/spring-ws/single/schema");
62-
63-
resource = new ClassPathResource("single.xsd", AbstractXsdSchemaTests.class);
64-
Document expected = this.documentBuilder.parse(SaxUtils.createInputSource(resource));
65-
DOMResult domResult = new DOMResult();
66-
this.transformer.transform(single.getSource(), domResult);
67-
Document result = (Document) domResult.getNode();
68-
69-
XmlAssert.assertThat(result).and(expected).ignoreWhitespace().areIdentical();
47+
Resource xsdResource = new ClassPathResource("single.xsd", AbstractXsdSchemaTests.class);
48+
XsdSchema xsdSchema = createSchema(xsdResource);
49+
assertThat(xsdSchema.getTargetNamespace()).isEqualTo("http://www.springframework.org/spring-ws/single/schema");
50+
Document actual = createDocument(xsdSchema);
51+
Document expected = createDocument(xsdResource);
52+
XmlAssert.assertThat(actual).and(expected).ignoreWhitespace().areIdentical();
7053
}
7154

7255
@Test
7356
void testIncludes() throws Exception {
74-
75-
Resource resource = new ClassPathResource("including.xsd", AbstractXsdSchemaTests.class);
76-
XsdSchema including = createSchema(resource);
77-
78-
assertThat(including.getTargetNamespace()).isEqualTo("http://www.springframework.org/spring-ws/include/schema");
79-
80-
resource = new ClassPathResource("including.xsd", AbstractXsdSchemaTests.class);
81-
Document expected = this.documentBuilder.parse(SaxUtils.createInputSource(resource));
82-
DOMResult domResult = new DOMResult();
83-
this.transformer.transform(including.getSource(), domResult);
84-
Document result = (Document) domResult.getNode();
85-
86-
XmlAssert.assertThat(result).and(expected).ignoreWhitespace().areIdentical();
57+
Resource xsdResource = new ClassPathResource("including.xsd", AbstractXsdSchemaTests.class);
58+
XsdSchema xsdSchema = createSchema(xsdResource);
59+
assertThat(xsdSchema.getTargetNamespace()).isEqualTo("http://www.springframework.org/spring-ws/include/schema");
60+
Document expected = createDocument(xsdResource);
61+
Document actual = createDocument(xsdSchema);
62+
XmlAssert.assertThat(actual).and(expected).ignoreWhitespace().areIdentical();
8763
}
8864

8965
@Test
9066
void testImports() throws Exception {
91-
92-
Resource resource = new ClassPathResource("importing.xsd", AbstractXsdSchemaTests.class);
93-
XsdSchema importing = createSchema(resource);
94-
95-
assertThat(importing.getTargetNamespace())
67+
Resource xsdResource = new ClassPathResource("importing.xsd", AbstractXsdSchemaTests.class);
68+
XsdSchema xsdSchema = createSchema(xsdResource);
69+
assertThat(xsdSchema.getTargetNamespace())
9670
.isEqualTo("http://www.springframework.org/spring-ws/importing/schema");
97-
98-
resource = new ClassPathResource("importing.xsd", AbstractXsdSchemaTests.class);
99-
Document expected = this.documentBuilder.parse(SaxUtils.createInputSource(resource));
100-
DOMResult domResult = new DOMResult();
101-
this.transformer.transform(importing.getSource(), domResult);
102-
Document result = (Document) domResult.getNode();
103-
104-
XmlAssert.assertThat(result).and(expected).ignoreWhitespace().areIdentical();
71+
Document expected = createDocument(xsdResource);
72+
Document actual = createDocument(xsdSchema);
73+
XmlAssert.assertThat(actual).and(expected).ignoreWhitespace().areIdentical();
10574
}
10675

10776
@Test
10877
void testXmlNamespace() throws Exception {
109-
110-
Resource resource = new ClassPathResource("xmlNamespace.xsd", AbstractXsdSchemaTests.class);
111-
XsdSchema importing = createSchema(resource);
112-
113-
assertThat(importing.getTargetNamespace()).isEqualTo("http://www.springframework.org/spring-ws/xmlNamespace");
114-
115-
resource = new ClassPathResource("xmlNamespace.xsd", AbstractXsdSchemaTests.class);
116-
Document expected = this.documentBuilder.parse(SaxUtils.createInputSource(resource));
117-
DOMResult domResult = new DOMResult();
118-
this.transformer.transform(importing.getSource(), domResult);
119-
Document result = (Document) domResult.getNode();
120-
121-
XmlAssert.assertThat(result).and(expected).ignoreWhitespace().areIdentical();
78+
Resource xsdResource = new ClassPathResource("xmlNamespace.xsd", AbstractXsdSchemaTests.class);
79+
XsdSchema xsdSchema = createSchema(xsdResource);
80+
assertThat(xsdSchema.getTargetNamespace()).isEqualTo("http://www.springframework.org/spring-ws/xmlNamespace");
81+
Document expected = createDocument(xsdResource);
82+
Document actual = createDocument(xsdSchema);
83+
XmlAssert.assertThat(actual).and(expected).ignoreWhitespace().areIdentical();
12284
}
12385

12486
@Test
12587
void testCreateValidator() throws Exception {
126-
127-
Resource resource = new ClassPathResource("single.xsd", AbstractXsdSchemaTests.class);
128-
XsdSchema single = createSchema(resource);
129-
XmlValidator validator = single.createValidator();
130-
88+
Resource xsdResource = new ClassPathResource("single.xsd", AbstractXsdSchemaTests.class);
89+
XsdSchema xsdSchema = createSchema(xsdResource);
90+
XmlValidator validator = xsdSchema.createValidator();
13191
assertThat(validator).isNotNull();
13292
}
13393

94+
@Test
95+
void testLoadXsdSchemaConcurrently() throws Exception {
96+
ClassPathResource xsdResource = new ClassPathResource("single.xsd", AbstractXsdSchemaTests.class);
97+
XsdSchema xsdSchema = createSchema(xsdResource);
98+
int numberOfThreads = 4;
99+
CountDownLatch startSignal = new CountDownLatch(1);
100+
CountDownLatch readyToStart = new CountDownLatch(numberOfThreads);
101+
ExecutorService executorService = Executors.newFixedThreadPool(numberOfThreads);
102+
List<Future<Document>> documents = new ArrayList<>();
103+
try {
104+
for (int i = 0; i < numberOfThreads; i++) {
105+
documents.add(executorService.submit(() -> {
106+
readyToStart.countDown();
107+
startSignal.await();
108+
return createDocument(xsdSchema);
109+
}));
110+
}
111+
readyToStart.await();
112+
startSignal.countDown();
113+
Document expected = createDocument(xsdResource);
114+
assertThat(documents).hasSize(numberOfThreads)
115+
.extracting(Future::get)
116+
.allSatisfy((actual) -> XmlAssert.assertThat(actual).and(expected).ignoreWhitespace());
117+
}
118+
finally {
119+
executorService.shutdownNow();
120+
}
121+
}
122+
134123
protected abstract XsdSchema createSchema(Resource resource) throws Exception;
135124

125+
private Document createDocument(Resource resource) throws Exception {
126+
DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactoryUtils.newInstance();
127+
documentBuilderFactory.setNamespaceAware(true);
128+
return documentBuilderFactory.newDocumentBuilder().parse(SaxUtils.createInputSource(resource));
129+
}
130+
131+
private Document createDocument(XsdSchema schema) throws Exception {
132+
DOMResult domResult = new DOMResult();
133+
TransformerFactory transformerFactory = TransformerFactoryUtils.newInstance();
134+
transformerFactory.newTransformer().transform(schema.getSource(), domResult);
135+
return (Document) domResult.getNode();
136+
}
137+
136138
}

‎spring-xml/src/test/java/org/springframework/xml/xsd/SimpleXsdSchemaTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020

2121
import org.springframework.core.io.Resource;
2222

23-
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
23+
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
2424

2525
class SimpleXsdSchemaTests extends AbstractXsdSchemaTests {
2626

@@ -35,7 +35,7 @@ protected XsdSchema createSchema(Resource resource) throws Exception {
3535

3636
@Test
3737
void testBareXsdSchema() {
38-
assertThatIllegalArgumentException().isThrownBy(() -> new SimpleXsdSchema().toString());
38+
assertThatIllegalStateException().isThrownBy(() -> new SimpleXsdSchema().toString());
3939
}
4040

4141
}

0 commit comments

Comments
 (0)