XML Parser Hardening v6#3078
Conversation
- Introduced `HardenedSaxParser` and `HardenedSchemaFactory` for XXE-protected XML parsing and schema validation. - Updated existing validators and utilities to leverage hardened parsers. - Added comprehensive test coverage to ensure external entity prevention across parsing and validation workflows.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesThe XML processing paths now use hardened SAX and schema factories, block external DTD access, detect external DTD subsets in XML security hardening
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SOAPRequest
participant XMLSchemaValidator
participant HardenedSaxParser
participant HardenedSchemaFactory
participant Validator
SOAPRequest->>XMLSchemaValidator: provide XML message body
XMLSchemaValidator->>HardenedSaxParser: create hardened XMLReader
XMLSchemaValidator->>HardenedSchemaFactory: create hardened schema factory
HardenedSchemaFactory->>Validator: configure validator
XMLSchemaValidator->>Validator: validate SAXSource
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
This pull request needs "/ok-to-test" from an authorized committer. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core/src/test/java/com/predic8/membrane/core/interceptor/schemavalidation/SOAPUtilTest.java (1)
32-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider extracting shared test helpers into a dedicated utility.
freePort/startRecordingServerare consumed here and inXMLSchemaValidatorTestandSoapOperationExtractorTestvia static imports fromHardenedSaxParserTest. Depending on another test class couples unrelated test lifecycles; moving these into a small shared test-support class would decouple them.🤖 Prompt for 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. In `@core/src/test/java/com/predic8/membrane/core/interceptor/schemavalidation/SOAPUtilTest.java` around lines 32 - 33, Extract freePort and startRecordingServer from HardenedSaxParserTest into a dedicated shared test-support utility, then update SOAPUtilTest, XMLSchemaValidatorTest, and SoapOperationExtractorTest to use static imports from that utility instead. Remove the cross-test-class dependency while preserving the helpers’ behavior and visibility.
🤖 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.
Nitpick comments:
In
`@core/src/test/java/com/predic8/membrane/core/interceptor/schemavalidation/SOAPUtilTest.java`:
- Around line 32-33: Extract freePort and startRecordingServer from
HardenedSaxParserTest into a dedicated shared test-support utility, then update
SOAPUtilTest, XMLSchemaValidatorTest, and SoapOperationExtractorTest to use
static imports from that utility instead. Remove the cross-test-class dependency
while preserving the helpers’ behavior and visibility.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 27a9ec20-c5d8-4b80-a8d9-446ebb5301a5
📒 Files selected for processing (14)
core/src/main/java/com/predic8/membrane/core/interceptor/schemavalidation/AbstractXMLSchemaValidator.javacore/src/main/java/com/predic8/membrane/core/interceptor/schemavalidation/XMLSchemaValidator.javacore/src/main/java/com/predic8/membrane/core/interceptor/soap/SoapOperationExtractor.javacore/src/main/java/com/predic8/membrane/core/interceptor/xmlprotection/XMLProtector.javacore/src/main/java/com/predic8/membrane/core/util/MessageUtil.javacore/src/main/java/com/predic8/membrane/core/util/SOAPUtil.javacore/src/main/java/com/predic8/membrane/core/util/xml/parser/HardenedSaxParser.javacore/src/main/java/com/predic8/membrane/core/util/xml/parser/HardenedSchemaFactory.javacore/src/test/java/com/predic8/membrane/core/interceptor/schemavalidation/SOAPUtilTest.javacore/src/test/java/com/predic8/membrane/core/interceptor/schemavalidation/XMLSchemaValidatorTest.javacore/src/test/java/com/predic8/membrane/core/interceptor/soap/SoapOperationExtractorTest.javacore/src/test/java/com/predic8/membrane/core/interceptor/xmlprotection/XMLProtectorTest.javacore/src/test/java/com/predic8/membrane/core/util/xml/parser/HardenedSaxParserTest.javacore/src/test/java/com/predic8/membrane/core/util/xml/parser/HardenedSchemaFactoryTest.java
|
/ok-to-test |
2 participants
Summary by CodeRabbit
Security Enhancements
Tests