Copilot commented on code in PR #22:
URL: https://github.com/apache/commons-xml/pull/22#discussion_r3553155188
##########
src/test/java/org/apache/commons/xml/BillionLaughsTest.java:
##########
@@ -24,237 +24,213 @@
/**
* Checks whether parsers reject a Billion Laughs payload (nested entity
expansion in the internal DTD subset).
*
- * <p>Two fixtures are used:</p>
+ * <p>Each {@code hardened*} test asserts the library blocks the payload;
+ * its {@code unconfigured*} positive control asserts the same payload parses
once the limit is disabled,
+ * so a block reflects the hardening rather than a broken wrapper.
+ * The library pins no custom entity-expansion limit; each parser keeps its
own secure-processing default, which varies by implementation:
+ * {@code 2,500} (stock JDK),
+ * {@code 64,000} (external Xerces under {@code FEATURE_SECURE_PROCESSING}),
+ * {@code 100,000} (external Xerces' and Woodstox's own security managers).
+ * A payload therefore has to exceed the largest of these.</p>
+ *
+ * <p>Every payload shares one six-level x10 {@link #DTD} (declaring entities
is free until they are referenced) and varies only the body it expands:</p>
*
* <ul>
- * <li>The <strong>medium</strong> fixture nests three levels of 16x
expansion ({@code lol1} through {@code lol3}), so resolving {@code &lol3;}
produces
- * {@code 16 * 16 * 16 = 4096} leaf {@code &lol;} expansions and a total
of {@code 1 + 16 + 256 + 4096 = 4369} entity-expansion events. That is
- * comfortably over the {@code entityExpansionLimit = 2500} this library
pins on every JAXP factory it returns (mirroring JDK 25's secure value), so the
- * hardened side trips on every JDK from 8 to 25. The unconfigured side
disables the limit explicitly via {@code setAttribute/setProperty} on the JDK
- * property name, so it parses the ~4 KB of expanded {@code "A"} text
and finishes immediately.</li>
- * <li>The <strong>large</strong> fixture nests seven levels of 10x
expansion ({@code lol1} through {@code lol7}), so resolving {@code &lol7;}
produces
- * {@code 10^7 = 10 000 000} leaf expansions, ~10 MB of {@code "A"}
text, and an amplification factor in the tens of thousands. Sized to trip
- * libexpat >= 2.4's built-in billion-laughs check (8 MiB activation
threshold and 100x amplification factor).</li>
+ * <li>{@link #CONTENT_120K} ({@code 120,000}) on the JVM: above every JVM
parser default.</li>
+ * <li>{@link #CONTENT_9M} ({@code 9,000,000}) on Android: above libexpat's
8 MiB billion-laughs activation threshold, the only defense there since the
limit is
+ * not configurable. For that same reason the positive controls do not
run on Android (see {@link #assumeEntityLimitConfigurable()}): a payload the
hardened test
+ * blocks cannot be parsed even without hardening.</li>
* </ul>
*
- * <p>Why a single character {@code "A"}: XSLTC compiles a stylesheet's
expanded text into a JVM string constant, which is capped at 65535 bytes. A
larger
- * expansion makes {@code newTransformer(stylesheet)} fail with a misleading
"GregorSamsa" stub-class error even when entity limits are disabled. The medium
- * fixture is sized to stay well under that ceiling. The large fixture cannot
be used for {@code Templates} / {@code Transformer} compilation for the same
- * reason.</p>
- *
- * <p>Which fixture each test uses:</p>
+ * <p>The XSLT payload spreads those same {@code 120,000} expansions over two
literal result elements with content {@link #CONTENT_60K} rather than one text
node,
+ * because XSLTC caps a compiled literal at 65,535 bytes; see {@link
#xsltPayload()}.
+ * A parser counts expansions across the whole document, so the split changes
nothing on the hardened side.</p>
*
- * <ul>
- * <li>{@code hardened*} tests pull from {@code hardened*Payload} helpers:
large fixture on Android (libexpat is the only defense), medium fixture on JDK
- * (entity-expansion count limit is sufficient).</li>
- * <li>{@code unconfigured*} positive controls pull from {@code
medium*Payload} helpers regardless of platform: libexpat's billion-laughs check
cannot be
- * disabled from Java, so a permissive parse of the large fixture would
still trip on Android.</li>
- * </ul>
+ * <p>Why a single character {@code "A"}: it makes the expanded size equal the
expansion count, so a payload's size maps directly onto each parser's limit, and
+ * (being ASCII) onto XSLTC's byte-counted constant-pool ceiling as well.</p>
*/
class BillionLaughsTest {
- private static final String LARGE_CONTENT = "&lol7;";
-
- private static final String LARGE_DTD =
- " <!ENTITY lol \"A\">\n"
- + " <!ENTITY lol1
\"&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;\">\n"
- + " <!ENTITY lol2
\"&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;\">\n"
- + " <!ENTITY lol3
\"&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;\">\n"
- + " <!ENTITY lol4
\"&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;\">\n"
- + " <!ENTITY lol5
\"&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;\">\n"
- + " <!ENTITY lol6
\"&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;\">\n"
- + " <!ENTITY lol7
\"&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;\">\n";
-
- private static final String MEDIUM_CONTENT = "&lol3;";
-
- private static final String MEDIUM_DTD =
- " <!ENTITY lol \"A\">\n"
- + " <!ENTITY lol1
\"&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;\">\n"
- + " <!ENTITY lol2
\"&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;\">\n"
- + " <!ENTITY lol3
\"&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;\">\n";
-
+ /** 6 x 10,000 = 60,000 expansions; each half of the split XSLT body. */
+ private static final String CONTENT_60K = repeatRef("lol4", 6);
+ /** 100,000 + 2 x 10,000 = 120,000 expansions; above every JVM parser's
secure default (2,500 / 64,000 / 100,000). */
+ private static final String CONTENT_120K = "&lol5;&lol4;&lol4;";
+ /** 9 x 1,000,000 = 9,000,000 expansions; above libexpat's 8 MiB
billion-laughs activation threshold. */
+ private static final String CONTENT_9M = repeatRef("lol6", 9);
/**
- * Hardened-side payload for DOM/SAX/XmlReader
- *
- * <ul>
- * <li>~10 MiB on Android to trip libexpats 8 MiB limit, and</li>
- * <li>~4 KiB on JDK to trip JDK 25's {@code entityExpansionLimit =
2500}.</li>
- * </ul>
+ * Shared DTD for every payload: a six-level x10 ladder, {@code &lol1;}
through {@code &lol6;} ({@code &lol6;} expands to 1,000,000). Declaring an
entity costs
+ * nothing until it is referenced, so the DTD is identical on every
platform and only the expanded body ({@link #content()}) varies.
*/
- private static String hardenedXmlPayload() {
- return AttackTestSupport.IS_ANDROID
- ? withDoctype("root", LARGE_DTD,
AttackTestSupport.xmlBody(LARGE_CONTENT))
- : mediumXmlPayload();
- }
+ private static final String DTD =
+ " <!ENTITY lol \"A\">\n"
+ + entityLine("lol1", "lol") // 10
+ + entityLine("lol2", "lol1") // 100
+ + entityLine("lol3", "lol2") // 1000
+ + entityLine("lol4", "lol3") // 10000
+ + entityLine("lol5", "lol4") // 100000
+ + entityLine("lol6", "lol5"); // 1000000
/**
- * Hardened-side XSD payload
+ * Skips a positive control on Android.
*
- * <ul>
- * <li>~10 MiB on Android to trip libexpats 8 MiB limit, and</li>
- * <li>~4 KiB on JDK to trip JDK 25's {@code entityExpansionLimit =
2500}.</li>
- * </ul>
+ * <p>The controls prove the hardened test blocked a payload that would
otherwise parse, so they must use the very payload the hardened test blocks.
+ * On Android the entity-expansion limit is not configurable (libexpat's
billion-laughs check cannot be lifted), so that payload
+ * cannot be parsed even without hardening, leaving nothing to prove.</p>
*/
- private static String hardenedXsdPayload() {
- return AttackTestSupport.IS_ANDROID
- ? withDoctype("xs:schema", LARGE_DTD,
AttackTestSupport.xsdBody(LARGE_CONTENT))
- : mediumXsdPayload();
+ private static void assumeEntityLimitConfigurable() {
+ Assumptions.assumeFalse(AttackTestSupport.IS_ANDROID, "Skipped on
Android: the entity-expansion limit is not configurable");
}
- /**
- * Hardened-side XSLT payload
- *
- * <ul>
- * <li>~10 MiB on Android to trip libexpats 8 MiB limit, and</li>
- * <li>~4 KiB on JDK to trip JDK 25's {@code entityExpansionLimit =
2500}, and</li>
- * <li>stay under XSLTC's 60 KB constant-pool cap, and</li>
- * </ul>
- */
- private static String hardenedXsltPayload() {
- return AttackTestSupport.IS_ANDROID
- ? withDoctype("xsl:stylesheet", LARGE_DTD,
AttackTestSupport.xsltBody(LARGE_CONTENT))
- : mediumXsltPayload();
+ /** The body to expand: 9,000,000 on Android, where libexpat is the only
defense and the limit is not configurable, 101,000 on the JVM. */
Review Comment:
The comment on `content()` says the JVM payload is "101,000" expansions, but
`content()` returns `CONTENT_120K` (documented as 120,000). This mismatch is
confusing when reasoning about which parser defaults the test is meant to
exceed.
--
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]
