Skip to content

Commit d20fd09

Browse files
committed
[CALCITE-5274] Improve DocumentBuilderFactory in DiffRepository test class by using secure features
1 parent 6302e6f commit d20fd09

File tree

1 file changed

+24
-8
lines changed

1 file changed

+24
-8
lines changed

testkit/src/main/java/org/apache/calcite/test/DiffRepository.java

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444

4545
import java.io.File;
4646
import java.io.IOException;
47+
import java.io.InputStream;
4748
import java.io.Writer;
4849
import java.net.URL;
4950
import java.util.AbstractList;
@@ -52,6 +53,7 @@
5253
import java.util.Objects;
5354
import java.util.SortedMap;
5455
import java.util.TreeMap;
56+
import javax.xml.XMLConstants;
5557
import javax.xml.parsers.DocumentBuilder;
5658
import javax.xml.parsers.DocumentBuilderFactory;
5759
import javax.xml.parsers.ParserConfigurationException;
@@ -174,6 +176,22 @@ public class DiffRepository {
174176
private static final LoadingCache<Key, DiffRepository> REPOSITORY_CACHE =
175177
CacheBuilder.newBuilder().build(CacheLoader.from(Key::toRepo));
176178

179+
private static final ThreadLocal<@Nullable DocumentBuilderFactory> DOCUMENT_BUILDER_FACTORY =
180+
ThreadLocal.withInitial(() -> {
181+
final DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance();
182+
documentBuilderFactory.setXIncludeAware(false);
183+
documentBuilderFactory.setExpandEntityReferences(false);
184+
documentBuilderFactory.setNamespaceAware(true);
185+
try {
186+
documentBuilderFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
187+
documentBuilderFactory
188+
.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
189+
} catch (final ParserConfigurationException e) {
190+
throw new IllegalStateException("Document Builder configuration failed", e);
191+
}
192+
return documentBuilderFactory;
193+
});
194+
177195
//~ Instance fields --------------------------------------------------------
178196

179197
private final DiffRepository baseRepository;
@@ -207,19 +225,17 @@ private DiffRepository(URL refFile, File logFile,
207225
this.modCount = 0;
208226

209227
// Load the document.
210-
DocumentBuilderFactory fac = DocumentBuilderFactory.newInstance();
211228
try {
212-
DocumentBuilder docBuilder = fac.newDocumentBuilder();
213-
try {
229+
DocumentBuilder docBuilder =
230+
Nullness.castNonNull(DOCUMENT_BUILDER_FACTORY.get()).newDocumentBuilder();
231+
try (InputStream inputStream = refFile.openStream()) {
214232
// Parse the reference file.
215-
this.doc = docBuilder.parse(refFile.openStream());
216-
// Don't write a log file yet -- as far as we know, it's still
217-
// identical.
233+
this.doc = docBuilder.parse(inputStream);
234+
// Don't write a log file yet -- as far as we know, it's still identical.
218235
} catch (IOException e) {
219236
// There's no reference file. Create and write a log file.
220237
this.doc = docBuilder.newDocument();
221-
this.doc.appendChild(
222-
doc.createElement(ROOT_TAG));
238+
this.doc.appendChild(doc.createElement(ROOT_TAG));
223239
flushDoc();
224240
}
225241
this.root = doc.getDocumentElement();

0 commit comments

Comments
 (0)