This is an automated email from the ASF dual-hosted git repository. ppkarwasz pushed a commit to branch fix/2.x/xinclude-fixup-attributes in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 3dce0bc55328add2083c66f453a890ac57f9304e Author: Piotr P. Karwasz <[email protected]> AuthorDate: Fri Jun 26 13:08:51 2026 +0200 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) <[email protected]> --- .../logging/log4j/core/StrictXmlConfigTest.java | 131 ---------- .../XmlConfigurationConstructHierarchyTest.java | 279 +++++++++++++++++++++ .../included-fragment.xml | 21 ++ .../included-with-id.xml | 26 ++ .../included.xml | 18 ++ .../src/test/resources/log4j-strict1.xml | 65 ----- .../log4j/core/config/xml/XmlConfiguration.java | 61 +---- src/changelog/.2.x.x/xinclude_fixup_attributes.xml | 11 + .../configuration/xinclude-xpointer-appenders.xml | 30 +++ .../configuration/xinclude-xpointer-main.xml | 34 +++ .../modules/ROOT/pages/manual/configuration.adoc | 62 +---- .../partials/manual/configuration-xml-format.adoc | 111 ++++++++ 12 files changed, 544 insertions(+), 305 deletions(-) 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 0bd00104b1..0000000000 --- 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<LogEvent> 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<LogEvent> 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<LogEvent> 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<LogEvent> 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<LogEvent> 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<LogEvent> 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<LogEvent> 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<LogEvent> 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<LogEvent> 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<LogEvent> 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 0000000000..a0fbb29324 --- /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)}. + * <p> + * 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}. + * </p> + */ +@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 <Appenders>} 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 "<Appenders xmlns:xi=\"http://www.w3.org/2001/XInclude\";><xi:include href=\"" + href + "\"" + pointer + + "/></Appenders>"; + } + + @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("<Appenders><Console name=\"STDOUT\"/></Appenders>"); + + 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 `<PatternLayout>` absorbs it as a `Pattern` attribute. + final Element element = parse("<PatternLayout><Pattern>%m%n</Pattern></PatternLayout>"); + + 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("<Appenders><Unknown><Console/></Unknown></Appenders>"); + + 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("<Properties><Property name=\"filename\">target/test.log</Property></Properties>"); + + 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("<Appenders><Appender type=\"Console\" name=\"STDOUT\"/></Appenders>"); + + 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("<Appenders><Appender type=\"Console\" name=\"STDOUT\"/></Appenders>"); + + 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("<Appenders><Console xml:base=\"urn:somewhere\" name=\"STDOUT\"/></Appenders>"); + + 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("<Appenders><Console xml:lang=\"en\" name=\"STDOUT\"/></Appenders>"); + + 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 + * + * <p>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.</p> + */ + @Test + void fixupAttributesAreProducedByDefault() throws Exception { + // `xml:lang` on the parent gives `fixup-language` a value to reconcile on the included `<Console>`. + final String document = "<Appenders xmlns:xi=\"http://www.w3.org/2001/XInclude\"; xml:lang=\"en\">" + + "<xi:include href=\"" + INCLUDED_URI + "\"/></Appenders>"; + 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 `<Console>` from `included.xml` has replaced the `<xi:include>` 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 (`<Fragment>`). + 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. + * + * <p>The {@code name} attribute is typed {@code ID} through the included document's internal DTD subset, + * which Log4j's hardening leaves enabled (only the <em>external</em> subset is disabled).</p> + */ + @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 0000000000..47e5e0906b --- /dev/null +++ b/log4j-core-test/src/test/resources/XmlConfigurationConstructHierarchyTest/included-fragment.xml @@ -0,0 +1,21 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + ~ 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. + --> +<Fragment> + <Console name="FIRST"/> + <Console name="SECOND"/> +</Fragment> 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 0000000000..b2c4f05d61 --- /dev/null +++ b/log4j-core-test/src/test/resources/XmlConfigurationConstructHierarchyTest/included-with-id.xml @@ -0,0 +1,26 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + ~ 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. + --> +<!DOCTYPE Fragment [ + <!ELEMENT Fragment (Console*)> + <!ELEMENT Console EMPTY> + <!ATTLIST Console name ID #IMPLIED> +]> +<Fragment> + <Console name="STDOUT"/> + <Console name="OTHER"/> +</Fragment> 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 0000000000..d087f23b12 --- /dev/null +++ b/log4j-core-test/src/test/resources/XmlConfigurationConstructHierarchyTest/included.xml @@ -0,0 +1,18 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + ~ 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. + --> +<Console name="STDOUT"/> 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 cb37026046..0000000000 --- a/log4j-core-test/src/test/resources/log4j-strict1.xml +++ /dev/null @@ -1,65 +0,0 @@ -<?xml version="1.0" encoding="UTF-8"?> -<!-- - ~ 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. - --> -<Configuration status="OFF" strict="true" name="XMLConfigTest"> - <Properties> - <Property name="filename">target/test.log</Property> - </Properties> - <Filter type="ThresholdFilter" level="trace"/> - - <Appenders> - <Appender type="Console" name="STDOUT"> - <Layout type="PatternLayout" pattern="%m MDC%X%n"/> - <Filters> - <Filter type="MarkerFilter" marker="FLOW" onMatch="DENY" onMismatch="NEUTRAL"/> - <Filter type="MarkerFilter" marker="EXCEPTION" onMatch="DENY" onMismatch="ACCEPT"/> - </Filters> - </Appender> - <Appender type="Console" name="FLOW"> - <Layout type="PatternLayout" pattern="%C{1}.%M %m %ex%n"/> - <Filters> - <Filter type="MarkerFilter" marker="FLOW" onMatch="ACCEPT" onMismatch="NEUTRAL"/> - <Filter type="MarkerFilter" marker="EXCEPTION" onMatch="ACCEPT" onMismatch="DENY"/> - </Filters> - </Appender> - <Appender type="File" name="File" fileName="${filename}"> - <Layout type="PatternLayout"> - <Pattern>%d %p %C{1.} [%t] %m%n</Pattern> - </Layout> - </Appender> - <Appender type="List" name="List"> - </Appender> - </Appenders> - - <Loggers> - <Logger name="org.apache.logging.log4j.test1" level="debug" additivity="false"> - <Filter type="ThreadContextMapFilter"> - <KeyValuePair key="test" value="123"/> - </Filter> - <AppenderRef ref="STDOUT"/> - </Logger>> - - <Logger name="org.apache.logging.log4j.test2" level="debug" additivity="false"> - <AppenderRef ref="File"/> - </Logger>> - - <Root level="trace"> - <AppenderRef ref="List"/> - </Root> - </Loggers> - -</Configuration> 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..8861171899 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.ByteArrayInputStream; 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.Configuration; 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 @@ 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 final List<Status> status = new ArrayList<>(); private Element rootElement; private boolean strict; private String schemaResource; @@ -231,8 +227,6 @@ public class XmlConfiguration extends AbstractConfiguration implements Reconfigu 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 class XmlConfiguration extends AbstractConfiguration implements Reconfigu 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 class XmlConfiguration extends AbstractConfiguration implements Reconfigu 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 @@ public class XmlConfiguration extends AbstractConfiguration implements Reconfigu 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 @@ public class XmlConfiguration extends AbstractConfiguration implements Reconfigu } } - 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 @@ public class XmlConfiguration extends AbstractConfiguration implements Reconfigu return element.getTagName(); } - private Map<String, String> processAttributes(final Node node, final Element element) { + private static Map<String, String> processAttributes(final Node node, final Element element) { final NamedNodeMap attrs = element.getAttributes(); final Map<String, String> attributes = node.getAttributes(); @@ -327,7 +317,9 @@ public class XmlConfiguration extends AbstractConfiguration implements Reconfigu 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 class XmlConfiguration extends AbstractConfiguration implements Reconfigu 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 0000000000..525ecfeddd --- /dev/null +++ b/src/changelog/.2.x.x/xinclude_fixup_attributes.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="fixed"> + <description format="asciidoc"> + Fix XML configurations exposing the reserved `xml:base` and `xml:lang` attributes as configuration attributes. + </description> +</entry> 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 0000000000..9587c6f8b5 --- /dev/null +++ b/src/site/antora/modules/ROOT/examples/manual/configuration/xinclude-xpointer-appenders.xml @@ -0,0 +1,30 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + ~ 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. + --> +<!DOCTYPE Appenders [ + <!-- Typing `name` as `ID` lets an `xpointer` shorthand select an appender by its name. --> + <!ATTLIST Console name ID #IMPLIED> + <!ATTLIST File name ID #IMPLIED> +]> +<Appenders> + <Console name="CONSOLE"> + <PatternLayout pattern="%d %m%n"/> + </Console> + <File name="FILE" fileName="${filename}"> + <PatternLayout pattern="%d %p %C{1.} [%t] %m%n"/> + </File> +</Appenders> 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 0000000000..c28a071fd8 --- /dev/null +++ b/src/site/antora/modules/ROOT/examples/manual/configuration/xinclude-xpointer-main.xml @@ -0,0 +1,34 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + ~ 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. + --> +<Configuration xmlns="https://logging.apache.org/xml/ns"; + xmlns:xi="http://www.w3.org/2001/XInclude"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation=" + https://logging.apache.org/xml/ns + https://logging.apache.org/xml/ns/log4j-config-2.xsd";> + <Properties> + <Property name="filename" value="app.log"/> + </Properties> + <Appenders> + <!-- Pull only the `CONSOLE` appender from the shared library, selected by its name. --> + <!-- tag::inclusion[] --> + <xi:include href="xinclude-xpointer-appenders.xml" xpointer="CONSOLE"/> + <!-- end::inclusion[] --> + </Appenders> + <xi:include href="xinclude-loggers.xml"/> +</Configuration> diff --git a/src/site/antora/modules/ROOT/pages/manual/configuration.adoc b/src/site/antora/modules/ROOT/pages/manual/configuration.adoc index 3b64deb3cb..e065d0ce9b 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 -<<configuration-attribute-schema>>. - -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 0000000000..5a19684e08 --- /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 +<<configuration-attribute-schema>>. + +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] +---- +<xi:include href="xinclude-appenders.xml" xpointer="element(/1/2)"/> +---- + +* 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
