Make XInclude processing in XML configurations opt-in - #4158
Draft
ppkarwasz wants to merge 8 commits into
Draft
Conversation
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
ppkarwasz
marked this pull request as draft
June 26, 2026 16:34
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) <noreply@anthropic.com>
Member
Author
|
The Now that XInclude is opt-in, that dance is no longer needed: if a user's JAXP parser does not support XInclude, they simply don't enable |
…e 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 <noreply@anthropic.com>
This was referenced Jun 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR makes XInclude processing in XML configurations opt-in: it is disabled by default and enabled with the
log4j2.configurationEnableXIncludeproperty.Note
Stacked on #4155 (
fix/2.x/xinclude-fixup-attributes), which is the base of this PR, so the diff shown here is only the opt-in change.Please review/merge that one first.
Behavior
XmlConfigurationreadslog4j2.configurationEnableXIncludein its constructor and configures theDocumentBuilderaccordingly. XInclude is no longer always on, and the previous LOG4J2-1127 "retry without XInclude" fallback is removed.xi:includeelements are left unexpanded and ignored, so a configuration that relied on them falls back to the default setup.Documentation
Tests
ConfigurationFactoryTest.xincludeenables the property, andtestGetConfigurationWithMultipleUrisno longer incidentally loads an XInclude file.ConfigurationFactoryUnitTestloads the same logical configuration as XML, JSON, YAML, Java properties, an XInclude-composed file (with the property enabled) and a cross-format (XML + YAML) composite, asserting through theConfigurationinterface that each yields the same components. It also covers the XInclude-disabled fallback to the default configuration.Checklist
2.x(stacked on Revamp XML configuration testing and documentation #4155)./mvnw verifysucceeds (only the affected modules were tested locally)src/changelog/.2.x.x