This is an automated email from the ASF dual-hosted git repository.
desruisseaux pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git
The following commit(s) were added to refs/heads/geoapi-4.0 by this push:
new 5bfa162bd5 Cleanup `XMLInputFactory` usages. Set `ACCESS_EXTERNAL_DTD`
property to a value that exclude the "file" protocol.
5bfa162bd5 is described below
commit 5bfa162bd56edb7d41d56a0c926592964d31d83b
Author: Martin Desruisseaux <[email protected]>
AuthorDate: Sat Dec 13 02:24:59 2025 +0100
Cleanup `XMLInputFactory` usages.
Set `ACCESS_EXTERNAL_DTD` property to a value that exclude the "file"
protocol.
---
.../org.apache.sis.metadata/main/module-info.java | 1 +
.../main/org/apache/sis/xml/PooledMarshaller.java | 1 +
.../org/apache/sis/xml/PooledUnmarshaller.java | 1 +
.../xml/internal/shared/ExternalLinkHandler.java | 4 +-
.../xml/{ => internal/shared}/InputFactory.java | 27 +++-
.../xml/{ => internal/shared}/OutputFactory.java | 10 +-
.../test/org/apache/sis/xml/LegacyCodesTest.java | 1 +
.../org/apache/sis/xml/MarshallerPoolTest.java | 1 +
.../test/org/apache/sis/xml/NamespacesTest.java | 1 +
.../test/org/apache/sis/xml/NilReasonTest.java | 3 +-
.../test/org/apache/sis/xml/TransformerTest.java | 1 +
.../apache/sis/xml/TransformingNamespacesTest.java | 1 +
.../org/apache/sis/xml/ValueConverterTest.java | 1 +
.../test/org/apache/sis/xml/XLinkTest.java | 3 +-
.../test/org/apache/sis/xml/XPointerTest.java | 1 +
.../sis/xml/internal/shared/InputFactoryTest.java | 160 +++++++++++++++++++++
.../sis/storage/geotiff/reader/XMLMetadata.java | 7 +-
.../org/apache/sis/storage/gpx/WritableStore.java | 2 +-
.../apache/sis/storage/xml/stream/InputType.java | 6 +-
.../apache/sis/storage/xml/stream/OutputType.java | 6 +-
.../sis/storage/xml/stream/RewriteOnUpdate.java | 2 +-
.../sis/storage/xml/stream/StaxDataStore.java | 74 +++++-----
.../sis/storage/xml/stream/StaxStreamWriter.java | 4 +-
.../org/apache/sis/storage/base/URIDataStore.java | 5 +-
24 files changed, 260 insertions(+), 63 deletions(-)
diff --git a/endorsed/src/org.apache.sis.metadata/main/module-info.java
b/endorsed/src/org.apache.sis.metadata/main/module-info.java
index e76f65657f..31f8ae9eb3 100644
--- a/endorsed/src/org.apache.sis.metadata/main/module-info.java
+++ b/endorsed/src/org.apache.sis.metadata/main/module-info.java
@@ -173,6 +173,7 @@ module org.apache.sis.metadata {
org.apache.sis.referencing,
org.apache.sis.storage,
org.apache.sis.storage.xml,
+ org.apache.sis.storage.geotiff,
org.apache.sis.gui; // In the "optional"
sub-project.
/*
diff --git
a/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/PooledMarshaller.java
b/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/PooledMarshaller.java
index b9a5cce3bf..eb619cfc6c 100644
---
a/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/PooledMarshaller.java
+++
b/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/PooledMarshaller.java
@@ -38,6 +38,7 @@ import org.xml.sax.ContentHandler;
import org.w3c.dom.Node;
import org.apache.sis.xml.bind.Context;
import org.apache.sis.xml.bind.UseLegacyMetadata;
+import org.apache.sis.xml.internal.shared.OutputFactory;
import org.apache.sis.xml.internal.shared.ExternalLinkHandler;
diff --git
a/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/PooledUnmarshaller.java
b/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/PooledUnmarshaller.java
index 00eead6dd6..84e86f0793 100644
---
a/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/PooledUnmarshaller.java
+++
b/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/PooledUnmarshaller.java
@@ -44,6 +44,7 @@ import jakarta.xml.bind.attachment.AttachmentUnmarshaller;
import org.w3c.dom.Node;
import org.xml.sax.InputSource;
import org.apache.sis.xml.bind.Context;
+import org.apache.sis.xml.internal.shared.InputFactory;
import org.apache.sis.xml.internal.shared.ExternalLinkHandler;
import org.apache.sis.util.resources.Errors;
diff --git
a/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/internal/shared/ExternalLinkHandler.java
b/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/internal/shared/ExternalLinkHandler.java
index 068f6a6b27..5aa850b588 100644
---
a/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/internal/shared/ExternalLinkHandler.java
+++
b/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/internal/shared/ExternalLinkHandler.java
@@ -283,7 +283,7 @@ public class ExternalLinkHandler {
if (!(property instanceof XMLResolver)) {
return new ExternalLinkHandler(base);
}
- final XMLResolver resolver = (XMLResolver) property;
+ final var resolver = (XMLResolver) property;
return new ExternalLinkHandler(base) {
@Override public Source openReader(final URI path) throws
Exception {
/*
@@ -311,6 +311,8 @@ public class ExternalLinkHandler {
/**
* Returns a string representation of this link handler for debugging
purposes.
+ *
+ * @return a string representation for debugging purposes.
*/
@Override
public String toString() {
diff --git
a/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/InputFactory.java
b/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/internal/shared/InputFactory.java
similarity index 81%
rename from
endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/InputFactory.java
rename to
endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/internal/shared/InputFactory.java
index 6e193fcf80..1fd6bd6437 100644
---
a/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/InputFactory.java
+++
b/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/internal/shared/InputFactory.java
@@ -14,10 +14,11 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-package org.apache.sis.xml;
+package org.apache.sis.xml.internal.shared;
import java.io.Reader;
import java.io.InputStream;
+import javax.xml.XMLConstants;
import javax.xml.stream.XMLEventReader;
import javax.xml.stream.XMLInputFactory;
import javax.xml.stream.XMLStreamReader;
@@ -35,17 +36,31 @@ import org.xml.sax.InputSource;
* This convenience is provided in a separated class in order to allow the JVM
to instantiate the factory
* only when first needed, when initializing this class.
*
+ * <h2>Security</h2>
+ * Unless the user has configured the {@code javax.xml.accessExternalDTD}
property to something else
+ * than {@code "all"}, this class disallows external DTDs referenced by the
{@code "file"} protocols.
+ * Allowed protocols are <abbr>HTTP</abbr> and <abbr>HTTPS</abbr> (list may be
expanded if needed).
+ *
* @author Martin Desruisseaux (Geomatys)
+ *
+ * @see <a href="https://openjdk.org/jeps/185";>JEP 185: Restrict Fetching of
External XML Resources</a>
*/
-final class InputFactory {
+public final class InputFactory {
/**
* The SIS-wide factory. This factory can be specified by the user, for
example using the
- * {@code javax.xml.stream.XMLInputFactory} system property.
+ * {@code javax.xml.stream.XMLInputFactory} system property or with {@code
META-INF/services}.
*
- * <div class="note"><b>Note:</b>
- * {@code XMLInputFactory} has an {@code newDefaultFactory()} method which
bypass user settings.</div>
+ * @see org.apache.sis.storage.xml.stream.StaxDataStore#inputFactory()
*/
private static final XMLInputFactory FACTORY =
XMLInputFactory.newInstance();
+ static {
+ if
(FACTORY.isPropertySupported(XMLConstants.FEATURE_SECURE_PROCESSING)) {
+ FACTORY.setProperty(XMLConstants.FEATURE_SECURE_PROCESSING,
Boolean.TRUE);
+ }
+ if
("all".equals(FACTORY.getProperty(XMLConstants.ACCESS_EXTERNAL_DTD))) {
+ FACTORY.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD,
"http,https");
+ }
+ }
/**
* Do not allow instantiation of this class.
@@ -68,7 +83,7 @@ final class InputFactory {
* @throws XMLStreamException if the reader cannot be created.
*/
public static XMLEventReader createXMLEventReader(final InputStream in)
throws XMLStreamException {
- return FACTORY.createXMLEventReader(in);
+ return FACTORY.createXMLEventReader(in, "UTF-8");
}
/**
diff --git
a/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/OutputFactory.java
b/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/internal/shared/OutputFactory.java
similarity index 95%
rename from
endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/OutputFactory.java
rename to
endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/internal/shared/OutputFactory.java
index 9818c0dad3..6a9e53f025 100644
---
a/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/OutputFactory.java
+++
b/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/xml/internal/shared/OutputFactory.java
@@ -14,7 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-package org.apache.sis.xml;
+package org.apache.sis.xml.internal.shared;
import java.io.OutputStream;
import java.io.Writer;
@@ -28,7 +28,6 @@ import javax.xml.transform.sax.SAXResult;
import javax.xml.transform.stax.StAXResult;
import org.w3c.dom.Node;
import org.xml.sax.ContentHandler;
-import org.apache.sis.xml.internal.shared.StreamWriterDelegate;
/**
@@ -38,13 +37,10 @@ import
org.apache.sis.xml.internal.shared.StreamWriterDelegate;
*
* @author Martin Desruisseaux (Geomatys)
*/
-final class OutputFactory {
+public final class OutputFactory {
/**
* The SIS-wide factory. This factory can be specified by the user, for
example using the
- * {@code javax.xml.stream.XMLOutputFactory} system property.
- *
- * <div class="note"><b>Note:</b>
- * {@code XMLOutputFactory} has an {@code newDefaultFactory()} method
which bypass user settings.</div>
+ * {@code javax.xml.stream.XMLOutputFactory} system property or with
{@code META-INF/services}.
*/
private static final XMLOutputFactory FACTORY =
XMLOutputFactory.newInstance();
diff --git
a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/LegacyCodesTest.java
b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/LegacyCodesTest.java
index 7a8f400963..28254b2936 100644
---
a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/LegacyCodesTest.java
+++
b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/LegacyCodesTest.java
@@ -27,6 +27,7 @@ import org.apache.sis.test.TestCase;
*
* @author Martin Desruisseaux (Geomatys)
*/
+@SuppressWarnings("exports")
public final class LegacyCodesTest extends TestCase {
/**
* Creates a new test case.
diff --git
a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/MarshallerPoolTest.java
b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/MarshallerPoolTest.java
index 8200948563..e32ec53158 100644
---
a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/MarshallerPoolTest.java
+++
b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/MarshallerPoolTest.java
@@ -31,6 +31,7 @@ import org.apache.sis.test.TestCase;
*
* @author Martin Desruisseaux (Geomatys)
*/
+@SuppressWarnings("exports")
public final class MarshallerPoolTest extends TestCase {
/**
* Creates a new test case.
diff --git
a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/NamespacesTest.java
b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/NamespacesTest.java
index 8adb8f9cd0..37eef8de53 100644
---
a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/NamespacesTest.java
+++
b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/NamespacesTest.java
@@ -29,6 +29,7 @@ import org.apache.sis.test.TestCase;
*
* @author Martin Desruisseaux (Geomatys)
*/
+@SuppressWarnings("exports")
public final class NamespacesTest extends TestCase {
/**
* Creates a new test case.
diff --git
a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/NilReasonTest.java
b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/NilReasonTest.java
index 6ad32a7d81..364c251248 100644
---
a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/NilReasonTest.java
+++
b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/NilReasonTest.java
@@ -34,10 +34,11 @@ import org.opengis.metadata.citation.Responsibility;
/**
- * Tests the {@link NilReason}.
+ * Tests {@link NilReason}.
*
* @author Martin Desruisseaux (Geomatys)
*/
+@SuppressWarnings("exports")
public final class NilReasonTest extends TestCase {
/**
* Creates a new test case.
diff --git
a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/TransformerTest.java
b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/TransformerTest.java
index 5bd3ad013c..6a07570429 100644
---
a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/TransformerTest.java
+++
b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/TransformerTest.java
@@ -32,6 +32,7 @@ import org.apache.sis.test.TestCase;
*
* @author Martin Desruisseaux (Geomatys)
*/
+@SuppressWarnings("exports")
public final class TransformerTest extends TestCase {
/**
* Creates a new test case.
diff --git
a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/TransformingNamespacesTest.java
b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/TransformingNamespacesTest.java
index b3ef68acd9..7aa72a61b5 100644
---
a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/TransformingNamespacesTest.java
+++
b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/TransformingNamespacesTest.java
@@ -36,6 +36,7 @@ import static org.apache.sis.test.Assertions.assertSetEquals;
*
* @author Martin Desruisseaux (Geomatys)
*/
+@SuppressWarnings("exports")
public final class TransformingNamespacesTest extends TestCase implements
NamespaceContext {
/**
* All prefixes declared in this test.
diff --git
a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/ValueConverterTest.java
b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/ValueConverterTest.java
index 700ccca3a4..a02f6de7eb 100644
---
a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/ValueConverterTest.java
+++
b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/ValueConverterTest.java
@@ -33,6 +33,7 @@ import org.apache.sis.test.TestCase;
*
* @author Martin Desruisseaux (Geomatys)
*/
+@SuppressWarnings("exports")
public final class ValueConverterTest extends TestCase {
/**
* Creates a new test case.
diff --git
a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/XLinkTest.java
b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/XLinkTest.java
index c118b1dbc6..67703036bd 100644
---
a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/XLinkTest.java
+++
b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/XLinkTest.java
@@ -28,10 +28,11 @@ import static
org.apache.sis.test.Assertions.assertMessageContains;
/**
- * Tests the {@link XLink}.
+ * Tests {@link XLink}.
*
* @author Martin Desruisseaux (Geomatys)
*/
+@SuppressWarnings("exports")
public final class XLinkTest extends TestCase {
/**
* Creates a new test case.
diff --git
a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/XPointerTest.java
b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/XPointerTest.java
index 74ed67096e..f0e35a627d 100644
---
a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/XPointerTest.java
+++
b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/XPointerTest.java
@@ -29,6 +29,7 @@ import org.apache.sis.test.TestCase;
*
* @author Martin Desruisseaux (Geomatys)
*/
+@SuppressWarnings("exports")
public final class XPointerTest extends TestCase {
/**
* Creates a new test case.
diff --git
a/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/internal/shared/InputFactoryTest.java
b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/internal/shared/InputFactoryTest.java
new file mode 100644
index 0000000000..82ef0427e3
--- /dev/null
+++
b/endorsed/src/org.apache.sis.metadata/test/org/apache/sis/xml/internal/shared/InputFactoryTest.java
@@ -0,0 +1,160 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sis.xml.internal.shared;
+
+import java.io.IOException;
+import java.io.StringReader;
+import java.nio.file.Path;
+import java.nio.file.Files;
+import javax.xml.XMLConstants;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
+
+// Test dependencies
+import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assumptions.*;
+import static org.apache.sis.test.Assertions.assertMessageContains;
+import org.apache.sis.test.TestCase;
+
+
+/**
+ * Tests related to {@link InputFactory}.
+ *
+ * <h2>Security</h2>
+ * This class checks the fix for the "XML External Entity Prevention"
vulnerability.
+ * Users of Apache <abbr>SIS</abbr> versions without this fix can avoid the
vulnerability
+ * by setting the {@code javax.xml.accessExternalDTD} property to a value such
as {@code ""}
+ * (for blocking all accesses) or {@code "https"} (for allowing
<abbr>HTTPS</abbr> accesses)
+ * at <abbr>JVM</abbr> launch time.
+ *
+ * @author Martin Desruisseaux (Geomatys)
+ *
+ * @see <a href="https://openjdk.org/jeps/185";>JEP 185: Restrict Fetching of
External XML Resources</a>
+ * @see <a
href="https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html";>XML
External Entity Prevention</a>
+ */
+@SuppressWarnings("exports")
+public final class InputFactoryTest extends TestCase {
+ /**
+ * Creates a new test case.
+ */
+ public InputFactoryTest() {
+ }
+
+ /**
+ * Verifies the capability to access an external <abbr>XML</abbr> entity.
+ * This is used as a reference for testing the effect of different options
+ * in other test cases.
+ *
+ * @throws IOException if an error occurred while writing the test file.
+ * @throws XMLStreamException if an error occurred while parsing the
<abbr>XML</abbr>.
+ */
+ @Test
+ public void verifyExternalEntityAccess() throws IOException,
XMLStreamException {
+ final XMLInputFactory factory = XMLInputFactory.newInstance();
+
assumeTrue("all".equals(factory.getProperty(XMLConstants.ACCESS_EXTERNAL_DTD)));
+
assumeTrue(Boolean.TRUE.equals(factory.getProperty(XMLInputFactory.SUPPORT_DTD)));
+
assumeTrue(Boolean.TRUE.equals(factory.getProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES)));
+ readExternalEntity(factory, false);
+ }
+
+ /**
+ * Verifies that the {@code IS_SUPPORTING_EXTERNAL_ENTITIES} property
blocks the access to the local file.
+ * Instead, the value is {@code null}.
+ *
+ * @throws IOException if an error occurred while writing the test file.
+ * @throws XMLStreamException if an error occurred while parsing the
<abbr>XML</abbr>.
+ */
+ @Test
+ public void testDisableExternalEntities() throws IOException,
XMLStreamException {
+ final XMLInputFactory factory = XMLInputFactory.newInstance();
+ factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES,
Boolean.FALSE);
+ readExternalEntity(factory, true);
+ }
+
+ /**
+ * Verifies that the {@code SUPPORT_DTD} property blocks the access to the
local file.
+ * Instead, an {@link XMLStreamException} is thrown.
+ *
+ * @throws IOException if an error occurred while writing the test file.
+ */
+ @Test
+ public void testDisableDTD() throws IOException {
+ final XMLInputFactory factory = XMLInputFactory.newInstance();
+ factory.setProperty(XMLInputFactory.SUPPORT_DTD, Boolean.FALSE);
+ var ex = assertThrows(XMLStreamException.class, () ->
readExternalEntity(factory, false));
+ assertMessageContains(ex, "xxe");
+ }
+
+ /**
+ * Verifies that the {@code SUPPORT_DTD} property blocks the access to the
local file.
+ * Instead, an {@link XMLStreamException} is thrown.
+ *
+ * @throws IOException if an error occurred while writing the test file.
+ */
+ @Test
+ public void testDisableAccessExternalDTD() throws IOException {
+ final XMLInputFactory factory = XMLInputFactory.newInstance();
+ factory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, "http,https");
+ var ex = assertThrows(XMLStreamException.class, () ->
readExternalEntity(factory, false));
+ assertMessageContains(ex, "sis-"); // Prefix of the temporary
file.
+ }
+
+ /**
+ * Tries to get the content of a temporary file on the local file by
parsing a <abbr>XML</abbr> document.
+ *
+ * @param factory the factory to use for attempting an attack.
+ * @param blocked whether we expect the access to be blocked.
+ * @throws IOException if an error occurred while writing the test file.
+ * @throws XMLStreamException if an error occurred while parsing the
<abbr>XML</abbr>.
+ */
+ private void readExternalEntity(final XMLInputFactory factory, final
boolean blocked) throws IOException, XMLStreamException {
+ String content = null;
+ final String text = "Some text which should be inaccessible.";
+ final Path path = Files.createTempFile("sis-", ".tmp");
+ try {
+ Files.writeString(path, text);
+ final String xml = String.format(
+ "<!DOCTYPE foo [%n" +
+ " <!ELEMENT foo ANY >%n" +
+ " <!ENTITY xxe SYSTEM \"%s\" >%n" +
+ "]>%n" +
+ "<foo>&xxe;</foo>%n", path.toUri());
+
+ final XMLStreamReader reader = factory.createXMLStreamReader(new
StringReader(xml));
+ while (reader.hasNext()) {
+ int event = reader.next();
+ if (event == XMLStreamReader.CHARACTERS) {
+ String found = reader.getText();
+ if (found != null && !(found = found.trim()).isEmpty()) {
+ assertNull(content); // We expect only one
occurrence.
+ content = found;
+ }
+ }
+ }
+ reader.close();
+ } finally {
+ Files.delete(path);
+ }
+ if (blocked) {
+ assertNull(content);
+ } else {
+ assertEquals(text, content);
+ }
+ }
+}
diff --git
a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/reader/XMLMetadata.java
b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/reader/XMLMetadata.java
index d4c8376956..8abcbb0e7b 100644
---
a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/reader/XMLMetadata.java
+++
b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/reader/XMLMetadata.java
@@ -30,7 +30,6 @@ import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.time.Instant;
import javax.xml.stream.XMLEventReader;
-import javax.xml.stream.XMLInputFactory;
import javax.xml.stream.XMLStreamException;
import javax.xml.stream.events.XMLEvent;
import javax.xml.stream.events.Attribute;
@@ -49,6 +48,7 @@ import org.apache.sis.util.collection.DefaultTreeTable;
import org.apache.sis.util.collection.TableColumn;
import org.apache.sis.util.resources.Errors;
import org.apache.sis.xml.XML;
+import org.apache.sis.xml.internal.shared.InputFactory;
/**
@@ -210,11 +210,10 @@ public final class XMLMetadata implements Filter {
* Returns a reader for the XML document, or {@code null} if the document
could not be read.
*/
private XMLEventReader toXML() throws XMLStreamException {
- final XMLInputFactory factory = XMLInputFactory.newFactory();
if (bytes != null) {
- return factory.createXMLEventReader(new
ByteArrayInputStream(bytes), "UTF-8");
+ return InputFactory.createXMLEventReader(new
ByteArrayInputStream(bytes));
} else if (string != null) {
- return factory.createXMLEventReader(new StringReader(string));
+ return InputFactory.createXMLEventReader(new StringReader(string));
} else {
return null;
}
diff --git
a/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/gpx/WritableStore.java
b/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/gpx/WritableStore.java
index 71a4317fef..8b182d90be 100644
---
a/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/gpx/WritableStore.java
+++
b/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/gpx/WritableStore.java
@@ -156,7 +156,7 @@ public final class WritableStore extends Store implements
WritableFeatureSet {
/*
* Get the writer if no read or other write operation is in
progress, then write the data.
*/
- try (Writer writer = new Writer(this,
org.apache.sis.storage.gpx.Metadata.castOrCopy(metadata, locale), null)) {
+ try (Writer writer = new Writer(this,
org.apache.sis.storage.gpx.Metadata.castOrCopy(metadata, dataLocale), null)) {
writer.writeStartDocument();
if (features != null) {
features.forEachOrdered(writer);
diff --git
a/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/xml/stream/InputType.java
b/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/xml/stream/InputType.java
index 82acfc8ddc..0f7746eb14 100644
---
a/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/xml/stream/InputType.java
+++
b/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/xml/stream/InputType.java
@@ -18,6 +18,7 @@ package org.apache.sis.storage.xml.stream;
import java.io.Reader;
import java.io.InputStream;
+import java.nio.charset.Charset;
import javax.xml.stream.XMLEventReader;
import javax.xml.stream.XMLInputFactory;
import javax.xml.stream.XMLStreamReader;
@@ -60,8 +61,9 @@ enum InputType {
STREAM(InputStream.class) {
@Override XMLStreamReader create(StaxDataStore ds, Object s) throws
XMLStreamException {
final XMLInputFactory f = ds.inputFactory();
- return (ds.encoding != null) ?
f.createXMLStreamReader((InputStream) s, ds.encoding.name())
- :
f.createXMLStreamReader((InputStream) s);
+ final Charset encoding = ds.getEncoding();
+ return (encoding != null) ? f.createXMLStreamReader((InputStream)
s, encoding.name())
+ : f.createXMLStreamReader((InputStream)
s);
}
},
diff --git
a/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/xml/stream/OutputType.java
b/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/xml/stream/OutputType.java
index 33247753b0..511027cee3 100644
---
a/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/xml/stream/OutputType.java
+++
b/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/xml/stream/OutputType.java
@@ -23,6 +23,7 @@ import java.io.OutputStream;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.Closeable;
+import java.nio.charset.Charset;
import javax.xml.stream.XMLEventWriter;
import javax.xml.stream.XMLOutputFactory;
import javax.xml.stream.XMLStreamWriter;
@@ -61,8 +62,9 @@ enum OutputType {
STREAM(OutputStream.class, InputType.STREAM) {
@Override XMLStreamWriter create(StaxDataStore ds, Object s) throws
XMLStreamException {
final XMLOutputFactory f = ds.outputFactory();
- return (ds.encoding != null) ?
f.createXMLStreamWriter((OutputStream) s, ds.encoding.name())
- :
f.createXMLStreamWriter((OutputStream) s);
+ final Charset encoding = ds.getEncoding();
+ return (encoding != null) ? f.createXMLStreamWriter((OutputStream)
s, encoding.name())
+ : f.createXMLStreamWriter((OutputStream)
s);
}
@Override Closeable snapshot(final Object s) {
if (s instanceof ByteArrayOutputStream) {
diff --git
a/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/xml/stream/RewriteOnUpdate.java
b/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/xml/stream/RewriteOnUpdate.java
index 1a1839bb4e..7885bbaa6c 100644
---
a/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/xml/stream/RewriteOnUpdate.java
+++
b/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/xml/stream/RewriteOnUpdate.java
@@ -98,7 +98,7 @@ public abstract class RewriteOnUpdate implements
AutoCloseable {
* @return the data locale, or {@code null}.
*/
protected final Locale getLocale() {
- return (source instanceof StaxDataStore) ? ((StaxDataStore)
source).locale : null;
+ return (source instanceof StaxDataStore) ? ((StaxDataStore)
source).getDataLocale() : null;
}
/**
diff --git
a/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/xml/stream/StaxDataStore.java
b/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/xml/stream/StaxDataStore.java
index 458f23c2aa..2c72f7f75a 100644
---
a/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/xml/stream/StaxDataStore.java
+++
b/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/xml/stream/StaxDataStore.java
@@ -20,7 +20,6 @@ import java.util.Locale;
import java.util.logging.Level;
import java.util.logging.LogRecord;
import java.util.logging.Filter;
-import java.time.ZoneId;
import java.io.Closeable;
import java.io.IOException;
import java.io.InputStream;
@@ -28,6 +27,7 @@ import java.io.OutputStream;
import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
import java.nio.charset.Charset;
+import javax.xml.XMLConstants;
import javax.xml.stream.Location;
import javax.xml.stream.XMLReporter;
import javax.xml.stream.XMLInputFactory;
@@ -63,28 +63,6 @@ import org.apache.sis.io.wkt.WKTFormat;
* @author Martin Desruisseaux (Geomatys)
*/
public abstract class StaxDataStore extends URIDataStore {
- /**
- * The locale to use for locale-sensitive data (<strong>not</strong> for
logging or warning messages),
- * or {@code null} if unspecified.
- *
- * @see OptionKey#LOCALE
- */
- protected final Locale locale;
-
- /**
- * The timezone to use when parsing or formatting dates and times without
explicit timezone,
- * or {@code null} if unspecified.
- *
- * @see OptionKey#TIMEZONE
- */
- protected final ZoneId timezone;
-
- /**
- * The character encoding of the file content, or {@code null} if
unspecified.
- * This is often (but not always) ignored at reading time, but taken in
account at writing time.
- */
- protected final Charset encoding;
-
/**
* Configuration information for JAXB (un)marshaller (actually the SIS
wrappers) or for the StAX factories.
* This object is a read-only map which may contain the following entries:
@@ -201,9 +179,6 @@ public abstract class StaxDataStore extends URIDataStore {
super(provider, connector);
final Integer indent;
storage = connector.getStorage();
- locale = connector.getOption(OptionKey.LOCALE);
- timezone = connector.getOption(OptionKey.TIMEZONE);
- encoding = connector.getOption(OptionKey.ENCODING);
indent = connector.getOption(OptionKey.INDENTATION);
indentation = (indent == null) ? Constants.DEFAULT_INDENTATION
: (byte)
Math.max(WKTFormat.SINGLE_LINE, Math.min(120, indent));
@@ -298,7 +273,7 @@ public abstract class StaxDataStore extends URIDataStore {
public Object get(final Object key) {
if (key instanceof String) {
switch ((String) key) {
- case XML.LOCALE: return locale;
+ case XML.LOCALE: return getDataLocale();
case XML.TIMEZONE: return timezone;
case XML.WARNING_FILTER: return this;
}
@@ -325,7 +300,7 @@ public abstract class StaxDataStore extends URIDataStore {
*/
@Override
public void report(String message, String errorType, Object info,
Location location) {
- final LogRecord record = new LogRecord(Level.WARNING, message);
+ final var record = new LogRecord(Level.WARNING, message);
record.setSourceClassName(StaxDataStore.this.getClass().getCanonicalName());
// record.setLoggerName(…) will be invoked by `listeners` with
inferred name.
listeners.warning(record);
@@ -348,7 +323,7 @@ public abstract class StaxDataStore extends URIDataStore {
*/
@Override
public String toString() {
- return Strings.toString(getClass(), "locale", locale, "timezone",
timezone);
+ return Strings.toString(getClass(), "dataLocale", dataLocale,
"timezone", timezone);
}
}
@@ -374,20 +349,53 @@ public abstract class StaxDataStore extends URIDataStore {
return provider.getShortName();
}
+ /**
+ * Returns the character encoding of the file content, or {@code null} if
unspecified.
+ * This is often (but not always) ignored at reading time, but taken in
account at writing time.
+ */
+ final Charset getEncoding() {
+ return encoding;
+ }
+
+ /**
+ * Returns the locale to use for locale-sensitive data
(<strong>not</strong> for logging or warning messages),
+ * or {@code null} if unspecified.
+ *
+ * @see OptionKey#LOCALE
+ */
+ final Locale getDataLocale() {
+ return dataLocale;
+ }
+
/**
* Returns the factory for StAX readers. The same instance is returned for
all {@code StaxDataStore} lifetime.
* Warnings emitted by readers created by this factory will be forwarded
to the {@link #listeners}.
*
* <p>This method is indirectly invoked by {@link
#createReader(StaxStreamReader)},
* through a call to {@link InputType#create(StaxDataStore, Object)}.</p>
+ *
+ * <h4>Security</h4>
+ * Unless the user has configured the {@code javax.xml.accessExternalDTD}
property to something else
+ * than {@code "all"}, this class disallows external DTDs referenced by
the {@code "file"} protocols.
+ * Allowed protocols are <abbr>HTTP</abbr> and <abbr>HTTPS</abbr> (list
may be expanded if needed).
+ *
+ * @see org.apache.sis.xml.internal.shared.InputFactory#FACTORY
*/
final XMLInputFactory inputFactory() {
assert Thread.holdsLock(this);
- if (inputFactory == null) {
- inputFactory = XMLInputFactory.newInstance();
- inputFactory.setXMLReporter(configuration);
+ XMLInputFactory factory = inputFactory;
+ if (factory == null) {
+ factory = XMLInputFactory.newInstance();
+ if
(factory.isPropertySupported(XMLConstants.FEATURE_SECURE_PROCESSING)) {
+ factory.setProperty(XMLConstants.FEATURE_SECURE_PROCESSING,
Boolean.TRUE);
+ }
+ if
("all".equals(factory.getProperty(XMLConstants.ACCESS_EXTERNAL_DTD))) {
+ factory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD,
"http,https");
+ }
+ factory.setXMLReporter(configuration);
+ inputFactory = factory; // Set only on success.
}
- return inputFactory;
+ return factory;
}
/**
diff --git
a/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/xml/stream/StaxStreamWriter.java
b/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/xml/stream/StaxStreamWriter.java
index b83bac8828..53e21e4d77 100644
---
a/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/xml/stream/StaxStreamWriter.java
+++
b/endorsed/src/org.apache.sis.storage.xml/main/org/apache/sis/storage/xml/stream/StaxStreamWriter.java
@@ -142,7 +142,7 @@ public abstract class StaxStreamWriter extends StaxStreamIO
implements Consumer<
* {@link DataStoreException}, {@link ClassCastException},
<i>etc.</i>
*/
public void writeStartDocument() throws Exception {
- final Charset encoding = owner.encoding;
+ final Charset encoding = owner.getEncoding();
if (encoding != null) {
writer.writeStartDocument(encoding.name(), null);
} else {
@@ -294,7 +294,7 @@ public abstract class StaxStreamWriter extends StaxStreamIO
implements Consumer<
m = getMarshallerPool().acquireMarshaller();
m.setProperty(Marshaller.JAXB_FRAGMENT, Boolean.TRUE);
m.setProperty(Marshaller.JAXB_FORMATTED_OUTPUT, Boolean.FALSE);
// Formatting will be done by FormattedWriter.
- final Charset encoding = owner.encoding;
+ final Charset encoding = owner.getEncoding();
if (encoding != null) {
m.setProperty(Marshaller.JAXB_ENCODING, encoding.name());
}
diff --git
a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/base/URIDataStore.java
b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/base/URIDataStore.java
index 0870fff38a..e57ab44988 100644
---
a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/base/URIDataStore.java
+++
b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/base/URIDataStore.java
@@ -16,7 +16,6 @@
*/
package org.apache.sis.storage.base;
-import java.util.Map;
import java.util.HashMap;
import java.util.Arrays;
import java.util.Optional;
@@ -28,12 +27,14 @@ import java.io.BufferedWriter;
import java.io.BufferedInputStream;
import java.io.InputStream;
import java.io.IOException;
+import java.io.FileNotFoundException;
import java.net.URL;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.file.Path;
import java.nio.file.Files;
import java.nio.file.StandardOpenOption;
+import java.nio.file.NoSuchFileException;
import java.nio.charset.Charset;
import javax.xml.transform.Source;
import jakarta.xml.bind.JAXBException;
@@ -413,7 +414,7 @@ public abstract class URIDataStore extends DataStore
implements StoreResource {
return false;
};
// Cannot use Map.of(…) because it does not accept null values.
- Map<String,Object> properties = new HashMap<>(8);
+ final var properties = new HashMap<String, Object>(8);
properties.put(XML.LOCALE, dataLocale);
properties.put(XML.TIMEZONE, timezone);
properties.put(XML.WARNING_FILTER, handler);
