This is an automated email from the ASF dual-hosted git repository. ppkarwasz pushed a commit to branch feat/xinclude in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 16a75566bf95f6ca0389445571942f1e67b5ce7d Author: Piotr P. Karwasz <[email protected]> AuthorDate: Thu Jun 11 10:50:20 2026 +0200 Make XInclude opt-in and rework XML configuration hardening (#4064) XInclude support in XML configurations is now optional and disabled by default. It can be enabled with the `log4j2.configurationEnableXInclude` property, and failures of `setXIncludeAware` are no longer ignored since the user opted in for it. Fixes #4064. The XML factory hardening code is replaced with `copernik-xml-factory`, an incubating project for Apache Commons (or Xerces). The security features set on the factories are now tailored to the factory type and no longer swallowed, with Android support out-of-the-box. External entities and XInclude resources are resolved through a custom resolver based on `ConfigurationSource.fromUri`, so resolution now uses Log4j features (such as `classpath:` support and conventions on relative URIs) and honors the configured protocol restrictions. The `XmlConfigurationSecurity` test is renamed to `XmlConfigurationSecurityTest` so that Surefire runs it again; it was previously skipped because its name did not end in `Test`. Assisted-By: Claude Opus 4.8 (1M context) <[email protected]> --- .../core/config/ConfigurationFactoryTest.java | 2 + .../core/config/xml/XmlConfigurationSecurity.java | 38 ------- .../config/xml/XmlConfigurationSecurityTest.java | 81 ++++++++++++++ .../config/xml/XmlConfigurationXIncludeTest.java | 78 +++++++++++++ .../external-parameter-entity.xml} | 0 .../xinclude.xml} | 9 +- ...onSecurity.xml => log4j-xinclude-classpath.xml} | 23 ++-- log4j-core/pom.xml | 5 + .../log4j/core/config/xml/XmlConfiguration.java | 122 ++++++++------------- .../log4j/osgi/tests/AbstractLoadBundleTest.java | 36 +++--- .../logging/log4j/osgi/tests/CoreOsgiTest.java | 1 + .../logging/log4j/osgi/tests/DisruptorTest.java | 1 + log4j-parent/pom.xml | 7 ++ src/changelog/.2.x.x/4064_xinclude_opt_in.xml | 12 ++ src/changelog/.2.x.x/4064_xinclude_resolver.xml | 11 ++ 15 files changed, 279 insertions(+), 147 deletions(-) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryTest.java index ee19ea7ea8..d6fa0d34ee 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryTest.java @@ -42,6 +42,7 @@ import org.apache.logging.log4j.core.appender.ConsoleAppender; import org.apache.logging.log4j.core.config.composite.CompositeConfiguration; import org.apache.logging.log4j.core.filter.ThreadContextMapFilter; import org.apache.logging.log4j.core.test.junit.LoggerContextSource; +import org.apache.logging.log4j.test.junit.SetTestProperty; import org.apache.logging.log4j.test.junit.TempLoggingDir; import org.apache.logging.log4j.util.Strings; import org.junit.jupiter.api.Tag; @@ -103,6 +104,7 @@ class ConfigurationFactoryTest { } @Test + @SetTestProperty(key = "log4j2.configurationEnableXInclude", value = "true") @LoggerContextSource("log4j-xinclude.xml") void xinclude(final LoggerContext context) throws IOException { checkConfiguration(context); diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationSecurity.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationSecurity.java deleted file mode 100644 index 64aa3aa4d6..0000000000 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationSecurity.java +++ /dev/null @@ -1,38 +0,0 @@ -/* - * 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.logging.log4j.core.config.xml; - -import static org.junit.jupiter.api.Assertions.assertNotNull; - -import org.apache.logging.log4j.core.LoggerContext; -import org.apache.logging.log4j.core.config.Configurator; -import org.junit.jupiter.api.Tag; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.Timeout; - -@Tag("functional") -@Tag("security") -class XmlConfigurationSecurity { - - @Test - @Timeout(5) - void xmlSecurity() { - final LoggerContext context = - Configurator.initialize("XmlConfigurationSecurity", "XmlConfigurationSecurity.xml"); - assertNotNull(context.getConfiguration().getAppender("list")); - } -} diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationSecurityTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationSecurityTest.java new file mode 100644 index 0000000000..f9ae9e37bf --- /dev/null +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationSecurityTest.java @@ -0,0 +1,81 @@ +/* + * 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.logging.log4j.core.config.xml; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.net.URI; +import org.apache.logging.log4j.core.config.Configuration; +import org.apache.logging.log4j.core.config.ConfigurationException; +import org.apache.logging.log4j.core.config.ConfigurationFactory; +import org.apache.logging.log4j.test.junit.SetTestProperty; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +/** + * Verifies the contract of {@link XmlConfiguration} regarding the resolution of external resources. + * + * <p>Configuration files are part of the trusted computing base (see the + * <a href="https://logging.apache.org/security.html";>Log4j security policy</a>); the hardenings checked here are + * defense in depth, not a security boundary against a malicious configuration. These tests only cover what Log4j is + * responsible for:</p> + * <ul> + * <li>a forbidden external fetch is surfaced as a {@link ConfigurationException} (rather than a half-built + * configuration), and</li> + * <li>XInclude resources are subject to the same {@code ALLOWED_PROTOCOLS} restrictions as the configuration + * file itself.</li> + * </ul> + * + * <p>All external fetches are forbidden; whether a forbidden fetch raises an error or is silently skipped is part of + * the contract of the {@code eu.copernik:copernik-xml-factory} library and is tested there.</p> + */ +@Tag("functional") +@Tag("security") +class XmlConfigurationSecurityTest { + + private static final ConfigurationFactory FACTORY = new XmlConfigurationFactory(); + + private static Configuration getConfiguration(final String resource) { + return FACTORY.getConfiguration(null, null, URI.create("classpath:XmlConfigurationSecurity/" + resource)); + } + + /** + * A configuration that triggers a forbidden external fetch fails to load with a {@link ConfigurationException}. + * The {@code @Timeout} guards against an SSRF: a fetch attempt against the unreachable host would block until the + * connection times out. + */ + @Test + @Timeout(5) + void forbiddenExternalFetchThrowsConfigurationException() { + assertThatThrownBy(() -> getConfiguration("external-parameter-entity.xml")) + .isInstanceOf(ConfigurationException.class); + } + + /** + * XInclude resources are resolved through {@code ConfigurationSource}, so they are subject to the same + * {@code ALLOWED_PROTOCOLS} restrictions as the configuration file. With {@code http} excluded, an {@code http} + * include cannot be resolved and the configuration fails to load. + */ + @Test + @Timeout(5) + @SetTestProperty(key = "log4j2.configurationEnableXInclude", value = "true") + @SetTestProperty(key = "log4j2.configurationAllowedProtocols", value = "file") + void xIncludeRespectsAllowedProtocols() { + assertThatThrownBy(() -> getConfiguration("xinclude.xml")).isInstanceOf(ConfigurationException.class); + } +} diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationXIncludeTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationXIncludeTest.java new file mode 100644 index 0000000000..b0c2d1c00e --- /dev/null +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationXIncludeTest.java @@ -0,0 +1,78 @@ +/* + * 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.logging.log4j.core.config.xml; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.net.URL; +import javax.xml.parsers.DocumentBuilder; +import org.junit.jupiter.api.Test; +import org.w3c.dom.Document; +import org.xml.sax.InputSource; + +/** + * Unit tests for the XInclude handling of {@link XmlConfiguration#newDocumentBuilder(boolean)}. + * <p> + * These tests exercise the {@link DocumentBuilder} directly, so they are independent of the + * {@code log4j2.configurationEnableXInclude} property plumbing covered by the integration tests in + * {@code ConfigurationFactoryTest}. + * </p> + */ +class XmlConfigurationXIncludeTest { + + private Document parse(final boolean enableXInclude) throws Exception { + return parse("/log4j-xinclude.xml", enableXInclude); + } + + private Document parse(final String resource, final boolean enableXInclude) throws Exception { + final URL url = getClass().getResource(resource); + final DocumentBuilder builder = XmlConfiguration.newDocumentBuilder(enableXInclude); + final InputSource source = new InputSource(url.openStream()); + // Required so that relative `href` attributes can be resolved against the configuration location. + source.setSystemId(url.toExternalForm()); + return builder.parse(source); + } + + @Test + void xInclude_enabled_resolves_includes() throws Exception { + final Document document = parse(true); + // The `xi:include` elements have been replaced by the content of the included files. + assertEquals(0, document.getElementsByTagNameNS("*", "include").getLength(), "no `xi:include` should remain"); + assertEquals(1, document.getElementsByTagName("Console").getLength(), "Console appender from include"); + assertEquals(1, document.getElementsByTagName("File").getLength(), "File appender from include"); + assertEquals(1, document.getElementsByTagName("List").getLength(), "List appender from include"); + assertEquals(2, document.getElementsByTagName("Logger").getLength(), "Loggers from include"); + } + + @Test + void xInclude_disabled_keeps_includes_unresolved() throws Exception { + final Document document = parse(false); + // Without XInclude support the `xi:include` elements are left untouched and nothing is included. + assertEquals(2, document.getElementsByTagNameNS("*", "include").getLength(), "`xi:include` elements remain"); + assertEquals(0, document.getElementsByTagName("Console").getLength(), "no appenders should be included"); + } + + @Test + void xInclude_resolves_classpath_scheme() throws Exception { + // The custom resolver delegates to `ConfigurationSource`, which understands the `classpath:` URI scheme. + final Document document = parse("/log4j-xinclude-classpath.xml", true); + assertEquals(0, document.getElementsByTagNameNS("*", "include").getLength(), "no `xi:include` should remain"); + assertEquals( + 1, document.getElementsByTagName("Console").getLength(), "Console appender from classpath include"); + assertEquals(2, document.getElementsByTagName("Logger").getLength(), "Loggers from classpath include"); + } +} diff --git a/log4j-core-test/src/test/resources/XmlConfigurationSecurity.xml b/log4j-core-test/src/test/resources/XmlConfigurationSecurity/external-parameter-entity.xml similarity index 100% copy from log4j-core-test/src/test/resources/XmlConfigurationSecurity.xml copy to log4j-core-test/src/test/resources/XmlConfigurationSecurity/external-parameter-entity.xml diff --git a/log4j-core-test/src/test/resources/XmlConfigurationSecurity.xml b/log4j-core-test/src/test/resources/XmlConfigurationSecurity/xinclude.xml similarity index 82% copy from log4j-core-test/src/test/resources/XmlConfigurationSecurity.xml copy to log4j-core-test/src/test/resources/XmlConfigurationSecurity/xinclude.xml index 4a648ad5e5..98b2b76128 100644 --- a/log4j-core-test/src/test/resources/XmlConfigurationSecurity.xml +++ b/log4j-core-test/src/test/resources/XmlConfigurationSecurity/xinclude.xml @@ -15,16 +15,15 @@ ~ See the License for the specific language governing permissions and ~ limitations under the License. --> -<!DOCTYPE Configuration [ - <!ENTITY % dtd SYSTEM "http://localhost:12345/evil1.dtd";> -%dtd;]> -<Configuration status="OFF" name="XmlConfigurationSecurity"> +<Configuration xmlns:xi="http://www.w3.org/2001/XInclude"; + status="OFF" name="XmlConfigurationSecurity"> <Appenders> <List name="list"> <PatternLayout pattern="%m%n"/> </List> </Appenders> - + <!-- An `http` resource that `ALLOWED_PROTOCOLS` must reject. --> + <xi:include href="http://localhost:12345/evil3.xml"/> <Loggers> <Root level="info"> <AppenderRef ref="list"/> diff --git a/log4j-core-test/src/test/resources/XmlConfigurationSecurity.xml b/log4j-core-test/src/test/resources/log4j-xinclude-classpath.xml similarity index 70% rename from log4j-core-test/src/test/resources/XmlConfigurationSecurity.xml rename to log4j-core-test/src/test/resources/log4j-xinclude-classpath.xml index 4a648ad5e5..e9e9802fc9 100644 --- a/log4j-core-test/src/test/resources/XmlConfigurationSecurity.xml +++ b/log4j-core-test/src/test/resources/log4j-xinclude-classpath.xml @@ -15,19 +15,12 @@ ~ See the License for the specific language governing permissions and ~ limitations under the License. --> -<!DOCTYPE Configuration [ - <!ENTITY % dtd SYSTEM "http://localhost:12345/evil1.dtd";> -%dtd;]> -<Configuration status="OFF" name="XmlConfigurationSecurity"> - <Appenders> - <List name="list"> - <PatternLayout pattern="%m%n"/> - </List> - </Appenders> - - <Loggers> - <Root level="info"> - <AppenderRef ref="list"/> - </Root> - </Loggers> +<Configuration xmlns:xi="http://www.w3.org/2001/XInclude"; + status="OFF" name="XMLConfigTest"> + <Properties> + <Property name="filename">${test:logging.path}/test-xinclude.log</Property> + </Properties> + <ThresholdFilter level="debug"/> + <xi:include href="classpath:log4j-xinclude-appenders.xml" /> + <xi:include href="classpath:log4j-xinclude-loggers.xml" /> </Configuration> diff --git a/log4j-core/pom.xml b/log4j-core/pom.xml index b2f34d4ab9..4edf0860c4 100644 --- a/log4j-core/pom.xml +++ b/log4j-core/pom.xml @@ -157,6 +157,11 @@ <artifactId>commons-csv</artifactId> <optional>true</optional> </dependency> + <!-- Create hardened JAXP factories --> + <dependency> + <groupId>eu.copernik</groupId> + <artifactId>copernik-xml-factory</artifactId> + </dependency> <!-- Alternative implementation of BlockingQueue using Conversant Disruptor for AsyncAppender --> <dependency> <groupId>com.conversantmedia</groupId> diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/xml/XmlConfiguration.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/xml/XmlConfiguration.java index 3218589bb1..56f62ee58a 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/xml/XmlConfiguration.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/xml/XmlConfiguration.java @@ -16,9 +16,12 @@ */ package org.apache.logging.log4j.core.config.xml; +import eu.copernik.xml.factory.XmlFactories; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; +import java.net.URI; +import java.net.URISyntaxException; import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; @@ -35,6 +38,7 @@ import javax.xml.validation.Validator; import org.apache.logging.log4j.core.LoggerContext; import org.apache.logging.log4j.core.config.AbstractConfiguration; import org.apache.logging.log4j.core.config.Configuration; +import org.apache.logging.log4j.core.config.ConfigurationException; import org.apache.logging.log4j.core.config.ConfigurationSource; import org.apache.logging.log4j.core.config.Node; import org.apache.logging.log4j.core.config.Reconfigurable; @@ -45,13 +49,14 @@ import org.apache.logging.log4j.core.util.Closer; import org.apache.logging.log4j.core.util.Integers; import org.apache.logging.log4j.core.util.Loader; import org.apache.logging.log4j.core.util.Patterns; -import org.apache.logging.log4j.core.util.Throwables; +import org.apache.logging.log4j.util.PropertiesUtil; import org.w3c.dom.Attr; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.NamedNodeMap; import org.w3c.dom.NodeList; import org.w3c.dom.Text; +import org.xml.sax.EntityResolver; import org.xml.sax.InputSource; import org.xml.sax.SAXException; @@ -60,8 +65,7 @@ import org.xml.sax.SAXException; */ public class XmlConfiguration extends AbstractConfiguration implements Reconfigurable { - private static final String XINCLUDE_FIXUP_LANGUAGE = "http://apache.org/xml/features/xinclude/fixup-language";; - private static final String XINCLUDE_FIXUP_BASE_URIS = "http://apache.org/xml/features/xinclude/fixup-base-uris";; + private static final String ENABLE_XINCLUDE_PROP = "log4j2.configurationEnableXInclude"; private final List<Status> status = new ArrayList<>(); private Element rootElement; @@ -84,24 +88,9 @@ public class XmlConfiguration extends AbstractConfiguration implements Reconfigu } final InputSource source = new InputSource(new ByteArrayInputStream(buffer)); source.setSystemId(configSource.getLocation()); - final DocumentBuilder documentBuilder = newDocumentBuilder(true); - Document document; - try { - document = documentBuilder.parse(source); - } catch (final Exception e) { - // LOG4J2-1127 - final Throwable throwable = Throwables.getRootCause(e); - if (throwable instanceof UnsupportedOperationException) { - LOGGER.warn( - "The DocumentBuilder {} does not support an operation: {}." - + "Trying again without XInclude...", - documentBuilder, - e); - document = newDocumentBuilder(false).parse(source); - } else { - throw e; - } - } + final boolean xIncludeAware = PropertiesUtil.getProperties().getBooleanProperty(ENABLE_XINCLUDE_PROP); + final DocumentBuilder documentBuilder = newDocumentBuilder(xIncludeAware); + final Document document = documentBuilder.parse(source); rootElement = document.getDocumentElement(); final Map<String, String> attrs = processAttributes(rootNode, rootElement); final StatusConfiguration statusConfig = new StatusConfiguration().withStatus(getDefaultStatus()); @@ -135,6 +124,7 @@ public class XmlConfiguration extends AbstractConfiguration implements Reconfigu statusConfig.initialize(); } catch (final SAXException | IOException | ParserConfigurationException e) { LOGGER.error("Error parsing " + configSource.getLocation(), e); + throw new ConfigurationException("Error parsing " + configSource.getLocation(), e); } if (strict && schemaResource != null && buffer != null) { try (final InputStream is = @@ -172,67 +162,22 @@ public class XmlConfiguration extends AbstractConfiguration implements Reconfigu /** * Creates a new DocumentBuilder suitable for parsing a configuration file. * - * @param xIncludeAware enabled XInclude + * @param enableXInclude enabled XInclude * @return a new DocumentBuilder * @throws ParserConfigurationException if a DocumentBuilder cannot be created, which satisfies the configuration requested. */ - static DocumentBuilder newDocumentBuilder(final boolean xIncludeAware) throws ParserConfigurationException { - final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + static DocumentBuilder newDocumentBuilder(final boolean enableXInclude) throws ParserConfigurationException { + final DocumentBuilderFactory factory = XmlFactories.newDocumentBuilderFactory(); factory.setNamespaceAware(true); - - disableDtdProcessing(factory); - - if (xIncludeAware) { - enableXInclude(factory); - } - return factory.newDocumentBuilder(); - } - - private static void disableDtdProcessing(final DocumentBuilderFactory factory) { - factory.setValidating(false); - factory.setExpandEntityReferences(false); - setFeature(factory, "http://xml.org/sax/features/external-general-entities";, false); - setFeature(factory, "http://xml.org/sax/features/external-parameter-entities";, false); - setFeature(factory, "http://apache.org/xml/features/nonvalidating/load-external-dtd";, false); - } - - private static void setFeature( - final DocumentBuilderFactory factory, final String featureName, final boolean value) { - try { - factory.setFeature(featureName, value); - } catch (final ParserConfigurationException e) { - LOGGER.warn( - "The DocumentBuilderFactory [{}] does not support the feature [{}]: {}", factory, featureName, e); - } catch (final AbstractMethodError err) { - LOGGER.warn( - "The DocumentBuilderFactory [{}] is out of date and does not support setFeature: {}", factory, err); + if (enableXInclude) { + factory.setXIncludeAware(true); } - } - /** - * Enables XInclude for the given DocumentBuilderFactory - * - * @param factory a DocumentBuilderFactory - */ - private static void enableXInclude(final DocumentBuilderFactory factory) { - try { - factory.setXIncludeAware(true); - // LOG4J2-3531: Xerces only checks if the feature is supported when creating a factory. To reproduce: - // -Dorg.apache.xerces.xni.parser.XMLParserConfiguration=org.apache.xerces.parsers.XML11NonValidatingConfiguration - try { - factory.newDocumentBuilder(); - } catch (final ParserConfigurationException e) { - factory.setXIncludeAware(false); - LOGGER.warn("The DocumentBuilderFactory [{}] does not support XInclude: {}", factory, e); - } - } catch (final UnsupportedOperationException e) { - LOGGER.warn("The DocumentBuilderFactory [{}] does not support XInclude: {}", factory, e); - } catch (final AbstractMethodError | NoSuchMethodError err) { - LOGGER.warn( - "The DocumentBuilderFactory [{}] is out of date and does not support XInclude: {}", factory, err); + final DocumentBuilder builder = factory.newDocumentBuilder(); + if (enableXInclude) { + builder.setEntityResolver(ConfigurationSourceEntityResolver.INSTANCE); } - setFeature(factory, XINCLUDE_FIXUP_BASE_URIS, true); - setFeature(factory, XINCLUDE_FIXUP_LANGUAGE, true); + return builder; } @Override @@ -368,4 +313,31 @@ public class XmlConfiguration extends AbstractConfiguration implements Reconfigu return "Status [name=" + name + ", element=" + element + ", errorType=" + errorType + "]"; } } + + /** + * Entity resolver that resolves external entities the same way the configuration itself is resolved. + */ + private static class ConfigurationSourceEntityResolver implements EntityResolver { + + private static final EntityResolver INSTANCE = new ConfigurationSourceEntityResolver(); + + @Override + public InputSource resolveEntity(final String publicId, final String systemId) throws SAXException { + InputSource source = null; + try { + final ConfigurationSource configurationSource = ConfigurationSource.fromUri(new URI(systemId)); + if (configurationSource != null) { + source = new InputSource(configurationSource.getInputStream()); + source.setPublicId(publicId); + source.setSystemId(systemId); + } + } catch (final URISyntaxException e) { + throw new SAXException("Unable to resolve system id " + systemId, e); + } + if (source == null) { + throw new SAXException("Unable to resolve entity: " + systemId); + } + return source; + } + } } diff --git a/log4j-osgi-test/src/test/java/org/apache/logging/log4j/osgi/tests/AbstractLoadBundleTest.java b/log4j-osgi-test/src/test/java/org/apache/logging/log4j/osgi/tests/AbstractLoadBundleTest.java index 06bb63611d..9d370ccf5b 100644 --- a/log4j-osgi-test/src/test/java/org/apache/logging/log4j/osgi/tests/AbstractLoadBundleTest.java +++ b/log4j-osgi-test/src/test/java/org/apache/logging/log4j/osgi/tests/AbstractLoadBundleTest.java @@ -75,6 +75,10 @@ abstract class AbstractLoadBundleTest { return installBundle("org.apache.logging.log4j.api.test"); } + private Bundle getXmlFactoryBundle() throws BundleException { + return installBundle("eu.copernik.xml.factory"); + } + /** * Tests starting, then stopping, then restarting, then stopping, and finally uninstalling the API and Core bundles */ @@ -82,19 +86,20 @@ abstract class AbstractLoadBundleTest { public void testApiCoreStartStopStartStop() throws BundleException { final Bundle api = getApiBundle(); + final Bundle xmlFactory = getXmlFactoryBundle(); final Bundle core = getCoreBundle(); assertEquals(Bundle.INSTALLED, api.getState(), "api is not in INSTALLED state"); assertEquals(Bundle.INSTALLED, core.getState(), "core is not in INSTALLED state"); // 1st start-stop - doOnBundlesAndVerifyState(Bundle::start, Bundle.ACTIVE, api, core); - doOnBundlesAndVerifyState(Bundle::stop, Bundle.RESOLVED, core, api); + doOnBundlesAndVerifyState(Bundle::start, Bundle.ACTIVE, api, xmlFactory, core); + doOnBundlesAndVerifyState(Bundle::stop, Bundle.RESOLVED, core, xmlFactory, api); // 2nd start-stop - doOnBundlesAndVerifyState(Bundle::start, Bundle.ACTIVE, api, core); - doOnBundlesAndVerifyState(Bundle::stop, Bundle.RESOLVED, core, api); + doOnBundlesAndVerifyState(Bundle::start, Bundle.ACTIVE, api, xmlFactory, core); + doOnBundlesAndVerifyState(Bundle::stop, Bundle.RESOLVED, core, xmlFactory, api); - doOnBundlesAndVerifyState(Bundle::uninstall, Bundle.UNINSTALLED, core, api); + doOnBundlesAndVerifyState(Bundle::uninstall, Bundle.UNINSTALLED, core, xmlFactory, api); } /** @@ -104,9 +109,10 @@ abstract class AbstractLoadBundleTest { public void testClassNotFoundErrorLogger() throws BundleException { final Bundle api = getApiBundle(); + final Bundle xmlFactory = getXmlFactoryBundle(); final Bundle core = getCoreBundle(); - doOnBundlesAndVerifyState(Bundle::start, Bundle.ACTIVE, api); + doOnBundlesAndVerifyState(Bundle::start, Bundle.ACTIVE, api, xmlFactory); // fails if LOG4J2-1637 is not fixed try { core.start(); @@ -126,8 +132,8 @@ abstract class AbstractLoadBundleTest { } assertEquals(Bundle.ACTIVE, core.getState(), String.format("`%s` bundle state mismatch", core)); - doOnBundlesAndVerifyState(Bundle::stop, Bundle.RESOLVED, core, api); - doOnBundlesAndVerifyState(Bundle::uninstall, Bundle.UNINSTALLED, core, api); + doOnBundlesAndVerifyState(Bundle::stop, Bundle.RESOLVED, core, xmlFactory, api); + doOnBundlesAndVerifyState(Bundle::uninstall, Bundle.UNINSTALLED, core, xmlFactory, api); } /** @@ -138,10 +144,11 @@ abstract class AbstractLoadBundleTest { public void testLog4J12Fragement() throws BundleException, ReflectiveOperationException { final Bundle api = getApiBundle(); + final Bundle xmlFactory = getXmlFactoryBundle(); final Bundle core = getCoreBundle(); final Bundle compat = get12ApiBundle(); - doOnBundlesAndVerifyState(Bundle::start, Bundle.ACTIVE, api, core); + doOnBundlesAndVerifyState(Bundle::start, Bundle.ACTIVE, api, xmlFactory, core); final Class<?> coreClassFromCore = core.loadClass("org.apache.logging.log4j.core.Core"); final Class<?> levelClassFrom12API = core.loadClass("org.apache.log4j.Level"); @@ -156,8 +163,8 @@ abstract class AbstractLoadBundleTest { levelClassFromAPI.getClassLoader(), "expected 1.2 API Level NOT to have the same class loader as API Level"); - doOnBundlesAndVerifyState(Bundle::stop, Bundle.RESOLVED, core, api); - doOnBundlesAndVerifyState(Bundle::uninstall, Bundle.UNINSTALLED, compat, core, api); + doOnBundlesAndVerifyState(Bundle::stop, Bundle.RESOLVED, core, xmlFactory, api); + doOnBundlesAndVerifyState(Bundle::uninstall, Bundle.UNINSTALLED, compat, core, xmlFactory, api); } /** @@ -166,13 +173,14 @@ abstract class AbstractLoadBundleTest { @Test public void testServiceLoader() throws BundleException, ReflectiveOperationException { final Bundle api = getApiBundle(); + final Bundle xmlFactory = getXmlFactoryBundle(); final Bundle core = getCoreBundle(); final Bundle apiTests = getApiTestsBundle(); final Class<?> osgiServiceLocator = api.loadClass("org.apache.logging.log4j.util.OsgiServiceLocator"); assertTrue((boolean) osgiServiceLocator.getMethod("isAvailable").invoke(null), "OsgiServiceLocator is active"); - doOnBundlesAndVerifyState(Bundle::start, Bundle.ACTIVE, api, core, apiTests); + doOnBundlesAndVerifyState(Bundle::start, Bundle.ACTIVE, api, xmlFactory, core, apiTests); final Class<?> osgiServiceLocatorTest = apiTests.loadClass("org.apache.logging.log4j.test.util.OsgiServiceLocatorTest"); @@ -187,8 +195,8 @@ abstract class AbstractLoadBundleTest { "org.apache.logging.log4j.core.impl.Log4jProvider", services.get(0).getClass().getName()); - doOnBundlesAndVerifyState(Bundle::stop, Bundle.RESOLVED, apiTests, core, api); - doOnBundlesAndVerifyState(Bundle::uninstall, Bundle.UNINSTALLED, apiTests, core, api); + doOnBundlesAndVerifyState(Bundle::stop, Bundle.RESOLVED, apiTests, core, xmlFactory, api); + doOnBundlesAndVerifyState(Bundle::uninstall, Bundle.UNINSTALLED, apiTests, core, xmlFactory, api); } private static void doOnBundlesAndVerifyState( diff --git a/log4j-osgi-test/src/test/java/org/apache/logging/log4j/osgi/tests/CoreOsgiTest.java b/log4j-osgi-test/src/test/java/org/apache/logging/log4j/osgi/tests/CoreOsgiTest.java index 493bb61c9e..77d7f54cdc 100644 --- a/log4j-osgi-test/src/test/java/org/apache/logging/log4j/osgi/tests/CoreOsgiTest.java +++ b/log4j-osgi-test/src/test/java/org/apache/logging/log4j/osgi/tests/CoreOsgiTest.java @@ -46,6 +46,7 @@ public class CoreOsgiTest { public Option[] config() { return options( linkBundle("org.apache.logging.log4j.api"), + linkBundle("eu.copernik.xml.factory"), linkBundle("org.apache.logging.log4j.core"), linkBundle("org.apache.logging.log4j.1.2.api").start(false), // required by Pax Exam's logging diff --git a/log4j-osgi-test/src/test/java/org/apache/logging/log4j/osgi/tests/DisruptorTest.java b/log4j-osgi-test/src/test/java/org/apache/logging/log4j/osgi/tests/DisruptorTest.java index 813331210c..018edca9f5 100644 --- a/log4j-osgi-test/src/test/java/org/apache/logging/log4j/osgi/tests/DisruptorTest.java +++ b/log4j-osgi-test/src/test/java/org/apache/logging/log4j/osgi/tests/DisruptorTest.java @@ -52,6 +52,7 @@ public class DisruptorTest { public Option[] config() { return options( linkBundle("org.apache.logging.log4j.api"), + linkBundle("eu.copernik.xml.factory"), linkBundle("org.apache.logging.log4j.core"), linkBundle("com.lmax.disruptor"), // required by Pax Exam's logging diff --git a/log4j-parent/pom.xml b/log4j-parent/pom.xml index 9a7b5aa2c8..869fab6c90 100644 --- a/log4j-parent/pom.xml +++ b/log4j-parent/pom.xml @@ -78,6 +78,7 @@ <commons-logging.version>1.3.5</commons-logging.version> <!-- `com.conversantmedia:disruptor` version 1.2.16 requires Java 9: --> <conversant.disruptor.version>1.2.15</conversant.disruptor.version> + <copernik-xml-factory.version>0.1.1</copernik-xml-factory.version> <disruptor.version>3.4.4</disruptor.version> <embedded-ldap.version>0.9.0</embedded-ldap.version> <error-prone-annotations.version>2.38.0</error-prone-annotations.version> @@ -392,6 +393,12 @@ <version>${commons-pool2.version}</version> </dependency> + <dependency> + <groupId>eu.copernik</groupId> + <artifactId>copernik-xml-factory</artifactId> + <version>${copernik-xml-factory.version}</version> + </dependency> + <dependency> <groupId>com.conversantmedia</groupId> <artifactId>disruptor</artifactId> diff --git a/src/changelog/.2.x.x/4064_xinclude_opt_in.xml b/src/changelog/.2.x.x/4064_xinclude_opt_in.xml new file mode 100644 index 0000000000..24a23488c6 --- /dev/null +++ b/src/changelog/.2.x.x/4064_xinclude_opt_in.xml @@ -0,0 +1,12 @@ +<?xml version="1.0" encoding="UTF-8"?> +<entry xmlns="https://logging.apache.org/xml/ns"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation=" + https://logging.apache.org/xml/ns + https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"; + type="changed"> + <issue id="4064" link="https://github.com/apache/logging-log4j2/issues/4064"/> + <description format="asciidoc"> + XInclude support in XML configurations is now opt-in (disabled by default) and can be enabled with the `log4j2.configurationEnableXInclude` property. + </description> +</entry> diff --git a/src/changelog/.2.x.x/4064_xinclude_resolver.xml b/src/changelog/.2.x.x/4064_xinclude_resolver.xml new file mode 100644 index 0000000000..89ce4d936e --- /dev/null +++ b/src/changelog/.2.x.x/4064_xinclude_resolver.xml @@ -0,0 +1,11 @@ +<?xml version="1.0" encoding="UTF-8"?> +<entry xmlns="https://logging.apache.org/xml/ns"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation=" + https://logging.apache.org/xml/ns + https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"; + type="added"> + <description format="asciidoc"> + XInclude resources are now resolved through `ConfigurationSource`, so they support the Log4j URI conventions. + </description> +</entry>
