Skip to content

Delegate the Java 18 XPathFactory property methods - #76

Merged
garydgregory merged 2 commits into
apache:mainfrom
ppkarwasz:fix/xpath-factory-property-delegation
Sep 1, 2026
Merged

Delegate the Java 18 XPathFactory property methods#76
garydgregory merged 2 commits into
apache:mainfrom
ppkarwasz:fix/xpath-factory-property-delegation

Conversation

@ppkarwasz

Copy link
Copy Markdown
Member

Java 18 added setProperty(String, String) and getProperty(String) to javax.xml.xpath.XPathFactory. Their default implementations throw UnsupportedOperationException, and the JDK's XPathFactoryImpl overrides both to accept its jdk.xml.xpath* processing limits.

This library compiles against the Java 8 API, so the wrapper overrode only the methods that existed there and inherited those defaults. On Java 18 or later that meant a secured factory answered UnsupportedOperationException for a property the delegate would have accepted: an operator could neither tighten jdk.xml.xpathExprGrpLimit and friends per factory, nor read back the effective value to audit it. It contradicts the package Javadoc's promise that "features, properties, and attributes delegate to" the implementation.

Nothing could be loosened this way either — the failure is loud and in the safe direction — so this is a limitation of the wrapper rather than a weakening of the securing.

Both methods are now delegated through method handles, the way newDefaultInstance already reaches a static method added in a later release. MethodHandleFactory gains a general findVirtual companion to its existing findStatic.

Two things a reviewer will want to know:

  • Neither method carries @Override, and neither can. At release 8 the supertype declares no such method, so the annotation would not compile. They override at run time on Java 18 or later, which is the point; both carry a Javadoc note saying so, since the natural instinct is to add the annotation.
  • Where the handle is absent (Java 8 through 17) they throw UnsupportedOperationException, matching the behaviour that release inherits — though the path is unreachable through an XPathFactory reference there, since the method does not exist to be called.

The delegate's own exceptions pass through unwrapped, so an unrecognised property still surfaces its IllegalArgumentException.

Tests

Gated on the presence of the method rather than on a version string:

JDK 17 JDK 25 without the delegation (JDK 25)
delegatesTheJava18PropertyApi skip pass fail
reportsAnUnknownPropertyLikeTheDelegate skip pass fail

They reach the pair reflectively — the suite compiles at release 8 too — which also means they exercise exactly the runtime dispatch a Java 18 caller gets. Both were verified to fail with the delegation removed, so they discriminate rather than passing vacuously.

Full surefire matrix green on JDK 17 and on JDK 25.

🤖 Generated with Claude Code

ppkarwasz and others added 2 commits September 1, 2026 13:06
Java 18 added setProperty and getProperty to XPathFactory, with
defaults that throw UnsupportedOperationException and an override in
the JDK implementation that accepts its jdk.xml.xpath* limits. The
wrapper is compiled against the Java 8 API, so it inherited those
defaults and answered for the delegate: on Java 18 or later a caller
could neither tighten nor read a limit through a secure factory, though
nothing could be loosened either.

Delegate both through method handles, the way newDefaultInstance
already reaches a later-release static method. Neither carries
@OverRide, which would not compile at release 8; each overrides at run
time where the platform declares the method, and reports the inherited
UnsupportedOperationException where it does not.

The tests reach the pair reflectively for the same reason, which is
also how a Java 18 caller resolves it, and skip on a platform without
the methods.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CLnTBsvmYtxzNTWVGNyz33
@garydgregory
garydgregory merged commit 632cb04 into apache:main Sep 1, 2026
16 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