Merge pull request #22 from ppkarwasz/feature/remove-limits-class

[COMMONSXML-2] Remove `Limits` class and related functionality

Author: garydgregory

src/main/java/org/apache/commons/xml/DocumentBuilderHardener.java

@@ -37,8 +37,6 @@
  *     <li><strong>FSP</strong>: required. It switches on the implementation's built-in security manager, which is what carries the processing limits.</li>
  *     <li><strong>{@code XERCES_LOAD_EXTERNAL_DTD}</strong>: optional. Where supported, it skips the external DTD subset on non-validating parsers so a
  *         DOCTYPE-only document parses without a fetch attempt. If not supported, the fetch will throw instead, due to the following settings.</li>
- *     <li><strong>Limits</strong>: applied best-effort by {@link Limits#tryApply(DocumentBuilderFactory)}, which adapts to the JDK attribute limits or Xerces'
- *         {@code SecurityManager} as appropriate.</li>
  *     <li><strong>Deny-all resolver floor</strong>: every produced {@link DocumentBuilder} is wrapped by a {@link HardeningDocumentBuilderFactory} that keeps a
  *         deny-all {@link EntityResolver} floor. That floor blocks external DTD, entity, schema and {@code xi:include} fetches in one place: the stock JDK's
  *         XInclude processor ignores {@code ACCESS_EXTERNAL_*} and consults the {@link EntityResolver} instead, so no {@code ACCESS_EXTERNAL_*} attributes are
@@ -60,8 +58,6 @@ static DocumentBuilderFactory harden(final DocumentBuilderFactory factory) {
         }
         // Required: enables the implementation's security manager, which carries the limits.
         setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true);
-        // Optional, implementation-based: JDK attribute limits or Xerces' SecurityManager.
-        Limits.tryApply(factory);
         // Optional: skip the external DTD subset on non-validating parsers so DOCTYPE-only documents parse without a blocked fetch attempt.
         setOptionalFeature(factory, XERCES_LOAD_EXTERNAL_DTD, false);
         // Required: HardeningDocumentBuilderFactory installs a deny-all EntityResolver floor on every DocumentBuilder.

src/main/java/org/apache/commons/xml/JaxpSetters.java

