Fix encoding of MSGID and SD-ID fields of StructuredDataMessage to XML#4136
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes XML output generation for StructuredDataMessage by ensuring the <type> (MSGID) and <id> (SD-ID) element contents are properly XML-escaped, preventing malformed XML when these fields contain reserved characters or invalid XML code points.
Changes:
- Escape XML for
StructuredDataMessage’s<type>and<id>fields usingStringBuilders.escapeXml(...). - Add regression tests covering reserved XML characters, quotes, non-BMP characters, and invalid XML characters in
typeandid. - Add a changelog entry documenting the fix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/changelog/.2.x.x/fix-StructuredDataMessage-XML-encoding.xml |
Documents the XML encoding fix in the 2.x.x changelog stream. |
log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java |
Escapes <type> and <id> values in XML formatting to produce valid XML. |
log4j-api-test/src/test/java/org/apache/logging/log4j/message/StructuredDataMessageTest.java |
Adds tests ensuring type/id XML encoding and invalid character handling are correct. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ppkarwasz
approved these changes
Jun 2, 2026
ppkarwasz
left a comment
ppkarwasz
left a comment
Member
There was a problem hiding this comment.
LGTM! 💯
I guess you can not create test cases with control characters forbidden in XML 1.0, because StructuredDataMessage will throw, right?
Member
Author
No, it doesn't, yet. It will start throwing after #4103. |
github-project-automation
Bot
moved this from Approved
to Merged
in Log4j pull request tracker
ramanathan1504
pushed a commit
to ramanathan1504/logging-log4j2
that referenced
this pull request
Jul 9, 2026
ramanathan1504
added a commit
that referenced
this pull request
Jul 24, 2026
#4118) * Fix NPE in ConfigurationScheduler by ensuring scheduledFuture is checked for null before accessing its methods; add test for race condition in scheduling. * Restrict JUnit dependencies to below version 6.x to maintain compatib… (#4120) * Restrict JUnit dependencies to below version 6.x to maintain compatibility with Java 8 * Update JUnit dependency restrictions for compatibility with Java 8 on 2.x * Don't override `apache-rat-plugin` version (#4123) Remove the version override for `apache-rat-plugin` so it falls back to the version provided by the ASF Parent POM. Apache RAT `0.18`, for which Dependabot opened an update PR, contains a bug ([RAT-552](https://issues.apache.org/jira/browse/RAT-552)) that effectively disables the `excludeSubProjects` plugin option whenever `<excludes>` is present in the configuration. Upgrading to `0.18` will require converting our `<excludes>` to `<inputExcludes>`, which depends on a new `logging-parent` release. * Bump actions/stale from 9.1.0 to 10.2.0 in the dependencies group across 1 directory (#4121) * Bump actions/stale in the dependencies group across 1 directory Bumps the dependencies group with 1 update in the / directory: [actions/stale](https://github.com/actions/stale). Updates `actions/stale` from 9.1.0 to 10.2.0 - [Release notes](https://github.com/actions/stale/releases) - [Changelog](https://github.com/actions/stale/blob/main/CHANGELOG.md) - [Commits](actions/stale@5bef64f...b5d41d4) --- updated-dependencies: - dependency-name: actions/stale dependency-version: 10.2.0 dependency-type: direct:production update-type: version-update:semver-major dependency-group: dependencies ... Signed-off-by: dependabot[bot] <support@github.com> * Generate changelog entries for #4121 --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Fix KafkaAppender reporting error after successful retry (#4125) * Fix KafkaAppender reporting error after successful retry When retryCount is configured and the initial tryAppend() fails, the retry loop uses break to exit on success. However, break only exits the while loop and execution always reaches the error() call afterward. This causes spurious error notifications for transient Kafka failures that were successfully recovered by a retry. Replace break with return so that a successful retry exits append() without reporting an error. Retry exceptions are now logged at DEBUG level for diagnostics instead of being silently discarded. Also remove dead code in Builder.getRetryCount() where Integer.valueOf(int) was wrapped in a NumberFormatException catch that can never fire. The bug was introduced in #315. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca> * Add changelog entry for KafkaAppender retry fix Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca> * Use CloseableThreadContext in testRetrySuccessDoesNotReportError Adopt reviewer suggestion to use try-with-resources CloseableThreadContext instead of manual ThreadContext.put/clearMap. --------- Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca> * Revamped `ListAppender` for thread-safety (#4111) Co-authored-by: Björn Michael <b.michael@gmx.de> Co-authored-by: Volkan Yazıcı <volkan.yazici@oracle.com> * Improve `CronExpression` tests to cover daylight saving and scheduling logic (#4081) Co-authored-by: Piotr P. Karwasz <pkarwasz-github@apache.org> Co-authored-by: Volkan Yazıcı <volkan@yazi.ci> * Fix `createOnDemand` for Rolling File Appender (#4072) Co-authored-by: Piotr P. Karwasz <pkarwasz-github@apache.org> Co-authored-by: Volkan Yazıcı <volkan@yazi.ci> * Add support for max compression delay in compression actions (#4071) Co-authored-by: Volkan Yazıcı <volkan@yazi.ci> * Improve logging for `LinkageError` scenarios involving the LMAX Disruptor library (#4124) Co-authored-by: Volkan Yazıcı <volkan@yazi.ci> * Fix changelog issue and PR references for Disruptor initialization error logging (#2250, #4124, #4134) * Fix encoding of `MSGID` and `SD-ID` fields of `StructuredDataMessage` to XML (#4136) * Fix stack trace rendering for exceptions with identity malfunction (#4133) * Fix circular reference detection for exceptions with colliding equals/hashCode implementations * Add changelog entry for circular reference detection fix with colliding equals/hashCode exceptions * Add changelog entry for circular reference detection fix with colliding equals/hashCode exceptions * Add test for ThrowableProxy serialization with colliding equals/hashCode implementations * Refactor ThrowableProxy serialization test to use modern Java I/O classes * Use `TestFriendlyException` to exercise the malfunction * Update changelog * Improve comments * Remove redundant change --------- Co-authored-by: Volkan Yazıcı <volkan@yazi.ci> * Remove the `patternFlags` attribute from `RegexFilter` (#4119) Co-authored-by: Jeff Thomas <jeffrey.thomas@t-systems.com> Co-authored-by: Volkan Yazıcı <volkan@yazi.ci> * Fix `DatePatternConverter` locale parsing when timezone is omitted (#4130) * Harden `readObject(ObjectInputStream)` method argument checks (#4098) Signed-off-by: SunWeb3Sec <infosecpt@gmail.com> Co-authored-by: Volkan Yazıcı <volkan@yazi.ci> * Fix resource leaks in `ConfigurationSource` when loading via URL (#4127) Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca> Co-authored-by: Ramanathan <ramanathanbscmca@gmail.com> Co-authored-by: Piotr P. Karwasz <pkarwasz-github@apache.org> Co-authored-by: Volkan Yazıcı <volkan@yazi.ci> * Adopt the Dependabot changelog 'draft trick' (#4148) Applies the consumer-side changes from logging-parent PR #476: * `build.yaml` and `codeql-analysis.yaml` now subscribe to the `ready_for_review` pull request type, so required checks re-run when a Dependabot PR is taken out of draft. * `process-dependabot.yaml` drops the `RECURSIVE_TOKEN` PAT secret, which is no longer needed now that the reusable workflow parks the PR in draft mode instead of pushing with a privileged token. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add Dependabot config for the 2.26.x maintenance branch (#4149) Configure a Maven update entry targeting the `2.26.x` branch that only proposes patch-level upgrades. All dependencies are bundled into a single `Maven patch updates` group with a 7-day cooldown. * Fix handling of non-finite numbers while encoding `MapMessage` to JSON (#4163) * Remove `/log4j-slf4j-impl` Dependabot execution (#4151) Instead of correcting the target branch, we can safely remove it: the only dependency version defined in `log4j-slf4j-impl` is SLF4J 1.x, which is highly unlikely to see any new releases. * Upgrade Maven and Maven Wrapper versions to `3.9.16` and `3.3.4`, respectively (#4167) * Adds `maxRandomDelay` configure to Rolling Appenders (#4165) * Rework random delay for Rolling Appender action chain * Stabilize `RollingAppenderDirectCronTest` * Merge `2.26.1` to `2.x` (#4169) * Add `2.25.5` release notes (#4174) * Add native tracing fields to LogEvent to eliminate async MDC overhead (#4171) * Add tracing fields to RingBufferLogEvent and related classes * Add W3C trace context support with pattern converters for trace ID, span ID, and trace flags * Enhance TraceContextProviderService for exception safety and simplify trace ID retrieval * Add tests for tracing fields serialization and pattern converters * Refactor tracing metadata documentation and improve code comments for clarity * Add native W3C tracing fields to LogEvent and introduce TraceContextProvider SPI * Add benchmark for ContextDataProvider tracing approach * Enhance TraceContextProviderService to handle SecurityManager restrictions gracefully * Fix NPE in ConfigurationScheduler by ensuring proper reset of scheduled futures * Fix NPE in ConfigurationScheduler for frequent cron schedules --------- Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca> Signed-off-by: SunWeb3Sec <infosecpt@gmail.com> Co-authored-by: Piotr P. Karwasz <pkarwasz-github@apache.org> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Sebastien Tardif <SebTardif@ncf.ca> Co-authored-by: Björn Michael <b.michael@gmx.de> Co-authored-by: Volkan Yazıcı <volkan.yazici@oracle.com> Co-authored-by: Volkan Yazıcı <volkan@yazi.ci> Co-authored-by: Jeff Thomas <jeffrey.thomas@t-systems.com> Co-authored-by: SunWeb3Sec <107249780+SunWeb3Sec@users.noreply.github.com>
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
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.
No description provided.