Skip to content

Revamp XML configuration testing and documentation#4155

Open
ppkarwasz wants to merge 4 commits into
2.xfrom
fix/2.x/xinclude-fixup-attributes
Open

Revamp XML configuration testing and documentation#4155
ppkarwasz wants to merge 4 commits into
2.xfrom
fix/2.x/xinclude-fixup-attributes

Conversation

@ppkarwasz

@ppkarwasz ppkarwasz commented Jun 26, 2026

Copy link
Copy Markdown
Member

This PR reworks how the XML configuration format is tested and documented so that each behavior of the DOM-to-Node conversion in XmlConfiguration is covered by its own focused test. Doing so surfaced a small attribute-handling bug, fixed here.

Testing

  • constructHierarchy, getType and processAttributes are now static and take the PluginManager and strict flag as parameters, so the DOM-to-Node conversion can be unit-tested with a mocked PluginManager instead of spinning up a full LoggerContext.
  • The StrictXmlConfigTest integration test (which only asserted event counts and merely happened to exercise strict-mode parsing) is replaced by XmlConfigurationConstructHierarchyTest, covering individually: plugin resolution, attribute folding, text-as-value, strict-mode type resolution, and XInclude whole-file / positional element() / by-name (ID) fragment resolution.
  • The unused Status/ErrorType bookkeeping is dropped in favor of logging the error directly.

Bug fix

  • The XInclude fixup-language feature adds an xml:lang attribute to included elements that was never stripped (only xml:base was) and was not covered by any test, so it leaked through as a Log4j configuration attribute. processAttributes now matches the reserved XML namespace instead of the literal attribute name, stripping xml:base, xml:lang, and any other reserved xml: attribute.
  • XmlConfiguration no longer explicitly enables the fixup-base-uris/fixup-language features: they default to true and only
    add attributes Log4j strips (they are not required for nested or relative includes to resolve).

Documentation

  • The XML format section of configuration.adoc is moved into a reusable AsciiDoc partial (partials/manual/configuration-xml-format.adoc).
  • The XInclude docs gain a section about including fragments with positional and by-name XPointer examples, plus a note that the JDK does not recognize xml:id and does not implement the xpointer() scheme.

There is no behavior change for valid configurations; the only runtime difference is that xml:lang no longer leaks into the configuration.

Checklist

  • Based on 2.x
  • ./mvnw verify succeeds (only the affected modules were tested locally)
  • Changelog entry in src/changelog/.2.x.x
  • Tests provided

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

1 participant