Keep the resolver floor on a reader obtained before reset() - #72
Merged
Merged
Conversation
A JAXP parser hands out one reader for its lifetime, and reset() reverts that reader to its just-created state, which removes the floor the wrapper installed after creation. SecureSAXParser only dropped its cached views, so the floor came back on the next getXMLReader() call while a reader a caller already held kept parsing without one. Re-secure the reader in place instead, through the view already vended, as SecureDocumentBuilder.reset() has always done. Keeping one wrapper also keeps a caller resolver set on that view routed through the floor that is actually installed. ResetSecureTest covers the retained reader; the SecureSAXParser unit test now asserts the views survive a reset with the floor in place, rather than that they are recreated. Assisted-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CLnTBsvmYtxzNTWVGNyz33
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
2 participants
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.
A JAXP
SAXParserhands out oneXMLReaderfor its lifetime — both the stock JDK and Xerces hold it in afinalfield and return that instance from everygetXMLReader()call — andreset()reverts it to its just-created state, restoring the initial (resolver-less) entity resolver captured before the floor existed.SecureSAXParser.reset()responded by dropping its cached views, so the floor was re-installed on the nextgetXMLReader()call. A caller that kept the reader it had already obtained — which is the casereset()exists for, instance reuse — went on parsing through that same underlying reader with no floor on it. The existingResetSecureTestcase materialised a reader before the reset but then re-fetched it afterwards, so it exercised the re-secured view rather than the retained one.The fix re-secures the reader in place, through the view already vended, matching the eager pattern
SecureDocumentBuilder.reset(),SecureTransformer.reset()andSecureValidator.reset()already use. Keeping a single wrapper also avoids orphaning its floor: a resolver the caller sets on that view stays routed through the floor that is actually installed.Tests:
ResetSecureTest.saxParserResetKeepsFloorOnReaderVendedBeforeReset— holds the reader across the reset and parses through it. It fails on both thetest-stockjdkandtest-xercesexecutions without the main-code change, and passes with it.SecureSAXParserTest— the reset case now asserts the security property (the views survive the reset and the floor is back on the underlying reader) instead of the previous mechanism (that the views are recreated), and is renamed accordingly.Verified with the full surefire matrix and the GraalVM
-Pnative-xalannative suite. Nochanges.xmlentry: the behaviour lands in the same release cycle that introduced it.🤖 Generated with Claude Code