ppkarwasz opened a new pull request, #76:
URL: https://github.com/apache/commons-secure-xml/pull/76

   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](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]

Reply via email to