@@ -41,7 +41,7 @@ final class JaxpSetters {
     private interface ThrowingAction {
         void run() throws Exception;
     }
-    private static final String KIND_ATTRIBUTE = "attribute";
+
     private static final String KIND_FEATURE = "feature";
 
     private static void apply(final Object factory, final String kind, final String name, final ThrowingAction action) {
@@ -52,10 +52,6 @@ private static void apply(final Object factory, final String kind, final String
         }
     }
 
-    static void setAttribute(final DocumentBuilderFactory factory, final String attribute, final Object value) {
-        apply(factory, KIND_ATTRIBUTE, attribute, () -> factory.setAttribute(attribute, value));
-    }
-
     static void setFeature(final DocumentBuilderFactory factory, final String feature, final boolean value) {
         apply(factory, KIND_FEATURE, feature, () -> factory.setFeature(feature, value));
     }
@@ -88,22 +84,6 @@ static void setFeature(final XMLReader reader, final String feature, final boole
         apply(reader, KIND_FEATURE, feature, () -> reader.setFeature(feature, value));
     }
 
-    static void setOptionalAttribute(final DocumentBuilderFactory factory, final String attribute, final Object value) {
-        try {
-            factory.setAttribute(attribute, value);
-        } catch (final Exception e) {
-            // Ignored: the implementation does not recognize this attribute.
-        }
-    }
-
-    static void setOptionalAttribute(final TransformerFactory factory, final String attribute, final Object value) {
-        try {
-            factory.setAttribute(attribute, value);
-        } catch (final Exception e) {
-            // Ignored: the implementation does not recognize this attribute.
-        }
-    }
-
     static void setOptionalFeature(final DocumentBuilderFactory factory, final String feature, final boolean value) {
         try {
             factory.setFeature(feature, value);

src/main/java/org/apache/commons/xml/Limits.java

@@ -51,8 +51,6 @@
  *         processing limits.</li>
  *     <li><strong>{@code XERCES_LOAD_EXTERNAL_DTD}</strong>: optional. Where supported, it skips the external DTD subset on non-validating parsers so a
  *         DOCTYPE-only document parses without a fetch attempt. If not supported, the fetch will throw instead, due to the following settings.</li>
- *     <li><strong>Limits</strong>: applied best-effort by {@link Limits#tryApply(XMLReader)}, which adapts to the JDK limit properties or Xerces'
- *         {@code SecurityManager} as appropriate.</li>
  *     <li><strong>Deny-all resolver floor</strong>: every reader is wrapped in a {@link HardeningXMLReader} that keeps a deny-all {@link EntityResolver} floor.
  *         That floor blocks external DTD, entity, schema and {@code xi:include} fetches in one place: the stock JDK's XInclude processor ignores
  *         {@code ACCESS_EXTERNAL_*} and consults the {@link EntityResolver} instead, so no {@code ACCESS_EXTERNAL_*} properties are needed here. A caller can
@@ -176,8 +174,6 @@ static XMLReader hardenReader(final XMLReader reader) {
         setFeature(reader, XMLConstants.FEATURE_SECURE_PROCESSING, true);
         // Optional: skip the external DTD subset on non-validating parsers so DOCTYPE-only documents parse without a blocked fetch attempt.
         setOptionalFeature(reader, XERCES_LOAD_EXTERNAL_DTD, false);
-        // Optional, implementation-based: JDK limit properties or Xerces' SecurityManager.
-        Limits.tryApply(reader);
         // Required: HardeningXMLReader installs a deny-all EntityResolver floor on the reader.
         // That floor blocks external DTD, entity, schema and xi:include fetches in one place: no ACCESS_EXTERNAL_* properties are needed here.
         // Callers can chain their resolvers, but not override the floor.

src/main/java/org/apache/commons/xml/SAXParserHardener.java

@@ -29,8 +29,6 @@
  * <p>Rather than branching on the implementation class, {@link #harden(XMLInputFactory)} consolidates the JDK Zephyr and Woodstox recipes into one pass that
  * probes which properties each factory accepts and adapts:</p>
  * <ul>
- *     <li><strong>Limits</strong>: applied best-effort by {@link Limits#tryApply(XMLInputFactory)}, which sets both the JDK and the Woodstox limit properties;
- *         each implementation honors its own and rejects the other's.</li>
  *     <li><strong>External DTD subset</strong>: skipped via Zephyr's {@value #ZEPHYR_IGNORE_EXTERNAL_DTD} (best-effort), so a DOCTYPE-only document parses
  *         without a fetch attempt instead of tripping the deny-all resolver below. Woodstox skips it through {@value #WSTX_DTD_RESOLVER} instead.</li>
  *     <li><strong>External entities</strong>: denied through a non-removable {@link Resolvers.FallbackDenyXMLResolver} floor on the entity-resolution hook,
@@ -82,8 +80,6 @@ protected Object onUnresolved(final String publicID, final String systemID, fina
     }
 
     static XMLInputFactory harden(final XMLInputFactory factory) {
-        // Optional, implementation-based: JDK limit properties or Woodstox limit properties.
-        Limits.tryApply(factory);
         // Optional: Zephyr's StAX equivalent of XERCES_LOAD_EXTERNAL_DTD=false skips the external DTD subset entirely.
         setOptionalProperty(factory, ZEPHYR_IGNORE_EXTERNAL_DTD, true);
 

src/main/java/org/apache/commons/xml/StaxHardener.java

@@ -43,8 +43,6 @@
  *         hardening surface is reachable only through a vendor API.</li>
  *     <li><strong>FSP</strong> ({@link XMLConstants#FEATURE_SECURE_PROCESSING}): required. On XSLTC it enables the runtime evaluator limits; on Xalan it disables
  *         reflection-based extension functions.</li>
- *     <li><strong>Limits</strong>: applied best-effort by {@link Limits#tryApply(TransformerFactory)}. XSLTC honors the JDK attribute limits; Xalan ignores them
- *         (its caps come from FSP).</li>
  *     <li><strong>{@link Resolvers.FallbackDenyURIResolver} floor</strong>: required. A deny-all {@link URIResolver} floor, installed by
  *         {@link HardeningTransformerFactory} and carried onto every produced {@link Transformer}, blocks {@code xsl:import}/{@code xsl:include} at compile time
  *         and {@code document()} at runtime, the one channel both XSLTC and Xalan route through. A caller-set {@link URIResolver} is routed through the floor
@@ -72,8 +70,6 @@ static TransformerFactory harden(final TransformerFactory factory) {
         }
         // Required: enables secure processing (XSLTC runtime limits; Xalan's extension-function block).
         setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true);
-        // Best-effort: JDK's XSLTC honors the JDK attribute limits, pinning them to JDK 25 secure values; Xalan ignores them.
-        Limits.tryApply(factory);
         // Required: source/stylesheet parsing provisions its own SAX reader otherwise; the wrapper routes every Source through a hardened one and installs the
         // deny-all URIResolver floor (blocking xsl:import/include at compile time and document() at runtime) that a caller-set resolver cannot remove.
         return new HardeningTransformerFactory((SAXTransformerFactory) factory);

src/main/java/org/apache/commons/xml/TransformerHardener.java

@@ -72,27 +72,29 @@ defends against: that is the reason reconfiguration moves a report
 
 ### Assumptions about the environment
 
-The library does not open network connections, spawn processes, install signal handlers, or read environment variables
-of its own: each `XmlFactories` method only configures and returns a JAXP factory, and reads the JDK system properties
-listed below. Which hardening recipe applies depends on the JAXP implementation present on the classpath.
+The library does not open network connections,
+spawn processes,
+install signal handlers,
+or read environment variables of its own:
+each `XmlFactories` method only configures and returns a JAXP factory.
+Which hardening recipe applies depends on the JAXP implementation present on the classpath.
 
 **System properties that modify behavior**
 
-When a factory or parser is created, the library reads the following JDK system properties and pins each value on every
-recognized parser, including bundled ones that ignore `jdk.xml.*` on their own. If a property is unset, the fixed value
-shown applies (matching the JDK 25 secure default), regardless of the JDK in use. These properties are trusted
-deployment configuration: an operator may set one to tighten (or loosen) a limit globally, but loosening through one is
-reconfiguration, treated like loosening any other reserved setting (see [What is out of scope](#what-is-out-of-scope)).
-
-- `jdk.xml.elementAttributeLimit`: `200`
-- `jdk.xml.entityExpansionLimit`: `2500`
-- `jdk.xml.entityReplacementLimit`: `100000`
-- `jdk.xml.maxElementDepth`: `100`
-- `jdk.xml.maxGeneralEntitySizeLimit`: `100000`
-- `jdk.xml.maxOccurLimit`: `5000`
-- `jdk.xml.maxParameterEntitySizeLimit`: `15000`
-- `jdk.xml.maxXMLNameLimit`: `1000`
-- `jdk.xml.totalEntitySizeLimit`: `100000`
+The library reads no system property of its own. It enables secure processing (`FEATURE_SECURE_PROCESSING`) on every
+recognized parser and leaves the resulting processing limits (entity expansion, element depth, attribute count, and
+similar) at the implementation's own secure default. Those defaults differ by implementation, and on the stock JDK by
+JDK version and the standard `jdk.xml.*` limit properties the JDK itself reads:
+
+- On the stock JDK, secure processing honors the `jdk.xml.*` limit properties (for example `jdk.xml.entityExpansionLimit`,
+  default `2500` on JDK 25 and `64000` on JDK 8 through 21). These are trusted deployment configuration: an operator may
+  set one to tighten (or loosen) a limit globally, but loosening through one is reconfiguration, treated like loosening
+  any other reserved setting (see [What is out of scope](#what-is-out-of-scope)).
+- The bundled parsers apply their own hardcoded secure defaults instead (for example external Xerces and Woodstox cap
+  entity expansion at `100000`) and do not read `jdk.xml.*`.
+
+Every one of these defaults still bounds entity expansion tightly enough to reject entity-expansion denial of service
+such as Billion Laughs.
 
 **Reserved settings (must not be loosened)**
 
@@ -115,7 +117,7 @@ produces, breaks the hardening for that instance.
 - `javax.xml.stream.isSupportingExternalEntities`
 - `javax.xml.stream.supportDTD`
 - `jdk.xml.overrideDefaultParser`
-- the JDK processing-limit properties listed above
+- the implementation's secure-processing limits (entity expansion, element depth, attribute count, and similar)
 
 This list is not exhaustive:
 any other feature, attribute, property, or system property that
@@ -226,8 +228,7 @@ A report judged against this model receives exactly one of:
 
 Revise this model when any of the following change: a new `XmlFactories` factory method or other public
 surface; support for a JAXP implementation beyond those listed under [What is in scope](#what-is-in-scope);
-a change in the pinned processing-limit defaults this list records; a new reserved setting; or a
-report that cannot be routed to one of the dispositions above.
+a new reserved setting; or a report that cannot be routed to one of the dispositions above.
 
 ## Security Vulnerabilities