jmestwa-coder commented on code in PR #4198:
URL: https://github.com/apache/logging-log4j2/pull/4198#discussion_r3820180346
##########
log4j-1.2-api/src/main/java/org/apache/log4j/xml/Log4jEntityResolver.java:
##########
@@ -46,6 +48,7 @@ public InputSource resolveEntity(final String publicId, final
String systemId) {
}
return new InputSource(in);
}
- return null;
+ LOGGER.warn("Ignoring external resource with public ID [{}] and system
ID [{}].", publicId, systemId);
+ return new InputSource(new
ByteArrayInputStream(Constants.EMPTY_BYTE_ARRAY));
Review Comment:
fair point, agreed there's little to gain from forbidding it outright.
switched to the #4161 route:
- `log4j.dtd` is still served from the jar
- everything else goes through `ConfigurationSource.fromUri`, so
`file`/`https`/`jar` keep working as before and anything else needs an opt-in
via `log4j2.Configuration.allowedProtocols`
- unresolvable resources fall back to an empty source with a status warning
updated the test to cover both sides (a `file` entity still resolves, an
`http` one is ignored with `file` as the only allowed protocol), and renamed it
and the PR accordingly since it's no longer about disabling anything.
--
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]
