From 3dce0bc55328add2083c66f453a890ac57f9304e Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Fri, 26 Jun 2026 13:08:51 +0200 Subject: [PATCH 1/8] Revamp XML configuration testing and documentation Rework the testing and documentation of the XML configuration format so that each behavior of the DOM-to-Node conversion is covered by its own focused test. `constructHierarchy`, `getType` and `processAttributes` are now static and take the `PluginManager` and `strict` flag as parameters, so the conversion can be unit-tested with a mocked `PluginManager` instead of a full `LoggerContext`. The `StrictXmlConfigTest` integration test is replaced by `XmlConfigurationConstructHierarchyTest`, which exercises plugin resolution, attribute folding, text values, strict-mode `type` resolution and XInclude whole-file/positional/by-id fragment resolution individually. The unused `Status`/`ErrorType` bookkeeping is dropped in favor of logging the error directly. The XML format documentation is moved into a reusable AsciiDoc partial and gains XPointer fragment examples. Testing each behavior individually surfaced a bug: the XInclude `fixup-language` feature adds an `xml:lang` attribute that was never stripped (only `xml:base` was) and was not covered by any test, so it leaked through as a Log4j configuration attribute. Matching the reserved XML namespace instead of the literal attribute name now strips both `xml:base` and `xml:lang`, and any other reserved `xml:` attribute. `XmlConfiguration` also stops explicitly enabling the XInclude `fixup-base-uris`/`fixup-language` features: they default to `true` and only add the attributes that Log4j strips. Assisted-By: Claude Opus 4.8 (1M context) --- .../log4j/core/StrictXmlConfigTest.java | 131 -------- ...mlConfigurationConstructHierarchyTest.java | 279 ++++++++++++++++++ .../included-fragment.xml | 21 ++ .../included-with-id.xml | 26 ++ .../included.xml | 18 ++ .../src/test/resources/log4j-strict1.xml | 65 ---- .../core/config/xml/XmlConfiguration.java | 61 +--- .../.2.x.x/xinclude_fixup_attributes.xml | 11 + .../xinclude-xpointer-appenders.xml | 30 ++ .../configuration/xinclude-xpointer-main.xml | 34 +++ .../ROOT/pages/manual/configuration.adoc | 62 +--- .../manual/configuration-xml-format.adoc | 111 +++++++ 12 files changed, 544 insertions(+), 305 deletions(-) delete mode 100644 log4j-core-test/src/test/java/org/apache/logging/log4j/core/StrictXmlConfigTest.java create mode 100644 log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationConstructHierarchyTest.java create mode 100644 log4j-core-test/src/test/resources/XmlConfigurationConstructHierarchyTest/included-fragment.xml create mode 100644 log4j-core-test/src/test/resources/XmlConfigurationConstructHierarchyTest/included-with-id.xml create mode 100644 log4j-core-test/src/test/resources/XmlConfigurationConstructHierarchyTest/included.xml delete mode 100644 log4j-core-test/src/test/resources/log4j-strict1.xml create mode 100644 src/changelog/.2.x.x/xinclude_fixup_attributes.xml create mode 100644 src/site/antora/modules/ROOT/examples/manual/configuration/xinclude-xpointer-appenders.xml create mode 100644 src/site/antora/modules/ROOT/examples/manual/configuration/xinclude-xpointer-main.xml create mode 100644 src/site/antora/modules/ROOT/partials/manual/configuration-xml-format.adoc diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/StrictXmlConfigTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/StrictXmlConfigTest.java deleted file mode 100644 index 0bd00104b17..00000000000 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/StrictXmlConfigTest.java +++ /dev/null @@ -1,131 +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; - -import static org.junit.jupiter.api.Assertions.assertEquals; - -import java.util.Date; -import java.util.List; -import java.util.Locale; -import org.apache.logging.log4j.MarkerManager; -import org.apache.logging.log4j.ThreadContext; -import org.apache.logging.log4j.core.test.appender.ListAppender; -import org.apache.logging.log4j.core.test.junit.LoggerContextSource; -import org.apache.logging.log4j.core.test.junit.Named; -import org.apache.logging.log4j.message.EntryMessage; -import org.apache.logging.log4j.message.StructuredDataMessage; -import org.junit.jupiter.api.Test; - -@LoggerContextSource("log4j-strict1.xml") -class StrictXmlConfigTest { - - org.apache.logging.log4j.Logger logger; - private ListAppender app; - - public StrictXmlConfigTest(final LoggerContext context, @Named("List") final ListAppender app) { - logger = context.getLogger("LoggerTest"); - this.app = app.clear(); - } - - @Test - void basicFlow() { - final EntryMessage entry = logger.traceEntry(); - logger.traceExit(entry); - final List events = app.getEvents(); - assertEquals(2, events.size(), "Incorrect number of events. Expected 2, actual " + events.size()); - } - - @Test - void basicFlowDeprecated() { - logger.traceEntry(); - logger.traceExit(); - final List events = app.getEvents(); - assertEquals(2, events.size(), "Incorrect number of events. Expected 2, actual " + events.size()); - } - - @Test - void simpleFlow() { - logger.traceEntry(); - logger.traceExit(0); - final List events = app.getEvents(); - assertEquals(2, events.size(), "Incorrect number of events. Expected 2, actual " + events.size()); - } - - @Test - void throwing() { - logger.throwing(new IllegalArgumentException("Test Exception")); - final List events = app.getEvents(); - assertEquals(1, events.size(), "Incorrect number of events. Expected 1, actual " + events.size()); - } - - @Test - void catching() { - try { - throw new NullPointerException(); - } catch (final Exception e) { - logger.catching(e); - } - final List events = app.getEvents(); - assertEquals(1, events.size(), "Incorrect number of events. Expected 1, actual " + events.size()); - } - - @Test - void debug() { - logger.debug("Debug message"); - final List events = app.getEvents(); - assertEquals(1, events.size(), "Incorrect number of events. Expected 1, actual " + events.size()); - } - - @Test - void debugObject() { - logger.debug(new Date()); - final List events = app.getEvents(); - assertEquals(1, events.size(), "Incorrect number of events. Expected 1, actual " + events.size()); - } - - @Test - void debugWithParms() { - logger.debug("Hello, {}", "World"); - final List events = app.getEvents(); - assertEquals(1, events.size(), "Incorrect number of events. Expected 1, actual " + events.size()); - } - - @Test - void mdc() { - ThreadContext.put("TestYear", "2010"); - logger.debug("Debug message"); - ThreadContext.clearMap(); - logger.debug("Debug message"); - final List events = app.getEvents(); - assertEquals(2, events.size(), "Incorrect number of events. Expected 2, actual " + events.size()); - } - - @Test - void structuredData() { - ThreadContext.put("loginId", "JohnDoe"); - ThreadContext.put("ipAddress", "192.168.0.120"); - ThreadContext.put("locale", Locale.US.getDisplayName()); - final StructuredDataMessage msg = new StructuredDataMessage("Audit@18060", "Transfer Complete", "Transfer"); - msg.put("ToAccount", "123456"); - msg.put("FromAccount", "123457"); - msg.put("Amount", "200.00"); - logger.info(MarkerManager.getMarker("EVENT"), msg); - ThreadContext.clearMap(); - final List events = app.getEvents(); - assertEquals(1, events.size(), "Incorrect number of events. Expected 1, actual " + events.size()); - } -} diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationConstructHierarchyTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationConstructHierarchyTest.java new file mode 100644 index 00000000000..a0fbb29324e --- /dev/null +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationConstructHierarchyTest.java @@ -0,0 +1,279 @@ +/* + * 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.assertThat; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; + +import java.io.StringReader; +import java.util.Objects; +import javax.xml.XMLConstants; +import javax.xml.parsers.DocumentBuilder; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.core.config.Node; +import org.apache.logging.log4j.core.config.plugins.util.PluginManager; +import org.apache.logging.log4j.core.config.plugins.util.PluginType; +import org.apache.logging.log4j.test.ListStatusListener; +import org.apache.logging.log4j.test.junit.UsingStatusListener; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.w3c.dom.Element; +import org.xml.sax.InputSource; + +/** + * Unit tests for {@link XmlConfiguration#constructHierarchy(Node, Element, PluginManager, boolean)}. + *

+ * These exercise the DOM-to-{@link Node} conversion in isolation: the {@link PluginManager} is mocked, so the test + * decides which element names resolve to a plugin without pulling in the real plugin registry or a running + * {@code LoggerContext}. + *

+ */ +@ExtendWith(MockitoExtension.class) +class XmlConfigurationConstructHierarchyTest { + + private static final String INCLUDED_URI = resourceUri("included.xml"); + private static final String FRAGMENT_URI = resourceUri("included-fragment.xml"); + private static final String WITH_ID_URI = resourceUri("included-with-id.xml"); + + @Mock + private PluginManager pluginManager; + + private static String resourceUri(final String name) { + return Objects.requireNonNull(XmlConfigurationConstructHierarchyTest.class.getResource( + "/XmlConfigurationConstructHierarchyTest/" + name)) + .toExternalForm(); + } + + /** Parses an inline XML snippet into its root {@link Element}, configuring the parser exactly as production does. */ + private static Element parse(final String xml) throws Exception { + return parse(xml, false); + } + + private static Element parse(final String xml, final boolean enableXInclude) throws Exception { + final DocumentBuilder builder = XmlConfiguration.newDocumentBuilder(enableXInclude); + return builder.parse(new InputSource(new StringReader(xml))).getDocumentElement(); + } + + /** Builds an {@code } document with a single {@code xi:include} of {@code href}, optionally pointed. */ + private static String including(final String href, final String xpointer) { + final String pointer = xpointer == null ? "" : " xpointer=\"" + xpointer + "\""; + return ""; + } + + @SuppressWarnings("unchecked") + private void givenPlugin(final String name) { + // `lenient`: `constructHierarchy` also looks up names that are intentionally left unresolved (returning + // `null`), which strict stubbing would otherwise flag as an argument mismatch. + lenient().when(pluginManager.getPluginType(name)).thenReturn(mock(PluginType.class)); + } + + /** A child element that resolves to a plugin becomes a child {@link Node} carrying that plugin type. */ + @Test + void resolvedElementBecomesChildNode() throws Exception { + givenPlugin("Console"); + final Element element = parse(""); + + final Node root = new Node(); + XmlConfiguration.constructHierarchy(root, element, pluginManager, false); + + assertThat(root.getChildren()).singleElement().satisfies(child -> { + assertThat(child.getName()).isEqualTo("Console"); + assertThat(child.getType()).isNotNull(); + assertThat(child.getAttributes()).containsEntry("name", "STDOUT"); + }); + } + + /** A leaf element with no plugin type and a text value is folded into its parent as an attribute. */ + @Test + void unresolvedLeafElementBecomesAttribute() throws Exception { + // `Pattern` has no plugin type, so `` absorbs it as a `Pattern` attribute. + final Element element = parse("%m%n"); + + final Node patternLayout = new Node(); + XmlConfiguration.constructHierarchy(patternLayout, element, pluginManager, false); + + assertThat(patternLayout.getChildren()).isEmpty(); + assertThat(patternLayout.getAttributes()).containsEntry("Pattern", "%m%n"); + } + + /** An element with no plugin type but with children cannot be folded: it is dropped and an error is logged. */ + @Test + @UsingStatusListener + void unresolvedElementWithChildrenIsDropped(final ListStatusListener listener) throws Exception { + givenPlugin("Console"); + final Element element = parse(""); + + final Node appenders = new Node(); + XmlConfiguration.constructHierarchy(appenders, element, pluginManager, false); + + // The `Unknown` element (and the `Console` nested in it) are not added to the tree. + assertThat(appenders.getChildren()).isEmpty(); + assertThat(listener.findStatusData(Level.ERROR)) + .anyMatch(data -> data.getMessage().getFormattedMessage().contains("Error processing element Unknown")); + } + + /** The text content of a resolved element becomes the node value. */ + @Test + void textContentBecomesNodeValue() throws Exception { + givenPlugin("Property"); + final Element element = + parse("target/test.log"); + + final Node properties = new Node(); + XmlConfiguration.constructHierarchy(properties, element, pluginManager, false); + + assertThat(properties.getChildren()).singleElement().satisfies(child -> { + assertThat(child.getName()).isEqualTo("Property"); + assertThat(child.getValue()).isEqualTo("target/test.log"); + assertThat(child.getAttributes()).containsEntry("name", "filename"); + }); + } + + /** In strict mode the plugin name comes from the {@code type} attribute, which is consumed in the process. */ + @Test + void strictModeResolvesPluginFromTypeAttribute() throws Exception { + givenPlugin("Console"); + final Element element = parse(""); + + final Node appenders = new Node(); + XmlConfiguration.constructHierarchy(appenders, element, pluginManager, true); + + assertThat(appenders.getChildren()).singleElement().satisfies(child -> { + assertThat(child.getName()).isEqualTo("Console"); + // The `type` attribute is consumed by the plugin-name lookup and not exposed as a Log4j attribute. + assertThat(child.getAttributes()).containsEntry("name", "STDOUT").doesNotContainKey("type"); + }); + } + + /** Without strict mode the tag name is the plugin name and {@code type} stays an ordinary attribute. */ + @Test + void nonStrictModeResolvesPluginFromTagName() throws Exception { + // With the tag name used as the plugin name, `Console` (the `type` attribute) is never looked up. + givenPlugin("Appender"); + final Element element = parse(""); + + final Node appenders = new Node(); + XmlConfiguration.constructHierarchy(appenders, element, pluginManager, false); + + assertThat(appenders.getChildren()).singleElement().satisfies(child -> { + assertThat(child.getName()).isEqualTo("Appender"); + assertThat(child.getAttributes()).containsEntry("type", "Console").containsEntry("name", "STDOUT"); + }); + } + + /** An {@code xml:base} attribute (as added by the XInclude {@code fixup-base-uris} feature) is not exposed. */ + @Test + void xmlBaseAttributeIsStripped() throws Exception { + givenPlugin("Console"); + final Element element = parse(""); + + final Node appenders = new Node(); + XmlConfiguration.constructHierarchy(appenders, element, pluginManager, false); + + assertThat(appenders.getChildren()).singleElement().satisfies(child -> assertThat(child.getAttributes()) + .containsEntry("name", "STDOUT") + .doesNotContainKey("xml:base")); + } + + /** An {@code xml:lang} attribute (as added by the XInclude {@code fixup-language} feature) is not exposed. */ + @Test + void xmlLangAttributeIsStripped() throws Exception { + givenPlugin("Console"); + final Element element = parse(""); + + final Node appenders = new Node(); + XmlConfiguration.constructHierarchy(appenders, element, pluginManager, false); + + assertThat(appenders.getChildren()).singleElement().satisfies(child -> assertThat(child.getAttributes()) + .containsEntry("name", "STDOUT") + .doesNotContainKey("xml:lang")); + } + + /** + * Checks the relevance of Log4j's attribute stripping + * + *

The XInclude {@code fixup-base-uris} and {@code fixup-language} features default to {@code true}, so a normal + * XInclude parse really does add {@code xml:base} and {@code xml:lang} to the included element.

+ */ + @Test + void fixupAttributesAreProducedByDefault() throws Exception { + // `xml:lang` on the parent gives `fixup-language` a value to reconcile on the included ``. + final String document = "" + + ""; + final Element console = + (Element) parse(document, true).getElementsByTagName("Console").item(0); + + assertThat(console.hasAttributeNS(XMLConstants.XML_NS_URI, "base")).isTrue(); + assertThat(console.hasAttributeNS(XMLConstants.XML_NS_URI, "lang")).isTrue(); + } + + /** With XInclude enabled, an {@code xi:include} is resolved and its content spliced into the tree. */ + @Test + void xIncludeIsHonored() throws Exception { + givenPlugin("Console"); + final Element element = parse(including(INCLUDED_URI, null), true); + + final Node appenders = new Node(); + XmlConfiguration.constructHierarchy(appenders, element, pluginManager, false); + + // The `` from `included.xml` has replaced the `` element. + assertThat(appenders.getChildren()).singleElement().satisfies(child -> { + assertThat(child.getName()).isEqualTo("Console"); + assertThat(child.getAttributes()).containsEntry("name", "STDOUT"); + }); + } + + /** A positional {@code xpointer} ({@code element(/1/2)}) selects a single element by its position. */ + @Test + void xIncludeResolvesPositionalPointer() throws Exception { + givenPlugin("Console"); + // `element(/1/2)`: the second child element of the document root (``). + final Element element = parse(including(FRAGMENT_URI, "element(/1/2)"), true); + + final Node appenders = new Node(); + XmlConfiguration.constructHierarchy(appenders, element, pluginManager, false); + + assertThat(appenders.getChildren()).singleElement().satisfies(child -> { + assertThat(child.getName()).isEqualTo("Console"); + assertThat(child.getAttributes()).containsEntry("name", "SECOND"); + }); + } + + /** + * A shorthand {@code xpointer} selects an element by an {@code ID}-typed attribute. + * + *

The {@code name} attribute is typed {@code ID} through the included document's internal DTD subset, + * which Log4j's hardening leaves enabled (only the external subset is disabled).

+ */ + @Test + void xIncludeResolvesIdPointer() throws Exception { + givenPlugin("Console"); + final Element element = parse(including(WITH_ID_URI, "OTHER"), true); + + final Node appenders = new Node(); + XmlConfiguration.constructHierarchy(appenders, element, pluginManager, false); + + assertThat(appenders.getChildren()).singleElement().satisfies(child -> { + assertThat(child.getName()).isEqualTo("Console"); + assertThat(child.getAttributes()).containsEntry("name", "OTHER"); + }); + } +} diff --git a/log4j-core-test/src/test/resources/XmlConfigurationConstructHierarchyTest/included-fragment.xml b/log4j-core-test/src/test/resources/XmlConfigurationConstructHierarchyTest/included-fragment.xml new file mode 100644 index 00000000000..47e5e0906b0 --- /dev/null +++ b/log4j-core-test/src/test/resources/XmlConfigurationConstructHierarchyTest/included-fragment.xml @@ -0,0 +1,21 @@ + + + + + + diff --git a/log4j-core-test/src/test/resources/XmlConfigurationConstructHierarchyTest/included-with-id.xml b/log4j-core-test/src/test/resources/XmlConfigurationConstructHierarchyTest/included-with-id.xml new file mode 100644 index 00000000000..b2c4f05d617 --- /dev/null +++ b/log4j-core-test/src/test/resources/XmlConfigurationConstructHierarchyTest/included-with-id.xml @@ -0,0 +1,26 @@ + + + + + +]> + + + + diff --git a/log4j-core-test/src/test/resources/XmlConfigurationConstructHierarchyTest/included.xml b/log4j-core-test/src/test/resources/XmlConfigurationConstructHierarchyTest/included.xml new file mode 100644 index 00000000000..d087f23b12b --- /dev/null +++ b/log4j-core-test/src/test/resources/XmlConfigurationConstructHierarchyTest/included.xml @@ -0,0 +1,18 @@ + + + diff --git a/log4j-core-test/src/test/resources/log4j-strict1.xml b/log4j-core-test/src/test/resources/log4j-strict1.xml deleted file mode 100644 index cb370260469..00000000000 --- a/log4j-core-test/src/test/resources/log4j-strict1.xml +++ /dev/null @@ -1,65 +0,0 @@ - - - - - target/test.log - - - - - - - - - - - - - - - - - - - - - %d %p %C{1.} [%t] %m%n - - - - - - - - - - - - - > - - - - > - - - - - - - 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 3218589bb1d..88611718998 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 @@ -20,7 +20,6 @@ import java.io.IOException; import java.io.InputStream; import java.time.Instant; -import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Map; @@ -38,6 +37,7 @@ import org.apache.logging.log4j.core.config.ConfigurationSource; import org.apache.logging.log4j.core.config.Node; import org.apache.logging.log4j.core.config.Reconfigurable; +import org.apache.logging.log4j.core.config.plugins.util.PluginManager; import org.apache.logging.log4j.core.config.plugins.util.PluginType; import org.apache.logging.log4j.core.config.status.StatusConfiguration; import org.apache.logging.log4j.core.internal.annotation.SuppressFBWarnings; @@ -60,10 +60,6 @@ */ 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 final List status = new ArrayList<>(); private Element rootElement; private boolean strict; private String schemaResource; @@ -231,8 +227,6 @@ private static void enableXInclude(final DocumentBuilderFactory factory) { LOGGER.warn( "The DocumentBuilderFactory [{}] is out of date and does not support XInclude: {}", factory, err); } - setFeature(factory, XINCLUDE_FIXUP_BASE_URIS, true); - setFeature(factory, XINCLUDE_FIXUP_LANGUAGE, true); } @Override @@ -241,13 +235,7 @@ public void setup() { LOGGER.error("No logging configuration"); return; } - constructHierarchy(rootNode, rootElement); - if (!status.isEmpty()) { - for (final Status s : status) { - LOGGER.error("Error processing element {} ({}): {}", s.name, s.element, s.errorType); - } - return; - } + constructHierarchy(rootNode, rootElement, pluginManager, strict); rootElement = null; } @@ -266,7 +254,9 @@ public Configuration reconfigure() { return null; } - private void constructHierarchy(final Node node, final Element element) { + // Package-private for testing + static void constructHierarchy( + final Node node, final Element element, final PluginManager pluginManager, final boolean strict) { processAttributes(node, element); final StringBuilder buffer = new StringBuilder(); final NodeList list = element.getChildNodes(); @@ -275,16 +265,16 @@ private void constructHierarchy(final Node node, final Element element) { final org.w3c.dom.Node w3cNode = list.item(i); if (w3cNode instanceof Element) { final Element child = (Element) w3cNode; - final String name = getType(child); + final String name = getType(child, strict); final PluginType type = pluginManager.getPluginType(name); final Node childNode = new Node(node, name, type); - constructHierarchy(childNode, child); + constructHierarchy(childNode, child, pluginManager, strict); if (type == null) { final String value = childNode.getValue(); if (!childNode.hasChildren() && value != null) { node.getAttributes().put(name, value); } else { - status.add(new Status(name, element, ErrorType.CLASS_NOT_FOUND)); + LOGGER.error("Error processing element {} ({}): CLASS_NOT_FOUND", name, element); } } else { children.add(childNode); @@ -301,7 +291,7 @@ private void constructHierarchy(final Node node, final Element element) { } } - private String getType(final Element element) { + private static String getType(final Element element, final boolean strict) { if (strict) { final NamedNodeMap attrs = element.getAttributes(); for (int i = 0; i < attrs.getLength(); ++i) { @@ -319,7 +309,7 @@ private String getType(final Element element) { return element.getTagName(); } - private Map processAttributes(final Node node, final Element element) { + private static Map processAttributes(final Node node, final Element element) { final NamedNodeMap attrs = element.getAttributes(); final Map attributes = node.getAttributes(); @@ -327,7 +317,9 @@ private Map processAttributes(final Node node, final Element ele final org.w3c.dom.Node w3cNode = attrs.item(i); if (w3cNode instanceof Attr) { final Attr attr = (Attr) w3cNode; - if (attr.getName().equals("xml:base")) { + // The XInclude `fixup-base-uris` and `fixup-language` features (both enabled by default) add + // `xml:base` and `xml:lang` attributes to the top-level included elements. + if (XMLConstants.XML_NS_URI.equals(attr.getNamespaceURI())) { continue; } attributes.put(attr.getName(), attr.getValue()); @@ -341,31 +333,4 @@ public String toString() { return getClass().getSimpleName() + "[location=" + getConfigurationSource() + ", lastModified=" + Instant.ofEpochMilli(getConfigurationSource().getLastModified()) + "]"; } - - /** - * The error that occurred. - */ - private enum ErrorType { - CLASS_NOT_FOUND - } - - /** - * Status for recording errors. - */ - private static class Status { - private final Element element; - private final String name; - private final ErrorType errorType; - - public Status(final String name, final Element element, final ErrorType errorType) { - this.name = name; - this.element = element; - this.errorType = errorType; - } - - @Override - public String toString() { - return "Status [name=" + name + ", element=" + element + ", errorType=" + errorType + "]"; - } - } } diff --git a/src/changelog/.2.x.x/xinclude_fixup_attributes.xml b/src/changelog/.2.x.x/xinclude_fixup_attributes.xml new file mode 100644 index 00000000000..525ecfeddde --- /dev/null +++ b/src/changelog/.2.x.x/xinclude_fixup_attributes.xml @@ -0,0 +1,11 @@ + + + + Fix XML configurations exposing the reserved `xml:base` and `xml:lang` attributes as configuration attributes. + + diff --git a/src/site/antora/modules/ROOT/examples/manual/configuration/xinclude-xpointer-appenders.xml b/src/site/antora/modules/ROOT/examples/manual/configuration/xinclude-xpointer-appenders.xml new file mode 100644 index 00000000000..9587c6f8b56 --- /dev/null +++ b/src/site/antora/modules/ROOT/examples/manual/configuration/xinclude-xpointer-appenders.xml @@ -0,0 +1,30 @@ + + + + + +]> + + + + + + + + diff --git a/src/site/antora/modules/ROOT/examples/manual/configuration/xinclude-xpointer-main.xml b/src/site/antora/modules/ROOT/examples/manual/configuration/xinclude-xpointer-main.xml new file mode 100644 index 00000000000..c28a071fd80 --- /dev/null +++ b/src/site/antora/modules/ROOT/examples/manual/configuration/xinclude-xpointer-main.xml @@ -0,0 +1,34 @@ + + + + + + + + + + + + + + diff --git a/src/site/antora/modules/ROOT/pages/manual/configuration.adoc b/src/site/antora/modules/ROOT/pages/manual/configuration.adoc index 3b64deb3cba..e065d0ce9bc 100644 --- a/src/site/antora/modules/ROOT/pages/manual/configuration.adoc +++ b/src/site/antora/modules/ROOT/pages/manual/configuration.adoc @@ -1165,67 +1165,7 @@ include::partial$manual/composite-configuration.adoc[tag=how] [id=format-specific-notes] == Format specific notes -[id=xml-features] -=== XML format - -[id=xml-global-configuration-attributes] -==== Global configuration attributes - -The XML format supports the following additional attributes on the `Configuration` element. - -[id=configuration-attribute-schema] -===== `schema` - -[cols="1h,5"] -|=== -| Type | classpath resource -| Default value | `null` -|=== - -Specifies the path to a classpath resource containing an XML schema. - -[id=configuration-attribute-strict] -===== `strict` - -[cols="1h,5"] -|=== -| Type | `boolean` -| Default value | `false` -|=== - -If set to `true,` all configuration files will be checked against the XML schema provided by the -<>. - -This setting also enables "XML strict mode" and allows one to specify an element's **plugin type** through a `type` attribute instead of the tag name. - -[id=xinclude] -==== [[XInlcude]] XInclude - -XML configuration files can include other files with -https://www.w3.org/TR/xinclude/[XInclude]. - -NOTE: The list of `XInclude` and `XPath` features supported depends upon your -https://docs.oracle.com/javase/{java-target-version}/docs/technotes/guides/xml/jaxp/index.html[JAXP implementation]. - -Here is an example `log4j2.xml` file that includes two other files: - -.Snippet from an example {antora-examples-url}/manual/configuration/xinclude-main.xml[`log4j2.xml`] -[source,xml] ----- -include::example$manual/configuration/xinclude-main.xml[lines=1;18..-1] ----- - -.Snippet from an example {antora-examples-url}/manual/configuration/xinclude-appenders.xml[`xinclude-appenders.xml`] -[source,xml] ----- -include::example$manual/configuration/xinclude-appenders.xml[lines=1;18..-1] ----- - -.Snippet from an example {antora-examples-url}/manual/configuration/xinclude-loggers.xml[`xinclude-loggers.xml`] -[source,xml] ----- -include::example$manual/configuration/xinclude-loggers.xml[lines=1;18..-1] ----- +include::partial$manual/configuration-xml-format.adoc[leveloffset=+2] [id=java-properties-features] === Java properties format diff --git a/src/site/antora/modules/ROOT/partials/manual/configuration-xml-format.adoc b/src/site/antora/modules/ROOT/partials/manual/configuration-xml-format.adoc new file mode 100644 index 00000000000..5a19684e08d --- /dev/null +++ b/src/site/antora/modules/ROOT/partials/manual/configuration-xml-format.adoc @@ -0,0 +1,111 @@ +//// + 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. +//// + +[id=xml-features] += XML format + +[id=xml-global-configuration-attributes] +== Global configuration attributes + +The XML format supports the following additional attributes on the `Configuration` element. + +[id=configuration-attribute-schema] +=== `schema` + +[cols="1h,5"] +|=== +| Type | classpath resource +| Default value | `null` +|=== + +Specifies the path to a classpath resource containing an XML schema. + +[id=configuration-attribute-strict] +=== `strict` + +[cols="1h,5"] +|=== +| Type | `boolean` +| Default value | `false` +|=== + +If set to `true,` all configuration files will be checked against the XML schema provided by the +<>. + +This setting also enables "XML strict mode" and allows one to specify an element's **plugin type** through a `type` attribute instead of the tag name. + +[id=xinclude] +== [[XInlcude]] XInclude + +XML configuration files can include other files with +https://www.w3.org/TR/xinclude/[XInclude]. + +NOTE: The list of `XInclude` and `XPath` features supported depends upon your +https://docs.oracle.com/javase/{java-target-version}/docs/technotes/guides/xml/jaxp/index.html[JAXP implementation]. + +Here is an example `log4j2.xml` file that includes two other files: + +.Snippet from an example {antora-examples-url}/manual/configuration/xinclude-main.xml[`log4j2.xml`] +[source,xml] +---- +include::example$manual/configuration/xinclude-main.xml[lines=1;18..-1] +---- + +.Snippet from an example {antora-examples-url}/manual/configuration/xinclude-appenders.xml[`xinclude-appenders.xml`] +[source,xml] +---- +include::example$manual/configuration/xinclude-appenders.xml[lines=1;18..-1] +---- + +.Snippet from an example {antora-examples-url}/manual/configuration/xinclude-loggers.xml[`xinclude-loggers.xml`] +[source,xml] +---- +include::example$manual/configuration/xinclude-loggers.xml[lines=1;18..-1] +---- + +Instead of a whole file, you can include a single element by appending an +https://www.w3.org/TR/xptr-framework/[XPointer] to the `href`. +The JDK's XInclude engine only implements the `element()` scheme and *shorthand* pointers, so two forms are available: + +* A *positional* pointer addresses an element by its position in the document. +For example `xpointer="element(/1/2)"` selects the second child element of the document root: ++ +[source,xml] +---- + +---- + +* A *shorthand* pointer addresses an element by an attribute of type `ID`. +The appender `name` is a natural identifier: ++ +.Snippet from an example {antora-examples-url}/manual/configuration/xinclude-xpointer-appenders.xml[`xinclude-xpointer-appenders.xml`] +[source,xml] +---- +include::example$manual/configuration/xinclude-xpointer-appenders.xml[lines=1;18..-1] +---- ++ +.Snippet from an example {antora-examples-url}/manual/configuration/xinclude-xpointer-main.xml[`log4j2.xml`] +[source,xml] +---- +include::example$manual/configuration/xinclude-xpointer-main.xml[tag=inclusion,indent=0] +---- + +[NOTE] +==== +The JDK's XInclude engine does **not** recognize the `xml:id` attribute as an `ID`, and it does **not** implement the full `xpointer()` scheme. +Only an attribute typed `ID` through a DTD (in practice, the included file's internal subset) can be addressed by a shorthand pointer. +==== \ No newline at end of file From f55fbd9d7d9ea3aa7cfbeb21322c0da41265e37d Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Fri, 26 Jun 2026 13:49:14 +0200 Subject: [PATCH 2/8] fix: add reference to PR --- src/changelog/.2.x.x/xinclude_fixup_attributes.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/src/changelog/.2.x.x/xinclude_fixup_attributes.xml b/src/changelog/.2.x.x/xinclude_fixup_attributes.xml index 525ecfeddde..f205162d3a3 100644 --- a/src/changelog/.2.x.x/xinclude_fixup_attributes.xml +++ b/src/changelog/.2.x.x/xinclude_fixup_attributes.xml @@ -5,6 +5,7 @@ https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd" type="fixed"> + Fix XML configurations exposing the reserved `xml:base` and `xml:lang` attributes as configuration attributes. From 9b3d763c1b5d632f7b19455826ae5cd9e5dac8d0 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Fri, 26 Jun 2026 13:56:52 +0200 Subject: [PATCH 3/8] Add a test for nested XInclude resolution Verify that an included document may itself contain an `xi:include`, and that the inner relative `href` resolves against the included document's own location rather than the top-level configuration's. Assisted-By: Claude Opus 4.8 (1M context) --- ...mlConfigurationConstructHierarchyTest.java | 28 +++++++++++++++++++ .../nested.xml | 21 ++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 log4j-core-test/src/test/resources/XmlConfigurationConstructHierarchyTest/nested.xml diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationConstructHierarchyTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationConstructHierarchyTest.java index a0fbb29324e..26c48e2edfd 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationConstructHierarchyTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationConstructHierarchyTest.java @@ -51,6 +51,7 @@ class XmlConfigurationConstructHierarchyTest { private static final String INCLUDED_URI = resourceUri("included.xml"); private static final String FRAGMENT_URI = resourceUri("included-fragment.xml"); private static final String WITH_ID_URI = resourceUri("included-with-id.xml"); + private static final String NESTED_URI = resourceUri("nested.xml"); @Mock private PluginManager pluginManager; @@ -276,4 +277,31 @@ void xIncludeResolvesIdPointer() throws Exception { assertThat(child.getAttributes()).containsEntry("name", "OTHER"); }); } + + /** + * XInclude resolution is recursive: an included document may itself contain an {@code xi:include}. The inner + * {@code href} is relative and must resolve against the included document's own location, not the + * top-level configuration's. + */ + @Test + void xIncludeResolvesNestedIncludes() throws Exception { + givenPlugin("Appenders"); + givenPlugin("Console"); + // `nested.xml` (an `` fragment) is included by absolute URI and itself includes `included.xml` + // (a ``) by a relative href. + final String document = "" + + ""; + final Element element = parse(document, true); + + final Node configuration = new Node(); + XmlConfiguration.constructHierarchy(configuration, element, pluginManager, false); + + assertThat(configuration.getChildren()).singleElement().satisfies(appenders -> { + assertThat(appenders.getName()).isEqualTo("Appenders"); + assertThat(appenders.getChildren()).singleElement().satisfies(console -> { + assertThat(console.getName()).isEqualTo("Console"); + assertThat(console.getAttributes()).containsEntry("name", "STDOUT"); + }); + }); + } } diff --git a/log4j-core-test/src/test/resources/XmlConfigurationConstructHierarchyTest/nested.xml b/log4j-core-test/src/test/resources/XmlConfigurationConstructHierarchyTest/nested.xml new file mode 100644 index 00000000000..b525f1a21d3 --- /dev/null +++ b/log4j-core-test/src/test/resources/XmlConfigurationConstructHierarchyTest/nested.xml @@ -0,0 +1,21 @@ + + + + + + From f79a1db4478a864be9de76222ecd513ed1090da0 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Fri, 26 Jun 2026 14:23:36 +0200 Subject: [PATCH 4/8] Fix formatting --- .../config/xml/XmlConfigurationConstructHierarchyTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationConstructHierarchyTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationConstructHierarchyTest.java index 26c48e2edfd..73c2019b6cb 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationConstructHierarchyTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationConstructHierarchyTest.java @@ -210,7 +210,7 @@ void xmlLangAttributeIsStripped() throws Exception { /** * Checks the relevance of Log4j's attribute stripping - * + * *

The XInclude {@code fixup-base-uris} and {@code fixup-language} features default to {@code true}, so a normal * XInclude parse really does add {@code xml:base} and {@code xml:lang} to the included element.

*/ @@ -289,8 +289,8 @@ void xIncludeResolvesNestedIncludes() throws Exception { givenPlugin("Console"); // `nested.xml` (an `` fragment) is included by absolute URI and itself includes `included.xml` // (a ``) by a relative href. - final String document = "" - + ""; + final String document = "" + ""; final Element element = parse(document, true); final Node configuration = new Node(); From ab09d5f22c54dc0b2fd668ab6ab938dd5062caf5 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Fri, 26 Jun 2026 14:34:44 +0200 Subject: [PATCH 5/8] Make XInclude processing in XML configurations opt-in XInclude support in XML configuration files is now disabled by default and must be enabled with the `log4j2.configurationEnableXInclude` property. The constructor reads the property and configures the `DocumentBuilder` accordingly, dropping the previous always-on behavior and its LOG4J2-1127 "retry without XInclude" fallback. The new property is documented alongside the other configuration-factory properties, and the XInclude manual section notes that it is now opt-in. The `ConfigurationFactoryTest.xinclude` test enables the property, and `testGetConfigurationWithMultipleUris` now composes two plain configurations instead of incidentally loading an XInclude file. A new `ConfigurationFactoryUnitTest` loads the same logical configuration as XML, JSON, YAML, Java properties, an XInclude-composed file (with the property enabled) and a cross-format composite, asserting through the `Configuration` interface that each yields the same components. It also covers the XInclude-disabled fallback to the default configuration. Assisted-By: Claude Opus 4.8 (1M context) --- .../core/config/ConfigurationFactoryTest.java | 4 +- .../config/ConfigurationFactoryUnitTest.java | 171 ++++++++++++++++++ .../src/test/resources/log4j-test1-basic.xml | 29 +++ .../test/resources/log4j-test1-extended.yaml | 53 ++++++ .../core/config/xml/XmlConfiguration.java | 28 +-- src/changelog/.2.x.x/xinclude_opt_in.xml | 12 ++ .../manual/configuration-xml-format.adoc | 8 + .../properties-configuration-factory.adoc | 14 ++ 8 files changed, 299 insertions(+), 20 deletions(-) create mode 100644 log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryUnitTest.java create mode 100644 log4j-core-test/src/test/resources/log4j-test1-basic.xml create mode 100644 log4j-core-test/src/test/resources/log4j-test1-extended.yaml create mode 100644 src/changelog/.2.x.x/xinclude_opt_in.xml 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 ee19ea7ea86..a7232d1f349 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.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 @@ void xml(final LoggerContext context) throws IOException { } @Test + @SetTestProperty(key = "log4j2.configurationEnableXInclude", value = "true") @LoggerContextSource("log4j-xinclude.xml") void xinclude(final LoggerContext context) throws IOException { checkConfiguration(context); @@ -181,7 +183,7 @@ void testGetConfigurationWithMultipleUris() throws Exception { final URI configLocation1 = getClass().getResource("/log4j-test1.xml").toURI(); final URI configLocation2 = - getClass().getResource("/log4j-xinclude.xml").toURI(); + getClass().getResource("/log4j-test2.xml").toURI(); final List configLocations = Arrays.asList(configLocation1, configLocation2); final Configuration config = factory.getConfiguration(context, "test", configLocations); assertInstanceOf(CompositeConfiguration.class, config); diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryUnitTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryUnitTest.java new file mode 100644 index 00000000000..b34bf0483a4 --- /dev/null +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryUnitTest.java @@ -0,0 +1,171 @@ +/* + * 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; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.net.URI; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.core.Appender; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.appender.ConsoleAppender; +import org.apache.logging.log4j.core.appender.FileAppender; +import org.apache.logging.log4j.core.config.composite.CompositeConfiguration; +import org.apache.logging.log4j.core.filter.ThreadContextMapFilter; +import org.apache.logging.log4j.core.test.appender.ListAppender; +import org.apache.logging.log4j.test.junit.SetTestProperty; +import org.apache.logging.log4j.test.junit.TempLoggingDir; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** + * Verifies that the same logical configuration, expressed as XML, JSON, YAML, Java properties, an XInclude-composed + * XML file, or a merged configuration produces the same components when loaded through {@link ConfigurationFactory}. + * + *

Unlike {@code ConfigurationFactoryTest}, this exercises the {@link Configuration} directly.

+ */ +class ConfigurationFactoryUnitTest { + + // The File appender creates the log file at construction time, not at startup. + // This directory will hold the created files and delete them after the test. + @TempLoggingDir + private static Path loggingDir; + + private static Configuration load(final String resource) throws Exception { + final URI uri = + ConfigurationFactoryUnitTest.class.getResource("/" + resource).toURI(); + final Configuration configuration = + ConfigurationFactory.getInstance().getConfiguration(new LoggerContext("test"), resource, uri); + configuration.initialize(); + return configuration; + } + + private static Configuration loadComposite(final String... resources) throws Exception { + final List uris = new ArrayList<>(); + for (final String resource : resources) { + uris.add(ConfigurationFactoryUnitTest.class + .getResource("/" + resource) + .toURI()); + } + final Configuration configuration = + ConfigurationFactory.getInstance().getConfiguration(new LoggerContext("test"), "composite", uris); + configuration.initialize(); + return configuration; + } + + @ParameterizedTest + @SetTestProperty(key = "log4j2.configurationEnableXInclude", value = "true") + @ValueSource( + strings = { + "log4j-test1.xml", + "log4j-test1.json", + "log4j-test1.yaml", + "log4j-test1.properties", + "log4j-xinclude.xml" + }) + void producesEquivalentComponents(final String resource) throws Exception { + final Configuration configuration = load(resource); + try { + assertAppenders(configuration); + assertLoggers(configuration); + // Every representation declares a `filename` substitution property resolving to a per-format log file. + assertThat(configuration.getStrSubstitutor().replace("${filename}")).endsWith(".log"); + } finally { + configuration.stop(); + } + } + + /** + * Tests behavior with XInclude disabled. + * + *

With XInclude disabled (the default), the {@code xi:include} elements are not expanded, + * so Log4j ignores them and falls back to the default setup: + * an {@code ERROR}-level root logger with a default {@code Console} appender.

+ * + *

The {@code Properties} declared directly in the file are still honored.

+ */ + @Test + void xIncludeDisabledIgnoresIncludesAndUsesDefaults() throws Exception { + final Configuration configuration = load("log4j-xinclude.xml"); + try { + // None of the included appenders or loggers are present. + final Map appenders = configuration.getAppenders(); + assertThat(appenders).doesNotContainKeys("STDOUT", "File", "List"); + assertThat(configuration.getLoggers()) + .doesNotContainKeys("org.apache.logging.log4j.test1", "org.apache.logging.log4j.test2"); + + // The default setup is used instead: a single default Console appender and an ERROR-level root logger. + assertThat(appenders.values()).singleElement().satisfies(appender -> { + assertThat(appender).isInstanceOf(ConsoleAppender.class); + assertThat(appender.getName()).startsWith("DefaultConsole-"); + }); + assertThat(configuration.getRootLogger().getLevel()).isEqualTo(Level.ERROR); + + // The `Properties` declared directly in the file (outside the includes) are still resolved. + assertThat(configuration.getStrSubstitutor().replace("${filename}")).endsWith(".log"); + } finally { + configuration.stop(); + } + } + + /** + * The same component graph can be assembled from several sources, possibly in different formats, merged into a + * {@link CompositeConfiguration}: here an XML base (one appender and the root logger) and a YAML remainder. + */ + @Test + void mergesMultipleSources() throws Exception { + final Configuration configuration = loadComposite("log4j-test1-basic.xml", "log4j-test1-extended.yaml"); + try { + assertThat(configuration).isInstanceOf(CompositeConfiguration.class); + assertAppenders(configuration); + assertLoggers(configuration); + assertThat(configuration.getStrSubstitutor().replace("${filename}")).endsWith(".log"); + } finally { + configuration.stop(); + } + } + + private static void assertAppenders(final Configuration configuration) { + final Map appenders = configuration.getAppenders(); + assertThat(appenders).containsOnlyKeys("STDOUT", "File", "List"); + assertThat(appenders.get("STDOUT")).isInstanceOf(ConsoleAppender.class); + assertThat(appenders.get("File")).isInstanceOf(FileAppender.class); + assertThat(appenders.get("List")).isInstanceOf(ListAppender.class); + } + + private static void assertLoggers(final Configuration configuration) { + final LoggerConfig test1 = configuration.getLoggerConfig("org.apache.logging.log4j.test1"); + assertThat(test1.getLevel()).isEqualTo(Level.DEBUG); + assertThat(test1.isAdditive()).isFalse(); + assertThat(test1.getFilter()).isInstanceOf(ThreadContextMapFilter.class); + assertThat(test1.getAppenderRefs()).extracting(AppenderRef::getRef).containsExactly("STDOUT"); + + final LoggerConfig test2 = configuration.getLoggerConfig("org.apache.logging.log4j.test2"); + assertThat(test2.getLevel()).isEqualTo(Level.DEBUG); + assertThat(test2.isAdditive()).isFalse(); + assertThat(test2.getAppenderRefs()).extracting(AppenderRef::getRef).containsExactly("File"); + + final LoggerConfig root = configuration.getRootLogger(); + assertThat(root.getLevel()).isEqualTo(Level.ERROR); + assertThat(root.getAppenderRefs()).extracting(AppenderRef::getRef).containsExactly("STDOUT"); + } +} diff --git a/log4j-core-test/src/test/resources/log4j-test1-basic.xml b/log4j-core-test/src/test/resources/log4j-test1-basic.xml new file mode 100644 index 00000000000..eff1f6b3de7 --- /dev/null +++ b/log4j-core-test/src/test/resources/log4j-test1-basic.xml @@ -0,0 +1,29 @@ + + + + + + + + + + + + + + diff --git a/log4j-core-test/src/test/resources/log4j-test1-extended.yaml b/log4j-core-test/src/test/resources/log4j-test1-extended.yaml new file mode 100644 index 00000000000..4c23eab3434 --- /dev/null +++ b/log4j-core-test/src/test/resources/log4j-test1-extended.yaml @@ -0,0 +1,53 @@ +# +# 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. +# +## +# Remainder of a composite configuration, merged with `log4j-test1-basic.xml` (a different format). +Configuration: + status: OFF + name: YAMLConfigTestExtended + properties: + property: + name: filename + value: ${test:logging.path}/test-composite.log + appenders: + File: + name: File + fileName: ${filename} + bufferedIO: false + PatternLayout: + Pattern: "%d %p %C{1.} [%t] %m%n" + List: + name: List + Filters: + ThresholdFilter: + level: error + Loggers: + logger: + - name: org.apache.logging.log4j.test1 + level: debug + additivity: false + ThreadContextMapFilter: + KeyValuePair: + key: test + value: 123 + AppenderRef: + ref: STDOUT + - name: org.apache.logging.log4j.test2 + level: debug + additivity: false + AppenderRef: + ref: File 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 88611718998..e36025f6d7b 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 @@ -45,7 +45,7 @@ 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; @@ -60,6 +60,11 @@ */ public class XmlConfiguration extends AbstractConfiguration implements Reconfigurable { + /** + * Property that enables XInclude processing in XML configuration files. Disabled by default. + */ + private static final String ENABLE_XINCLUDE_PROP = "log4j2.configurationEnableXInclude"; + private Element rootElement; private boolean strict; private String schemaResource; @@ -80,24 +85,9 @@ public XmlConfiguration(final LoggerContext loggerContext, final ConfigurationSo } 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 attrs = processAttributes(rootNode, rootElement); final StatusConfiguration statusConfig = new StatusConfiguration().withStatus(getDefaultStatus()); diff --git a/src/changelog/.2.x.x/xinclude_opt_in.xml b/src/changelog/.2.x.x/xinclude_opt_in.xml new file mode 100644 index 00000000000..b3ae06d871c --- /dev/null +++ b/src/changelog/.2.x.x/xinclude_opt_in.xml @@ -0,0 +1,12 @@ + + + + + Make XInclude processing in XML configurations opt-in (disabled by default): it can be enabled with the `log4j2.configurationEnableXInclude` property. + + diff --git a/src/site/antora/modules/ROOT/partials/manual/configuration-xml-format.adoc b/src/site/antora/modules/ROOT/partials/manual/configuration-xml-format.adoc index 5a19684e08d..cd8d4232322 100644 --- a/src/site/antora/modules/ROOT/partials/manual/configuration-xml-format.adoc +++ b/src/site/antora/modules/ROOT/partials/manual/configuration-xml-format.adoc @@ -54,6 +54,14 @@ This setting also enables "XML strict mode" and allows one to specify an element XML configuration files can include other files with https://www.w3.org/TR/xinclude/[XInclude]. +[IMPORTANT] +==== +Since version `2.27.0`, XInclude processing is **disabled by default**. +Set the +xref:manual/systemproperties.adoc#log4j2.configurationEnableXInclude[`log4j2.configurationEnableXInclude`] +configuration property to `true` to enable it. +==== + NOTE: The list of `XInclude` and `XPath` features supported depends upon your https://docs.oracle.com/javase/{java-target-version}/docs/technotes/guides/xml/jaxp/index.html[JAXP implementation]. diff --git a/src/site/antora/modules/ROOT/partials/manual/systemproperties/properties-configuration-factory.adoc b/src/site/antora/modules/ROOT/partials/manual/systemproperties/properties-configuration-factory.adoc index 1841a4ee4dc..e9f0f1f7b59 100644 --- a/src/site/antora/modules/ROOT/partials/manual/systemproperties/properties-configuration-factory.adoc +++ b/src/site/antora/modules/ROOT/partials/manual/systemproperties/properties-configuration-factory.adoc @@ -14,6 +14,20 @@ See the License for the specific language governing permissions and limitations under the License. //// +[id=log4j2.configurationEnableXInclude] +== `log4j2.configurationEnableXInclude` + +[cols="1h,5"] +|=== +| Env. variable | `LOG4J_CONFIGURATION_ENABLE_XINCLUDE` +| Type | `boolean` +| Default value | `false` +|=== + +If set to `true`, enables https://www.w3.org/TR/xinclude/[XInclude] processing in XML configuration files. + +See xref:manual/configuration.adoc#xinclude[XInclude] for details. + [id=log4j2.configurationFactory] == `log4j2.configurationFactory` From b6fe46a92efc02aac4254bc0d568c950e4f31a0d Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Fri, 26 Jun 2026 18:26:26 +0200 Subject: [PATCH 6/8] fix: add reference to PR --- .../.2.x.x/{xinclude_opt_in.xml => 4158_xinclude_opt_in.xml} | 1 + 1 file changed, 1 insertion(+) rename src/changelog/.2.x.x/{xinclude_opt_in.xml => 4158_xinclude_opt_in.xml} (88%) diff --git a/src/changelog/.2.x.x/xinclude_opt_in.xml b/src/changelog/.2.x.x/4158_xinclude_opt_in.xml similarity index 88% rename from src/changelog/.2.x.x/xinclude_opt_in.xml rename to src/changelog/.2.x.x/4158_xinclude_opt_in.xml index b3ae06d871c..00ecd660f64 100644 --- a/src/changelog/.2.x.x/xinclude_opt_in.xml +++ b/src/changelog/.2.x.x/4158_xinclude_opt_in.xml @@ -6,6 +6,7 @@ https://logging.apache.org/xml/ns/log4j-changelog-0.xsd" type="changed"> + Make XInclude processing in XML configurations opt-in (disabled by default): it can be enabled with the `log4j2.configurationEnableXInclude` property. From b75050644662d12a7b6b2cf919e7dc3de6fdc909 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Fri, 26 Jun 2026 18:43:51 +0200 Subject: [PATCH 7/8] Simplify XInclude enablement With XInclude now opt-in, drop the defensive LOG4J2-3531 workaround that probed the parser and silently disabled XInclude when it was unsupported. Since the user explicitly opts in, just set `setXIncludeAware(true)` and let an unsupported parser surface the error instead of degrading to a silent no-op. Assisted-By: Claude Opus 4.8 (1M context) --- .../core/config/xml/XmlConfiguration.java | 26 +------------------ 1 file changed, 1 insertion(+), 25 deletions(-) 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 e36025f6d7b..56a8fabef82 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 @@ -169,7 +169,7 @@ static DocumentBuilder newDocumentBuilder(final boolean xIncludeAware) throws Pa disableDtdProcessing(factory); if (xIncludeAware) { - enableXInclude(factory); + factory.setXIncludeAware(true); } return factory.newDocumentBuilder(); } @@ -195,30 +195,6 @@ private static void setFeature( } } - /** - * 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); - } - } - @Override public void setup() { if (rootElement == null) { From f1137c906dc9a04248e5742dfbbf82ae9720136c Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Sat, 27 Jun 2026 15:25:49 +0200 Subject: [PATCH 8/8] Start configurations in `ConfigurationFactoryUnitTest` to release file handles A `FileAppender` opens its file when constructed (during the configuration's `initialize()`), but `Configuration.stop()` only closes appenders that have been started. The test initialized the configuration without starting it, so `stop()` skipped the appenders and their files stayed open. On Windows the open files then blocked deletion of the temporary logging directory, failing the test during cleanup. Start the configuration so the matching `stop()` closes the appenders and releases the file handles. Assisted-By: Claude Opus 4.8 --- .../config/ConfigurationFactoryUnitTest.java | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryUnitTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryUnitTest.java index b34bf0483a4..d1bd2702f1b 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryUnitTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryUnitTest.java @@ -45,8 +45,7 @@ */ class ConfigurationFactoryUnitTest { - // The File appender creates the log file at construction time, not at startup. - // This directory will hold the created files and delete them after the test. + // Holds the log files created by the loaded configurations, deleted after the test. @TempLoggingDir private static Path loggingDir; @@ -55,8 +54,7 @@ private static Configuration load(final String resource) throws Exception { ConfigurationFactoryUnitTest.class.getResource("/" + resource).toURI(); final Configuration configuration = ConfigurationFactory.getInstance().getConfiguration(new LoggerContext("test"), resource, uri); - configuration.initialize(); - return configuration; + return startConfiguration(configuration); } private static Configuration loadComposite(final String... resources) throws Exception { @@ -68,7 +66,19 @@ private static Configuration loadComposite(final String... resources) throws Exc } final Configuration configuration = ConfigurationFactory.getInstance().getConfiguration(new LoggerContext("test"), "composite", uris); - configuration.initialize(); + return startConfiguration(configuration); + } + + /** + * Initializes and starts the configuration. + * + *

A {@link FileAppender} opens its file when it is constructed, during {@link Configuration#initialize()}, but + * {@link Configuration#stop()} only closes appenders that have been started. The configuration must therefore be + * started so that the matching {@code stop()} in each test releases the file handles; otherwise the open files + * would, on Windows, prevent deletion of the temporary logging directory.

+ */ + private static Configuration startConfiguration(final Configuration configuration) { + configuration.start(); return configuration; }