Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,4 @@ updates:
schedule:
interval: "weekly"
cooldown:
default-days: 7
default-days: 7
2 changes: 1 addition & 1 deletion .github/workflows/codeql-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ jobs:
permissions:
actions: read
contents: read
security-events: write
security-events: write
2 changes: 1 addition & 1 deletion .github/workflows/maven.yml
Original file line number Diff line number Diff line change
Expand Up @@ -198,4 +198,4 @@ jobs:
android-tests/build/outputs/androidTest-results/
android-tests/build/reports/androidTests/
if-no-files-found: ignore
retention-days: 14
retention-days: 14
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ static DocumentBuilderFactory harden(final DocumentBuilderFactory factory) {
// ACCESS_EXTERNAL_* support is the dividing capability between JAXP 1.5 implementations and older ones.
if (trySetAttribute(factory, XMLConstants.ACCESS_EXTERNAL_DTD, "")
&& trySetAttribute(factory, XMLConstants.ACCESS_EXTERNAL_SCHEMA, "")) {
// Honoured: the JAXP 1.5 properties block external fetches, so the bare factory is already hardened.
// Honored: the JAXP 1.5 properties block external fetches, so the bare factory is already hardened.
return factory;
}
// Rejected: external Xerces ignores ACCESS_EXTERNAL_*; install a deny-all resolver on every DocumentBuilder.
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/apache/commons/xml/Limits.java
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ final class Limits {
*/
private static final int DEFAULT_MAX_ELEMENT_DEPTH = 100;
/**
* Maximum length, in characters, of an XML name (element name, attribute name, namespace prefix, etc); JDK 25 secure value.
* Maximum length, in characters, of an XML name (element name, attribute name, namespace prefix, and so on); JDK 25 secure value.
*
* <p>Override with system property {@value #SP_MAX_NAME_LIMIT}.</p>
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ static SAXParserFactory harden(final SAXParserFactory factory) {
*/
static XMLReader hardenReader(final XMLReader reader) {
if (reader instanceof HardeningXMLReader) {
// Already hardened (e.g. handed back through XmlFactories.harden(XMLReader)); the floor is already in place.
// Already hardened (for example, handed back through XmlFactories.harden(XMLReader)); the floor is already in place.
return reader;
}
if (ANDROID_EXPAT_READER.equals(reader.getClass().getName())) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/apache/commons/xml/XmlFactories.java
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
* <h2>Thread safety</h2>
*
* <p>The returned factories inherit the thread-safety properties of the underlying JAXP implementation, which in practice means they are <strong>not
* guaranteed to be thread-safe</strong>. Create a new factory per thread or synchronise externally.</p>
* guaranteed to be thread-safe</strong>. Create a new factory per thread or synchronize externally.</p>
*
* <p>This class itself is thread-safe: all methods are static and stateless.</p>
*/
Expand Down
4 changes: 2 additions & 2 deletions src/site/markdown/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ stock JDK and external JAXP implementations.

Any Java library that parses XML has to harden JAXP before handing a factory to user code, and every library ends up
copy-pasting the same hardening snippet. The snippet is fragile: the attributes and features needed to harden a factory
are not standardised, each JAXP implementation exposes a slightly different set, and setting an unknown one throws an
are not standardized, each JAXP implementation exposes a slightly different set, and setting an unknown one throws an
exception that callers routinely swallow. Writing this block correctly for every implementation is real work, and
duplicating it across projects means every project owns the maintenance burden on its own.

Expand Down Expand Up @@ -142,5 +142,5 @@ pass the result as a `DOMSource` or `SAXSource`.

There is no caching or pooling inside `XmlFactories`; callers on a hot path are responsible for their own caching. The
returned factories inherit the thread-safety properties of the underlying JAXP implementation, which in practice means
they are not thread-safe. Create a new factory per thread or synchronise externally.
they are not thread-safe. Create a new factory per thread or synchronize externally.

20 changes: 10 additions & 10 deletions src/test/java/org/apache/commons/xml/AttackTestSupport.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,17 +58,17 @@
/**
* Shared fixtures for attack tests.
*
* <p>The hardened-side helpers come in three flavours, distinguished by their suffix:</p>
* <p>The hardened-side helpers come in three flavors, distinguished by their suffix:</p>
*
* <ul>
* <li>{@code assert*Blocks(...)} runs the payload through a hardened factory from {@link XmlFactories} and asserts the parse throws. Used when the hardening
* layer is expected to reject the attack outright.</li>
* <li>{@code assert*DoesNotLeak(...)} runs the payload through a hardened factory and asserts the parse completes without throwing and without producing the
* {@link #LEAKED_MARKER} string. Used when the hardening contract guarantees the parse succeeds but never resolves the external resource (e.g.
* {@link #LEAKED_MARKER} string. Used when the hardening contract guarantees the parse succeeds but never resolves the external resource (for example,
* {@code XERCES_LOAD_EXTERNAL_DTD=false} silently skipping the external subset, with the body's undeclared entity reference dropped per XML 1.0
* §4.1).</li>
* <li>{@code assert*BlocksOrDoesNotLeak(...)} accepts either of the previous two outcomes. Used where the same hardening contract surfaces differently across
* providers (e.g. stock-JDK XSLTC throws via {@code ACCESS_EXTERNAL_DTD} while Apache Xalan silently skips because its source-rewrite routes parsing
* providers (for example, stock-JDK XSLTC throws via {@code ACCESS_EXTERNAL_DTD} while Apache Xalan silently skips because its source-rewrite routes parsing
* through a {@code XERCES_LOAD_EXTERNAL_DTD=false} reader).</li>
* </ul>
*
Expand Down Expand Up @@ -189,7 +189,7 @@ static void assertDomBlocks(final String payload) {
* Asserts a hardened DOM parse completes without throwing and without leaked content.
*
* <p>{@link DocumentBuilder#parse(InputSource)} via {@link XmlFactories#newDocumentBuilderFactory()}; use this when the hardening guarantee is "the parse
* succeeds but never resolves the external resource", e.g. when {@code XERCES_LOAD_EXTERNAL_DTD=false} silently skips the external subset.</p>
* succeeds but never resolves the external resource", for example, when {@code XERCES_LOAD_EXTERNAL_DTD=false} silently skips the external subset.</p>
*/
static void assertDomDoesNotLeak(final String payload) {
assertNoLeakStrict(() -> domParseAndCaptureText(payload), "DOM");
Expand All @@ -208,7 +208,7 @@ static void assertDomParses(final String payload) {
* Skeleton for every {@code assert*BlocksOrDoesNotLeak} helper.
*
* <p>Treats a thrown exception of one of the {@code expected} types as "hardening blocked at parse" (acceptable); otherwise asserts the captured output
* omits {@link #LEAKED_MARKER}. A throw whose type does not match {@code expected} fails the test, so unrelated failures (e.g. a {@link HardeningException}
* omits {@link #LEAKED_MARKER}. A throw whose type does not match {@code expected} fails the test, so unrelated failures (for example, a {@link HardeningException}
* because no recipe matched the JAXP implementation) cannot be silently accepted as a clean block.</p>
*
* @param action the parse to execute, returning the captured output text checked for {@link #LEAKED_MARKER}.
Expand Down Expand Up @@ -409,7 +409,7 @@ static void assertSaxBlocks(final String payload) {
* Asserts a hardened SAX parse completes without throwing and without leaked content.
*
* <p>{@link XMLReader#parse(InputSource)} on a parser from {@link XmlFactories#newSAXParserFactory()}; use this when the hardening guarantee is "the parse
* succeeds but never resolves the external resource", e.g. when {@code XERCES_LOAD_EXTERNAL_DTD=false} silently skips the external subset.</p>
* succeeds but never resolves the external resource", for example, when {@code XERCES_LOAD_EXTERNAL_DTD=false} silently skips the external subset.</p>
*/
static void assertSaxDoesNotLeak(final String payload) {
assertNoLeakStrict(() -> captureCharacters(strictXMLReader(XmlFactories.newSAXParserFactory()), payload), "SAX");
Expand Down Expand Up @@ -446,7 +446,7 @@ static void assertSchemaCompiles(final Source xsd) {
* Asserts a hardened Schema compilation completes without throwing.
*
* <p>{@link SchemaFactory#newSchema(Source)} via {@link XmlFactories#newSchemaFactory()}; use this when the hardening contract guarantees the compile
* succeeds but never resolves the external resource (e.g. {@code XERCES_LOAD_EXTERNAL_DTD=false} silently skipping the external subset, with the body's
* succeeds but never resolves the external resource (for example, {@code XERCES_LOAD_EXTERNAL_DTD=false} silently skipping the external subset, with the body's
* undeclared entity reference dropped per XML 1.0 §4.1).</p>
*/
static void assertSchemaDoesNotLeak(final Source xsd) {
Expand All @@ -456,7 +456,7 @@ static void assertSchemaDoesNotLeak(final Source xsd) {
/**
* Asserts a hardened StAX parse of the payload throws.
*
* <p>{@link XMLStreamReader} and {@link XMLEventReader} from {@link XmlFactories#newXMLInputFactory()}; both flavours are exercised and either must
* <p>{@link XMLStreamReader} and {@link XMLEventReader} from {@link XmlFactories#newXMLInputFactory()}; both flavors are exercised and either must
* throw.</p>
*/
static void assertStaxBlocks(final String payload) {
Expand All @@ -467,8 +467,8 @@ static void assertStaxBlocks(final String payload) {
/**
* Asserts a hardened StAX parse completes without throwing and without leaked content.
*
* <p>{@link XMLStreamReader} and {@link XMLEventReader} from {@link XmlFactories#newXMLInputFactory()}; both flavours are exercised. Use this when the
* hardening guarantee is "the parse succeeds but never resolves the external resource", e.g. when the JDK's {@code ignore-external-dtd} property silently
* <p>{@link XMLStreamReader} and {@link XMLEventReader} from {@link XmlFactories#newXMLInputFactory()}; both flavors are exercised. Use this when the
* hardening guarantee is "the parse succeeds but never resolves the external resource", for example, when the JDK's {@code ignore-external-dtd} property silently
* skips the external subset.</p>
*/
static void assertStaxDoesNotLeak(final String payload) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
* <p>Which fixture each test uses:</p>
*
* <ul>
* <li>{@code hardened*} tests pull from {@code hardened*Payload} helpers: large fixture on Android (libexpat is the only defence), medium fixture on JDK
* <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>
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/apache/commons/xml/NoDoctypeTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
* Checks that a plain document without a {@code DOCTYPE} declaration parses cleanly through every hardened JAXP surface.
*
* <p>This is the realistic 99% case for hardened input; the hardening contract being verified is "documents without a DOCTYPE parse cleanly through every
* JAXP surface" so accidental tightening (e.g. a resolver that refuses the synthetic-external-subset hook) is caught.</p>
* JAXP surface" so accidental tightening (for example, a resolver that refuses the synthetic-external-subset hook) is caught.</p>
*/
class NoDoctypeTest {

Expand Down
4 changes: 2 additions & 2 deletions src/test/java11/org/apache/commons/xml/DescriptorIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ private static JarEntry findModuleInfo(final JarFile jar) {
}

/**
* Returns whether {@code pkg} is exported by an OSGi system bundle, i.e. it sits under one of {@link #PLATFORM_PACKAGE_PREFIXES}.
* Returns whether {@code pkg} is exported by an OSGi system bundle, that is, it sits under one of {@link #PLATFORM_PACKAGE_PREFIXES}.
*/
private static boolean isPlatformPackage(final String pkg) {
for (final String prefix : PLATFORM_PACKAGE_PREFIXES) {
Expand Down Expand Up @@ -150,7 +150,7 @@ private static List<String> splitClauses(final String header) {
}

/**
* Returns the package name of a clause, i.e. everything before its first {@code ;} attribute separator.
* Returns the package name of a clause, that is, everything before its first {@code ;} attribute separator.
*/
private static String packageName(final String clause) {
final int semicolon = clause.indexOf(';');
Expand Down
Loading