Copilot commented on code in PR #435:
URL:
https://github.com/apache/jackrabbit-filevault/pull/435#discussion_r4211377942
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/DocViewFormat.java:
##########
@@ -147,16 +148,11 @@ private byte[] format(File file, Checksum original,
Checksum formatted) throws I
FormattingXmlStreamWriter writer =
FormattingXmlStreamWriter.create(out, format)) {
// cannot use XMlStreamReader due to comment handling:
//
https://stackoverflow.com/questions/15792007/why-does-xmlstreamreader-staxsource-strip-comments-from-xml
- TransformerFactory tf = TransformerFactory.newInstance();
- tf.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
- tf.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
+ TransformerFactory tf = SecureTransformerFactory.newInstance();
SAXSource saxSource = new SAXSource(new InputSource(in));
- SAXParserFactory sf = SAXParserFactory.newInstance();
- sf.setNamespaceAware(true);
+ SAXParserFactory sf = SecureSAXParserFactory.newNSInstance();
Review Comment:
DocViewFormatTest checks formatting output but does not exercise
external-resource blocking. This migration removes the explicit SAX and
transformer restrictions, so add a regression through DocViewFormat.format
using external DTD/entity references. Assert that the pipeline makes no
external-resource access and does not include entity contents in the output; a
parsing failure alone does not prove that no access occurred.
##########
vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/AdvancedFilterValidatorFactory.java:
##########
@@ -83,20 +85,18 @@ public AdvancedFilterValidatorFactory() throws IOException {
}
static @NotNull DocumentBuilderFactory
createFilterXsdAwareDocumentBuilder(Locale locale) throws IOException {
- DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+ DocumentBuilderFactory factory =
SecureDocumentBuilderFactory.newInstance();
Review Comment:
The schema-aware factory now relies on Commons Secure XML instead of
explicit external-access restrictions, but AdvancedFilterValidatorTest has no
external-resource regression cases. Add tests through validateMetaInfData with
external DTD/entity references and external schema locations, asserting that no
external resource is resolved after the bundled schema is attached. Keep the
existing valid/invalid filter assertions to verify that schema validation still
works.
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/DocViewParser.java:
##########
@@ -230,13 +230,9 @@ private static boolean isDocView(Reader reader) throws
IOException {
}
private SAXParser createSaxParser() throws ParserConfigurationException,
SAXException {
- SAXParserFactory factory = SAXParserFactory.newInstance();
- factory.setNamespaceAware(true);
+ SAXParserFactory factory = SecureSAXParserFactory.newNSInstance();
Review Comment:
The DocView parsing tests do not cover external-resource blocking after the
explicit ACCESS_EXTERNAL_* settings are removed. Add an end-to-end case through
DocViewParser and DocViewSAXHandler with external DTD/entity references,
asserting that no external resource is accessed. Exercise the actual parse call
rather than just the factory: SAXParser.parse(inputSource, handler) installs
the handler as the entity resolver, so this integration needs coverage too.
--
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]
