Repository navigation
Fix merging of structured data elements with the same SD-ID in Rfc5424Layout - #4420
Open
MaxFreedomPollard wants to merge 2 commits into
Open
MaxFreedomPollard wants to merge 2 commits into
MaxFreedomPollard wants to merge 2 commits into
Conversation
…4Layout `Rfc5424Layout.addStructuredData` merges a `StructuredDataMessage` into an element that already has the same SD-ID, such as the MDC element, a `LoggerFields` element or an earlier message of a `StructuredDataCollectionMessage`. It found the element by the SD-ID computed with `getId`, but then fetched it with `StructuredDataId.toString()`. For an ID without an enterprise number `getId` appends the layout's enterprise number and `toString()` does not, so the lookup returned `null` and the merge threw a `NullPointerException`. Elements created from a `StructuredDataMessage` also kept the unmodifiable map returned by `MapMessage.getData()`, so merging a second message with the same SD-ID into one threw an `UnsupportedOperationException`. Look the element up by the computed SD-ID and copy the fields when merging. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MaxFreedomPollard
force-pushed
the
fix/rfc5424-merge-structured-data
branch
from
October 22, 2026 22:34
eaba60d to
fbf551c
Compare
| type="fixed"> | ||
| <issue id="4420" link="https://github.com/apache/logging-log4j2/pull/4420"/> | ||
| <description format="asciidoc"> | ||
| Fix `Rfc5424Layout` throwing an exception when merging structured data elements with the same SD-ID. |
Contributor
There was a problem hiding this comment.
The description is 100 characters. A shorter line reads better in the release notes:
Suggested change
| Fix `Rfc5424Layout` throwing an exception when merging structured data elements with the same SD-ID. | |
| Fix `Rfc5424Layout` failing on two SD elements with one SD-ID. |
Contributor
|
Looking up the element by |
Use the wording suggested in review. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
Thanks, I've applied your suggested wording to the changelog entry. |
ramanathan1504
enabled auto-merge (squash)
October 29, 2026 06:31
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.
What breaks
Rfc5424Layoutmerges aStructuredDataMessageinto an element that already uses the same SD-ID: the MDC element, aLoggerFieldselement, or an earlier message of aStructuredDataCollectionMessage. That merge path throws instead of merging:With
"Transfer@32473"as the ID it throwsUnsupportedOperationExceptioninstead. A singlenew StructuredDataMessage("mdc", ...)logged with a non-empty thread context also throws theNullPointerException, because its SD-ID matches the default MDC element.Cause
Two problems in the merge path of
log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java:addStructuredDatacheckssdElements.containsKey(sdId), wheresdIdcomes fromgetId(id), but then fetches the element withsdElements.get(id.toString()). For an ID without an enterprise number,getIdappends the layout's enterprise number (Transfer@32473) andStructuredDataId.toString()does not (Transfer), so the lookup returnsnull.StructuredDataMessagekeeps the map returned byMapMessage.getData(), which is unmodifiable, soStructuredDataElement.unionfails onputAllwhen a second message with the same SD-ID is merged into it.Fix
Fetch the element by
sdId, and letStructuredDataElement.unioncopy the fields into a newHashMapbefore adding the new ones. The copy only happens when two elements are merged. The same code is onmain.Testing
Two tests in
Rfc5424LayoutTest:testCollectionMergesElementsWithSameId, parameterized withTransferandTransfer@32473, expects[Transfer@32473 Amount="200.00" ToAccount="123456"].testMdcElementMergesStructuredDataWithSameIdexpects[mdc@32473 Amount="200.00" loginId="JohnDoe"].I could not run Maven in my environment, so
./mvnw verify(including Spotless) and the real JUnit run have not been done. Instead I compiledRfc5424LayoutandRfc5424LayoutTestwithjavac(JDK 17) against thelog4j-api,log4j-coreandlog4j-core-testsources. JUnit and AssertJ were replaced by small stand-ins, and theLog4j2Plugins.datwas generated withPluginProcessor. I then ran every test method ofRfc5424LayoutTestthrough a reflection-based runner. On2.xat 8d7d3c5, 35 of 38 invocations pass, and the 3 failures are the new ones (the twoNullPointerExceptions and theUnsupportedOperationExceptionabove). With the fix, all 38 pass.Checklist
2.xbranch if you are targeting Log4j 2; usemainotherwise./mvnw verifysucceeds (the build instructions): not run, see Testingsrc/changelog/.2.x.xdirectory🤖 Generated with Claude Code