ppkarwasz opened a new pull request, #75:
URL: https://github.com/apache/commons-secure-xml/pull/75
`setResolverProperty` routed a caller's resolver by calling `setDelegate` on
the floor already installed on the named hook. That treats the floor as private
to that hook, and it is not — confirmed in Woodstox's own sources rather than
inferred:
```java
// com.ctc.wstx.api.ReaderConfig
public void setXMLResolver(XMLResolver r) { mEntityResolver = r;
mDtdResolver = r; }
public ReaderConfig createNonShared(SymbolTable sym) {
...
rc.mDtdResolver = mDtdResolver;
rc.mEntityResolver = mEntityResolver;
```
The constructor installs the floor through `setXMLResolver`, so on Woodstox
one object lands on both the DTD-subset and entity hooks, and `createNonShared`
copies that reference into every reader the factory creates. Mutating it
therefore had two effects the caller never asked for:
- a resolver set on `com.ctc.wstx.dtdResolver` also answered
`com.ctc.wstx.entityResolver` (and the reverse), silently widening the scope of
an opt-in and discarding any resolver previously set on the other hook;
- readers created *before* the call had their resolution policy changed,
including readers already parsing — the write was also unsynchronized, so
another thread could observe it arbitrarily late.
Everything stayed behind a floor throughout: an unresolved reference still
resolved to empty and no raw fetch was ever re-opened. This is a state and
aliasing bug, in the same family as the shared mutable empty document in #69,
not a way to bypass the securing.
The fix installs a new floor on the named hook rather than re-delegating the
one already there. Each hook then keeps the floor it was given and each reader
the one it captured, which is both per-hook independence and the creation-time
binding StAX callers expect. Since nothing is mutated after publication any
more, the cross-thread write disappears too, so no `volatile` is needed.
### Tests
The `com.ctc.wstx.*` hooks had no coverage at all. Two tests are added, and
I verified both **fail against the pre-fix code**, so they discriminate rather
than passing vacuously:
- `woodstoxResolverHooksStayIndependent` — a resolver set on the DTD hook
must not answer the entity hook (skips where the implementation does not know
the properties).
- `settingAResolverInstallsAFreshFloorInsteadOfMutatingTheInstalledOne` —
the floor an existing reader captured keeps resolving to empty.
`setXMLResolverRoutesCallerBehindInstalledFloor` asserted the old mechanism
(that the installed floor object be mutated, and that `setProperty` never be
called on the hook), which is exactly what changes here. It now asserts the
contract it was standing in for: the hook still holds a floor, the caller's
resolver is its delegate, and both getters report it unwrapped.
Full surefire matrix green. No `changes.xml` entry: the behaviour is
fail-secure throughout and lands in the same release cycle that introduced it.
🤖 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]
