garydgregory commented on code in PR #60:
URL: https://github.com/apache/commons-xml/pull/60#discussion_r3887207486
##########
src/test/java/org/apache/commons/xml/XMLFilterTest.java:
##########
@@ -87,6 +97,19 @@ void secureFilterDoesNotLeakExternalEntity() throws
Exception {
assertFalse(filterAndCapture(filter,
entityPayload()).contains(AttackTestSupport.LEAKED_MARKER), "external entity
through XMLFilter leaked");
}
+ @Test
+ void secureFilterDoesNotReWrapParseError() throws Exception {
+ // Dual contract: a malformed input's SAXParseException is swallowed
(Xalan), hidden inside an implementation wrapper (XSLTC), or surfaces; when the
+ // cause chain carries it, parse must rethrow it directly rather than
bury it under a fresh SAXException.
+ final XMLFilter filter =
SaxSurfaceTestSupport.secureFactory().newXMLFilter(AttackTestSupport.streamSource(IDENTITY_XSLT));
+ filter.setContentHandler(AttackTestSupport.capturingHandler(new
StringBuilder()));
+ try {
+ filter.parse(new InputSource(new StringReader("<root>")));
Review Comment:
This test passes if SAXException is NOT thrown, which would be a bug, which
is means this test should fail but doesn't because it doesn't use
`assertThrows()` when calling `parse()`.
##########
src/test/java/org/apache/commons/xml/XMLFilterTest.java:
##########
@@ -96,6 +119,25 @@ void secureFilterFromTemplatesDoesNotLeakDocument() throws
Exception {
assertFalse(filterAndCapture(filter,
"<root/>").contains(AttackTestSupport.LEAKED_MARKER), "document() through
XMLFilter(Templates) leaked");
}
+ @Test
+ void secureFilterRethrowsHandlerSAXException() throws Exception {
+ // Dual contract: the delegate transformer either swallows the
handler's exception (Xalan) or surfaces it wrapped in a TransformerException;
when it
+ // surfaces, parse must rethrow the original instance, not nest it
under a new SAXException.
+ final XMLFilter filter =
SaxSurfaceTestSupport.secureFactory().newXMLFilter(AttackTestSupport.streamSource(IDENTITY_XSLT));
+ final SAXException handlerFailure = new SAXException("handler
failure");
+ filter.setContentHandler(new DefaultHandler() {
+ @Override
+ public void startDocument() throws SAXException {
+ throw handlerFailure;
+ }
+ });
+ try {
Review Comment:
This test passes if SAXException is NOT thrown, which would be a bug, which
is means this test should fail but doesn't because it doesn't use
`assertThrows()` when calling `parse()`.
--
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]
