ppkarwasz opened a new pull request, #45:
URL: https://github.com/apache/commons-xml/pull/45
Recognizes Saxon factories by package prefix instead of an exact class-name
set, addressing finding f008.
## Problem
`TransformerHardener` and `XPathHardener` route a factory to `SaxonProvider`
(the only recipe that installs Saxon's locked-down `Configuration`) by matching
its class name against a fixed set of three names. Saxon-HE ships a second
public `TransformerFactory`, `net.sf.saxon.BasicTransformerFactory` (a subclass
of the registered `net.sf.saxon.TransformerFactoryImpl`), selectable through
the standard `javax.xml.transform.TransformerFactory` system property. It is
not in the set, so it falls to the generic branch. That branch wraps the
factory (FSP, the URIResolver floor, the Source rewrite) and looks successful,
but never installs the empty `CollectionFinder`, so `fn:collection` fetches
directly and stays open.
## Changes
- `SaxonProvider` gains a package-private `isSaxon(Class<?>)` that matches
the `net.sf.saxon.` and `com.saxonica.` package prefixes. The outer class
references no Saxon types (they live in the inner configurer), so it still
loads without Saxon on the classpath.
- `TransformerHardener` and `XPathHardener` drop their exact-name sets and
call `SaxonProvider.isSaxon(...)`; javadoc updated from "recognized by class
name" to "recognized by package prefix", noting that subclasses such as
`BasicTransformerFactory` now route to the same recipe.
- `SaxonProvider.configure(...)` now catches the cast `ClassCastException`
and rethrows it as a `HardeningException` naming the factory class: with prefix
matching, a hypothetical Saxon-package factory that is not castable to
`SaxonTransformerFactory`/`XPathFactoryImpl` fails closed with a message rather
than a bare `ClassCastException` (the exact-name set previously made the cast
infallible).
## Tests
New `SaxonAlternateFactoryTest` (`@Tag("trax")`, reflective load of
`BasicTransformerFactory`, skipped when Saxon is absent, so effective on
test-saxon and test-saxon-xerces):
- `hardenedBasicFactoryDoesNotLeakCollection`: routes a
`BasicTransformerFactory` through `TransformerHardener.harden(...)` and runs a
stylesheet whose `collection()` would pull `LEAKED_MARKER` into the output;
blocks-or-does-not-leak.
- `unconfiguredBasicFactoryLeaksCollection`: the bare factory resolves it
(leak control).
`collection()` is the deliberately chosen channel: it bypasses Saxon's
resource-resolution chain, so only the empty `CollectionFinder` in
`SaxonProvider` closes it. Verified by probe on the pinned Saxon-HE 13.0 that
`collection()` leaks through both the bare factory and the generic
`HardeningTransformerFactory` branch but returns empty through
`SaxonProvider.configure`. Discrimination confirmed: with the exact-name
recognition temporarily restored, the hardened test fails on both Saxon
executions while the leak control passes. (The report's suggested
`unparsed-text()` probe does not discriminate on Saxon 13, since the generic
branch's URIResolver floor already covers that channel; `collection()` is the
one it does not.)
Full `mvn clean` and plain `mvn` (checkstyle, spotbugs, pmd, javadoc, whole
surefire matrix) green, plus a post-build non-vacuous `spotbugs:check`.
## Note
`TransformerHardener` is also rewritten by open PR #41 (Saxon wrapped like
every other implementation) and `XPathHardener` by open PR #44; whichever lands
later reconciles small conflicts, and the `SaxonProvider.isSaxon` prefix
recognition should be carried into #41's routing.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
