ppkarwasz commented on code in PR #766:
URL: https://github.com/apache/commons-text/pull/766#discussion_r3955557639
##########
src/test/java/org/apache/commons/text/lookup/XmlStringLookupTest.java:
##########
@@ -50,6 +51,19 @@ class XmlStringLookupTest {
private static final String DOC_RELATIVE = DOC_DIR + "document.xml";
private static final String DOC_ROOT = "/document.xml";
+ /**
+ * Asserts the secure contract for an external reference: the parser
either rejects the document or parses it
+ * with the reference resolved to empty content, but the external content
never appears in the result.
+ */
+ static void assertBlocksOrDoesNotLeak(final Supplier<String> lookup, final
String external) {
+ try {
+ final String result = lookup.get();
+ assertFalse(result != null && result.contains(external), () ->
"external content leaked: " + result);
+ } catch (final IllegalArgumentException e) {
Review Comment:
Fixed in
https://github.com/apache/commons-text/pull/766/commits/bc9c20447a11dfbde21defdcc7713a62a0ba7ee8
`assertBlocksOrDoesNotLeak` intentionally does not use `assertThrows()`: we
really don't care whether parsing an XML document referencing an external
entity throws. The important part is that the content is not fetched.
In the refactored version I replaced it with `asserDoesNotLeak`, so throwing
is no longer an option.
--
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]
