[ 
https://issues.apache.org/jira/browse/TIKA-4935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18119816#comment-18119816
 ] 

ASF GitHub Bot commented on TIKA-4935:
--------------------------------------

ppkarwasz commented on code in PR #3261:
URL: https://github.com/apache/tika/pull/3261#discussion_r4115221029


##########
tika-core/src/main/java/org/apache/tika/utils/XMLReaderUtils.java:
##########
@@ -304,66 +285,22 @@ public static DocumentBuilder getDocumentBuilder() throws 
TikaException {
      * @since Apache Tika 1.13
      */
     public static XMLInputFactory getXMLInputFactory() {
-        XMLInputFactory factory = XMLInputFactory.newFactory();
+        XMLInputFactory factory = SecureXMLInputFactory.newFactory();
         if (LOG.isDebugEnabled()) {
             LOG.debug("XMLInputFactory class {}", factory.getClass());
         }
 
         tryToSetStaxProperty(factory, XMLInputFactory.IS_NAMESPACE_AWARE, 
true);
 
         //try to configure secure processing
-        tryToSetStaxProperty(factory, XMLConstants.ACCESS_EXTERNAL_DTD, "");
         tryToSetStaxProperty(factory, XMLInputFactory.IS_VALIDATING, false);
         tryToSetStaxProperty(factory, XMLInputFactory.SUPPORT_DTD, false);
         tryToSetStaxProperty(factory, 
XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
 
-        //defense in depth
-        factory.setXMLResolver(IGNORING_STAX_ENTITY_RESOLVER);

Review Comment:
   All the `tryToSetStaxProperty` calls can also be removed:
   
   - `IS_NAMESPACE_AWARE` is `true` by default (mandated by the StAX 
specification),
   - `IS_VALIDATING` is `false` by default,
   - `SUPPORT_DTD` and `IS_SUPPORTING_EXTERNAL_ENTITIES` would be 
“defense-in-depth”: they disable DTD **before** it is passed on to the resolver.
   - The resolver was removed, because Commons Secure XML installs its own.
   - `ACCESS_EXTERNAL_DTD` will never be triggered, because resources opted-in 
by a resolver are not checked against this property. 





> Delegate JAXP parser configuration to Apache Commons Secure XML
> ---------------------------------------------------------------
>
>                 Key: TIKA-4935
>                 URL: https://issues.apache.org/jira/browse/TIKA-4935
>             Project: Tika
>          Issue Type: Improvement
>            Reporter: Gary D. Gregory
>            Priority: Minor
>             Fix For: 4.1.0
>
>
> Tika maintains custom XML security configuration across SAX, DOM, StAX, and 
> XSLT processing. Adopt Apache Commons Secure XML 1.0.0 to centralize these 
> protections and reduce duplicated configuration and resolver code.
> PR [3261|https://github.com/apache/tika/pull/3261]:
>  * Uses Commons Secure XML factories in {{{}XMLReaderUtils{}}}, MIME type 
> loading, and XML-related tests.
>  * Removes manual SAX/DOM feature configuration, transformer external-access 
> attributes, and the custom StAX fallback resolver.
>  * Retains Tika’s configurable entity expansion limits, parser pooling, DOM 
> entity-reference settings, and StAX restrictions on DTD and external entity 
> processing.
>  * Routes async configuration writer document and transformer creation 
> through {{{}XMLReaderUtils{}}}.
>  * Adds Maven dependencies and updates the OSGi integration-test setup.
> Regression tests cover external entity blocking and external resource access 
> through XSLT {{{}document(){}}}, {{{}xsl:include{}}}, and {{xsl:import}} for 
> both transformer factory getters. They also verify that explicitly supplied 
> resolvers remain usable and that the async writer can create new XML 
> configurations and preserve existing configuration content.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to