paulrutter commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5531786684
One correction before I open the plurl issue: the `shouldHandle(URL)` shape
I proposed doesn't work, and it took running plurl's own test suite to see why.
`PlurlRootURLStreamHandler.parseURL` pins the selected handler onto the URL
through the private `java.net.URL.handler` field. Wherever that field is
writable — plurl's own test build passes `--add-opens java.base/java.net`, and
the Felix framework bundle declares `Add-opens: java.base/java.net` — the swap
succeeds, plurl steps aside, and `openConnection` never comes back through it.
So a URL-based hook is unreachable in exactly the deployments that need it. My
Felix test passed only because that JVM had no add-opens, which is the opposite
of reassuring.
Selection has to be decidable at the moment plurl chooses, i.e. mid-parse,
when the URL has no host yet. The spec does exist then:
```java
default boolean shouldHandle(String protocol, String spec) { return false; }
```
consulted in `findFactory` before the call stack is walked, with
`PlurlFactoryHolder` delegating it the way it already delegates
`shouldHandle(Class)` — without that, `findFactory` only ever asks the holders,
which answer with the interface default.
On your compatibility point: the signature uses only `java.lang` types, so
the reflective path finds it on a factory compiled against a different copy of
the package, and a factory that predates the method is tolerated rather than
throwing — the lookup is cached, since it sits on the URL parsing path.
Existing factories are unaffected either way. To be straight about the limit
though: this only helps when the *installed* router is new enough to consult
it. If an older plurl wins the install, Felix falls back to today's behaviour,
and nothing on the factory side can change that.
- [`81442bc`](https://github.com/apache/felix-dev/commit/81442bc1bd) — the
plurl change, vendored copy only
- [`07d572a`](https://github.com/apache/felix-dev/commit/07d572a503) — the
Felix side, now claiming by UUID read from the spec
51/51 in the plurl repo, including a new `PlurlURLSelectionTest` that fails
without the patch (both URLs served by the first-added factory) and passes with
it. Felix's suite is unchanged. I'll raise the issue and PR upstream with that
test included.
--
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]
