fix: honor message charset in XMLUtil.getInputSource - #3142
Conversation
Previously the body was pre-decoded with an InputStreamReader using the JVM default charset, so both the message's declared Content-Type charset and the XML encoding declaration were ignored. Non-UTF-8 bodies with non-ASCII characters could be misdecoded, e.g. silently breaking XPath comparisons in XPathExchangeExpression and the xpath() built-in function. Pass the byte stream to the parser instead and set InputSource encoding from the message's charset when present, following RFC 7303 precedence. Fixes #3129 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesXML charset parsing
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/src/test/java/com/predic8/membrane/core/util/XMLUtilTest.java`:
- Around line 52-55: Update the header-charset fixture in XMLUtilTest so its XML
body omits the encoding declaration while retaining the ISO-8859-1 Content-Type
charset and encoded text. Leave the XML declaration in the separate fallback
test unchanged, then run the targeted XMLUtilTest command.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d0ca1f4e-a6f0-4b32-8d60-8fac4d84ae72
📒 Files selected for processing (2)
core/src/main/java/com/predic8/membrane/core/util/xml/XMLUtil.javacore/src/test/java/com/predic8/membrane/core/util/XMLUtilTest.java
|
This pull request needs "/ok-to-test" from an authorized committer. |
|
/ok-to-test |
Problem
XMLUtil.getInputSourcedecoded the message body with anInputStreamReaderusing the JVM default charset, so:Content-Typecharset was ignored (-Dfile.encodingdecided instead).encodingdeclaration had no effect either, since per the XML spec it's only consulted when the parser is fed a byte stream, not a pre-decoded character stream.This affects both consumers of
XMLUtil.getInputSource:XPathExchangeExpression(xpath configuration) and thexpath()built-in function. A non-UTF-8 body with non-ASCII characters could be misdecoded, silently breaking xpath comparisons.Fix
Pass the byte stream straight to the
InputSourceand set its encoding frommsg.getHeader().getCharset()when the header declares one, letting the parser fall back to its own BOM/declaration detection otherwise — following RFC 7303 precedence.Tests
Added to
XMLUtilTest:getInputSourceHonorsTheMessagesCharset— reproduces the issue's ISO-8859-1Bönnigheimcase with a header charset.getInputSourceFallsBackToTheXmlDeclarationWithoutAHeaderCharset— same body with no header charset, confirming the XML declaration still decides.Fixes #3129
🤖 Generated with Claude Code
Summary by CodeRabbit