sharanggupta opened a new pull request, #4397: URL: https://github.com/apache/logging-log4j2/pull/4397
Fixes #4349. The Javadoc of `MapMessage` and `StructuredDataMessage` says the default format follows RFC 5424, but `MapMessage.appendMap` appended each value without the escaping that [RFC 5424, section 6.3.3](https://datatracker.ietf.org/doc/html/rfc5424#section-6.3.3) requires for a `PARAM-VALUE`. A value such as `a" other="b` was rendered as `key="a" other="b"`, which reads as two entries, and a `]` in a value ended the `StructuredDataMessage` element `[id ...]` early. This takes the first option of the issue: `appendMap` now escapes `"`, `\` and `]` in values with a backslash. `StructuredDataMessage` uses the same method, so it is fixed too. The XML, JSON and Java formats, `Rfc5424Layout` (which formats structured data itself) and keys are unchanged. The Javadoc now says what the default format does. The escaping is done in place after the value is appended, in a single linear pass, so a value full of quotes cannot make formatting quadratic. The helper is private, so there is no public API change. Behaviour change worth a reviewer's eye, inherent to what the RFC requires: output changes for values that contain these characters. For example a Windows path `C:\temp` is now rendered as `C:\\temp`, and a `]` in a value, including the closing bracket of a nested list or array value such as `[1, 2]`, is rendered as `\]`. ## Tests Five cases added; they fail on `2.x` and pass with this change, except the last `MapMessageTest` one, which pins that nothing else is escaped: * `MapMessageTest.testDefaultFormatEscapesQuotes`: the case from the issue, `a" other="b` yields `key="a\" other=\"b"`. * `MapMessageTest.testDefaultFormatEscapesBackslashes` * `MapMessageTest.testDefaultFormatEscapesClosingBrackets` * `MapMessageTest.testDefaultFormatLeavesOtherCharactersUntouched` (passes before and after) * `StructuredDataMessageTest.testMsgEscapesParamValues`: `]`, `"` and `\` in a value cannot close the element or the value early. ## Verification * `./mvnw -pl :log4j-api-test -am verify` (including spotless and the API checks) is green: 959 tests, 0 failures. * The `log4j-core-test` classes that render these messages (the Syslog, `Rfc5424Layout`, `MapRewritePolicy`, `StructuredData*`, `MapPattern*`, `MessagePattern*`, `CollectionLogging` and routing tests, 154 tests in the default Surefire execution, 0 failures, 4 skipped) pass. * I did not run the complete `./mvnw verify` of the whole reactor locally and rely on CI for it. Generative-AI disclosure (ASF guidance): Claude Code (Anthropic) assisted in drafting the change, tests and this description; the result was reviewed, the new tests were confirmed to fail on `2.x` and pass with the change, and the tests and formatting checks above were run before opening this PR. ## Checklist * [x] Base your changes on `2.x` branch if you are targeting Log4j 2; use `main` otherwise * [ ] `./mvnw verify` succeeds: not run for the whole reactor locally, see Verification above * [x] Non-trivial changes contain an entry file in the `src/changelog/.2.x.x` directory * [x] Tests are provided (`MapMessageTest`: 4 new tests, `StructuredDataMessageTest`: 1 new test) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
