Skip to content

Install a fresh StAX resolver floor per hook instead of mutating one - #75

Merged
garydgregory merged 1 commit into
apache:mainfrom
ppkarwasz:fix/stax-resolver-floor-per-hook
Sep 1, 2026
Merged

Install a fresh StAX resolver floor per hook instead of mutating one#75
garydgregory merged 1 commit into
apache:mainfrom
ppkarwasz:fix/stax-resolver-floor-per-hook

Conversation

@ppkarwasz

Copy link
Copy Markdown
Member

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:

// 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

Routing a caller resolver by calling setDelegate on the floor already
installed treated that floor as private to the hook being set, which it
is not. Woodstox routes setXMLResolver to both its DTD-subset and
entity hooks, so one floor object sits on several of them, and its
ReaderConfig.createNonShared copies the reference into every reader it
creates. Mutating the object therefore answered hooks the call never
named, and changed the resolution policy of readers created earlier,
including ones already parsing on another thread.

Install a new floor on the named hook instead. Each hook then keeps the
floor it was given and each reader the one it captured, so a resolver
is scoped to the hook it was set on and bound when the reader was made.
Nothing is mutated after publication any more, which also removes the
unsynchronized cross-thread write.

The com.ctc.wstx.* hooks had no tests; they have two now, both verified
to fail without the main-code change. The existing setXMLResolver test
asserted the mutation itself, so it now asserts the contract it was
standing in for: the hook keeps a floor and the caller sits behind it.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CLnTBsvmYtxzNTWVGNyz33
@garydgregory
garydgregory merged commit 73b6bbf into apache:main Sep 1, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants