Re: RFR 8064924: Update java.net.URL to work with modules
On 13 Feb 2015, at 16:11, Alan Bateman wrote: > On 10/02/2015 16:20, Chris Hegarty wrote: >> On 10 Feb 2015, at 11:35, Alan Bateman wrote: >> >>> On 09/02/2015 14:57, Chris Hegarty wrote: : Updated webrev [ spec and implementation] : http://cr.openjdk.java.net/~chegar/8064924/04/ >>> The updated javadoc looks good. I haven't had a chance yet to look at the >>> implementation but I think you will need to update >>> /make/common/CORE_PKGS.gmk for the spi package. >> Sorry, I have the change locally but forgot it. I updated the webrev with >> the changes to the top level repo ( below ): >> > I think the approach is good. > > One thing in the implementation is detection for recursive initialization. In > other areas of the platforms (ClassLoader.getSystemClassLoader and > FileSystem.getDefault) then an exception is thrown. In an earlier revision recursion detection threw an Exception, but I was inspired by the the lookup mechanism in Charset, which returns null when it encounters recursion. I chose the latter as it means that we do not need to filter out specific protocols from lookup, like ‘jar’. Returning null just falls back to searching the built-in handlers. > I wonder if we need this here too. If the initialization of a > URLStreamHandlerFactory is dependent on the deployment of another then it > could be very tricky to diagnose as there isn't (yet) any control on the > ordering. I had not thought of that scenario, and I’m not sure how it would work, at least with ServiceLoader. Do you think it is important ? > I'd probably use isOverridable or canOverride for the method name. I’ll update from overrideable(String) -> isOverridable(String) > In the spi package-info.java then I assume it time that the second paragraph > will need to change as there will be other provider interfaces. Right. I think it is ok for now, and when additional providers are added it can be updated then. Thanks, -Chris. > -Alan. >
Re: RFR 8064924: Update java.net.URL to work with modules
On 10/02/2015 16:20, Chris Hegarty wrote: On 10 Feb 2015, at 11:35, Alan Bateman wrote: On 09/02/2015 14:57, Chris Hegarty wrote: : Updated webrev [ spec and implementation] : http://cr.openjdk.java.net/~chegar/8064924/04/ The updated javadoc looks good. I haven't had a chance yet to look at the implementation but I think you will need to update /make/common/CORE_PKGS.gmk for the spi package. Sorry, I have the change locally but forgot it. I updated the webrev with the changes to the top level repo ( below ): I think the approach is good. One thing in the implementation is detection for recursive initialization. In other areas of the platforms (ClassLoader.getSystemClassLoader and FileSystem.getDefault) then an exception is thrown. I wonder if we need this here too. If the initialization of a URLStreamHandlerFactory is dependent on the deployment of another then it could be very tricky to diagnose as there isn't (yet) any control on the ordering. I'd probably use isOverridable or canOverride for the method name. In the spi package-info.java then I assume it time that the second paragraph will need to change as there will be other provider interfaces. -Alan.
Re: RFR 8064924: Update java.net.URL to work with modules
On 10 Feb 2015, at 11:35, Alan Bateman wrote: > On 09/02/2015 14:57, Chris Hegarty wrote: >> : >> >> Updated webrev [ spec and implementation] : >> http://cr.openjdk.java.net/~chegar/8064924/04/ >> > The updated javadoc looks good. I haven't had a chance yet to look at the > implementation but I think you will need to update > /make/common/CORE_PKGS.gmk for the spi package. Sorry, I have the change locally but forgot it. I updated the webrev with the changes to the top level repo ( below ): $ hg diff make/common/CORE_PKGS.gmk diff --git a/make/common/CORE_PKGS.gmk b/make/common/CORE_PKGS.gmk --- a/make/common/CORE_PKGS.gmk +++ b/make/common/CORE_PKGS.gmk @@ -103,6 +103,7 @@ java.lang.reflect \ java.math \ java.net \ +java.net.spi \ java.nio \ java.nio.channels \ java.nio.channels.spi \ -Chris. > -Alan.
Re: RFR 8064924: Update java.net.URL to work with modules
On 09/02/2015 14:57, Chris Hegarty wrote: : Updated webrev [ spec and implementation] : http://cr.openjdk.java.net/~chegar/8064924/04/ The updated javadoc looks good. I haven't had a chance yet to look at the implementation but I think you will need to update /make/common/CORE_PKGS.gmk for the spi package. -Alan.
Re: RFR 8064924: Update java.net.URL to work with modules
Thank you for the comments Alan. On 06/02/15 10:20, Alan Bateman wrote: On 04/02/2015 15:11, Chris Hegarty wrote: Agreed. Updated in-place http://cr.openjdk.java.net/~chegar/8064924/03/specdiff/overview-summary.html I think the approach and naming is good. A few small comments on the wording: 1. "used to locate URLStreamHandlerProvider providers" - the wording hints as further redirection, maybe it would be better as "URLStreamHandlerProvider implementations". Updated. ( result of previous refactoring ) 2. "the ordering that providers are located" - maybe this should be "the order that providers are located". Updated. Thanks 3. "Some protocol, that are fundamental ...". Here's a re-worded statement to consider: "Some protocol handlers, for example those used for loading platform classes or classes on the class path, may not be overridden. The details of such restrictions, and when those restrictions apply (during initialization of the runtime for example), are implementation specific and therefore not specified". This is better. Updated. One other thing in this area is setURLStreamHandlerFactory(null). Long standing behavior is to remove the system-wide URLStreamHandlerFactory, should this continue? setURLStreamHandlerFactory can be called many times with 'null', but once it is called with a non-null arg then it can never be called again ( it will throw ). What I found was that code setting the pkg prefix property would call setURLStreamHandlerFactory(null) to clear the protocol handlers cache, and subsequent lookups would then consult the updated property. This is really only interesting if you plan to override existing protocols, and appears benign. I have not changed this in the latest webrev. It seems like a corner case, and could be argued either way. The spec is less than clear about the expected behavior of this method when you pass it null. Updated webrev [ spec and implementation] : http://cr.openjdk.java.net/~chegar/8064924/04/ Regarding jaxws, I suspect that I can simply reverse the changes in the webrev, and do nothing. Running on pre-JDK9 the property will continue to be set and used, on JDK9 and later the property will be set but ignored. I don't see that it is necessary anyway. Looks like technical debt. I'll file a separate issue for Miran to follow up and verify this. -Chris. -Alan
Re: RFR 8064924: Update java.net.URL to work with modules
On 04/02/2015 15:11, Chris Hegarty wrote: Agreed. Updated in-place http://cr.openjdk.java.net/~chegar/8064924/03/specdiff/overview-summary.html I think the approach and naming is good. A few small comments on the wording: 1. "used to locate URLStreamHandlerProvider providers" - the wording hints as further redirection, maybe it would be better as "URLStreamHandlerProvider implementations". 2. "the ordering that providers are located" - maybe this should be "the order that providers are located". 3. "Some protocol, that are fundamental ...". Here's a re-worded statement to consider: "Some protocol handlers, for example those used for loading platform classes or classes on the class path, may not be overridden. The details of such restrictions, and when those restrictions apply (during initialization of the runtime for example), are implementation specific and therefore not specified". One other thing in this area is setURLStreamHandlerFactory(null). Long standing behavior is to remove the system-wide URLStreamHandlerFactory, should this continue? -Alan
Re: RFR 8064924: Update java.net.URL to work with modules
On 02/04/2015 05:19 PM, Chris Hegarty wrote: On 04/02/15 16:01, Peter Levart wrote: On 02/04/2015 04:45 PM, Alan Bateman wrote: On 04/02/2015 15:10, Weijun Wang wrote: It should be checked, otherwise a non-initialized parent object comes into being. In general then permission checks in constructors are a bad idea but we have an established idiom that has the no-arg constructor invoking a static method that does the permission check. -Alan There is an alternative. The com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.checkMBeanTrustPermission(Class) checks the permission without running any code of the class. I did look at checking the permission of the impl class, but with service loader this would have to be done after the instance is created, which in itself should not be a big issue here as the provider would not be used by java.net.URL, but the side-effect is that we end up creating an instance that will not be used. > But if verify-er checks constructor flow then this is as good as proposed. I am happy that the verifier will catch this. This is an established idiom that has been used in other, more sensitive, areas. I'll see if I can prove this with an out-of-order compilation, or something. -Chris. The idiom with constructor is also more correct from security standpoint as all the constructors in class hierarchy are on the call-stack when the check is made. If particular classes from the hierarchy have separate protection domains, the check covers all of them and that's correct since code from any or all of them could be involved in later URL constructions. Peter
Re: RFR 8064924: Update java.net.URL to work with modules
On 04/02/15 16:01, Peter Levart wrote: On 02/04/2015 04:45 PM, Alan Bateman wrote: On 04/02/2015 15:10, Weijun Wang wrote: It should be checked, otherwise a non-initialized parent object comes into being. In general then permission checks in constructors are a bad idea but we have an established idiom that has the no-arg constructor invoking a static method that does the permission check. -Alan There is an alternative. The com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.checkMBeanTrustPermission(Class) checks the permission without running any code of the class. I did look at checking the permission of the impl class, but with service loader this would have to be done after the instance is created, which in itself should not be a big issue here as the provider would not be used by java.net.URL, but the side-effect is that we end up creating an instance that will not be used. > But if verify-er checks constructor flow then this is as good as proposed. I am happy that the verifier will catch this. This is an established idiom that has been used in other, more sensitive, areas. I'll see if I can prove this with an out-of-order compilation, or something. -Chris.
Re: RFR 8064924: Update java.net.URL to work with modules
On 02/04/2015 04:45 PM, Alan Bateman wrote: On 04/02/2015 15:10, Weijun Wang wrote: It should be checked, otherwise a non-initialized parent object comes into being. In general then permission checks in constructors are a bad idea but we have an established idiom that has the no-arg constructor invoking a static method that does the permission check. -Alan There is an alternative. The com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.checkMBeanTrustPermission(Class) checks the permission without running any code of the class. But if verify-er checks constructor flow then this is as good as proposed. Peter
Re: RFR 8064924: Update java.net.URL to work with modules
On 04/02/2015 15:10, Weijun Wang wrote: It should be checked, otherwise a non-initialized parent object comes into being. In general then permission checks in constructors are a bad idea but we have an established idiom that has the no-arg constructor invoking a static method that does the permission check. -Alan
Re: RFR 8064924: Update java.net.URL to work with modules
It should be checked, otherwise a non-initialized parent object comes into being. On 2/4/2015 22:38, Peter Levart wrote: Just a thought,... Is JVM bytecode verifier checking that a constructor chains to super constructor? If yes, we are ok. If not, specially crafted bytecode could skip security checks.
Re: RFR 8064924: Update java.net.URL to work with modules
Agreed. Updated in-place http://cr.openjdk.java.net/~chegar/8064924/03/specdiff/overview-summary.html -Chris. On 04/02/15 14:44, Alan Bateman wrote: On 04/02/2015 14:29, Peter Levart wrote: : I agree that this is the most appropriate way with which you can force some provider's class code (the constructor) in the call stack so that you get correct AccessControlContext to check against. But the name starts to sound like German words. :-) Wouldn't URLStreamHandlerProvider be enough? Since it's a provider for URLStreamHandlers and not URLStreamHandlerFactories. If URLStreamHandlerFactory were an abstract class rather than an interface then this would have been easy. I agree the naming is awkward as this abstract class is a URLStreamHandlerFactory rather than a provider of URLStreamHandlerFactory objects. Renaming it to URLStreamHandlerProvider seems a good idea. -Alan
Re: RFR 8064924: Update java.net.URL to work with modules
On 04/02/2015 14:29, Peter Levart wrote: : I agree that this is the most appropriate way with which you can force some provider's class code (the constructor) in the call stack so that you get correct AccessControlContext to check against. But the name starts to sound like German words. :-) Wouldn't URLStreamHandlerProvider be enough? Since it's a provider for URLStreamHandlers and not URLStreamHandlerFactories. If URLStreamHandlerFactory were an abstract class rather than an interface then this would have been easy. I agree the naming is awkward as this abstract class is a URLStreamHandlerFactory rather than a provider of URLStreamHandlerFactory objects. Renaming it to URLStreamHandlerProvider seems a good idea. -Alan
Re: RFR 8064924: Update java.net.URL to work with modules
On 02/04/2015 03:29 PM, Peter Levart wrote:
On 02/04/2015 02:54 PM, Chris Hegarty wrote:
On 02/02/15 20:52, Alan Bateman wrote:
I'm happy with this approach. One outstanding point from the discussion
is whether the URLStreamHandlerFactory implementation will need to be
granted RuntimePermission("setFactory"), if so then this will need
to go
into the javadoc.
I think that we should check "setFactory" for providers located
by service loader. The most appropriate way to do this is with
a new public abstract type who's constructor checks this,
similar to CharsetProvider ( and others ).
The updated spec revision, link below, defines the new public
provider type in a new networking service provider interface
package, java.net.spi. Future service provider interfaces for
the java.net package should be defined into this package.
http://cr.openjdk.java.net/~chegar/8064924/03/specdiff/overview-summary.html
Note:
The spec does no mention what happens when a SecurityException is
thrown ( it will be the cause of the SCE ). The prototype
implementation I have will will swallow it, and continue searching,
as is done for CharsetProviders. A SecurityException, locating
a provider, should not prevent the the app from making progress.
The provider will just not be available.
-Chris.
Hi Chris,
I agree that this is the most appropriate way with which you can force
some provider's class code (the constructor) in the call stack so that
you get correct AccessControlContext to check against. But the name
starts to sound like German words. :-)
Wouldn't URLStreamHandlerProvider be enough? Since it's a provider for
URLStreamHandlers and not URLStreamHandlerFactories.
Regards, Peter
Just a thought,...
Is JVM bytecode verifier checking that a constructor chains to super
constructor? If yes, we are ok. If not, specially crafted bytecode could
skip security checks.
Peter
Re: RFR 8064924: Update java.net.URL to work with modules
On 02/04/2015 02:54 PM, Chris Hegarty wrote:
On 02/02/15 20:52, Alan Bateman wrote:
I'm happy with this approach. One outstanding point from the discussion
is whether the URLStreamHandlerFactory implementation will need to be
granted RuntimePermission("setFactory"), if so then this will need to go
into the javadoc.
I think that we should check "setFactory" for providers located
by service loader. The most appropriate way to do this is with
a new public abstract type who's constructor checks this,
similar to CharsetProvider ( and others ).
The updated spec revision, link below, defines the new public
provider type in a new networking service provider interface
package, java.net.spi. Future service provider interfaces for
the java.net package should be defined into this package.
http://cr.openjdk.java.net/~chegar/8064924/03/specdiff/overview-summary.html
Note:
The spec does no mention what happens when a SecurityException is
thrown ( it will be the cause of the SCE ). The prototype
implementation I have will will swallow it, and continue searching,
as is done for CharsetProviders. A SecurityException, locating
a provider, should not prevent the the app from making progress.
The provider will just not be available.
-Chris.
Hi Chris,
I agree that this is the most appropriate way with which you can force
some provider's class code (the constructor) in the call stack so that
you get correct AccessControlContext to check against. But the name
starts to sound like German words. :-)
Wouldn't URLStreamHandlerProvider be enough? Since it's a provider for
URLStreamHandlers and not URLStreamHandlerFactories.
Regards, Peter
Re: RFR 8064924: Update java.net.URL to work with modules
On 02/02/15 20:52, Alan Bateman wrote:
I'm happy with this approach. One outstanding point from the discussion
is whether the URLStreamHandlerFactory implementation will need to be
granted RuntimePermission("setFactory"), if so then this will need to go
into the javadoc.
I think that we should check "setFactory" for providers located
by service loader. The most appropriate way to do this is with
a new public abstract type who's constructor checks this,
similar to CharsetProvider ( and others ).
The updated spec revision, link below, defines the new public
provider type in a new networking service provider interface
package, java.net.spi. Future service provider interfaces for
the java.net package should be defined into this package.
http://cr.openjdk.java.net/~chegar/8064924/03/specdiff/overview-summary.html
Note:
The spec does no mention what happens when a SecurityException is
thrown ( it will be the cause of the SCE ). The prototype
implementation I have will will swallow it, and continue searching,
as is done for CharsetProviders. A SecurityException, locating
a provider, should not prevent the the app from making progress.
The provider will just not be available.
-Chris.
Re: RFR 8064924: Update java.net.URL to work with modules
On 02/02/2015 11:26, Chris Hegarty wrote:
I agree with Alan, addURLStreamHandlerFactory is probably an
attractive nuisance. It is not strictly necessary to achieve the goal
here; replace the problematic ( with modules ) system property with a
service lookup.
For now, I'd like to move this issue forward without the additional
new public method. We can have deploy use setURLSHF(), and document
the compatibility issue if applets/Webstart apps also try to set a
factory. We can revisit this later in 9, if it becomes an issue.
Updated specdiff:
http://cr.openjdk.java.net/~chegar/8064924/02/specdiff/
I this revision, I omitted the implementation changes, so we can agree
the spec changes first. They are much simpler now.
I'm happy with this approach. One outstanding point from the discussion
is whether the URLStreamHandlerFactory implementation will need to be
granted RuntimePermission("setFactory"), if so then this will need to go
into the javadoc.
-Alan.
Re: RFR 8064924: Update java.net.URL to work with modules
On Feb 2, 2015, at 12:34 PM, Chris Hegarty wrote:
> On 02/02/15 11:00, Paul Sandoz wrote:
>>
>> On Jan 30, 2015, at 10:10 PM, Alan Bateman wrote:
>>
>>> On 30/01/2015 15:35, Chris Hegarty wrote:
:
Update webrev and spec diffs:
http://cr.openjdk.java.net/~chegar/8064924/01/
>>> I think the javadoc looks much better now, thanks.
>>>
>>
>> Minor comments in URL:
>>
>> Java doc on URL constructor:
>>
>> 269 * If the previous step fails to find a protocol handler, then
>> the
>> 270 * {@linkplain java.util.ServiceLoader ServiceLoader} mechanism
>> is used
>> 271 * to locate a {@code URLStreamHandlerFactory} provider using
>> the system
>>
>> "to locate {@code URLStreamHandlerFactory} providers using..."
>
> Thanks Paul, Updated in the latest version ( see other mail ).
>
Ok.
>>
>> The code might read better if you place the stuff above the synchronized
>> block within it (assuming that causes no issues):
>>
>> if (!checkedFactory) {
>> handle = handlerFromSettableFactory(protocol);
>> if (handle == NULL_HANDLER) handle = null;
>> }
>> if (handle == null) {
>> handler = defaultFactory.createURLStreamHandler(protocol);
>> }
>> if (handle != null) {
>> handlers.put(protocol, handler);
>> }
>
> That is a possibility, but great effort has been put into this area recently,
> by Peter, to attempt to keep the lookup of handlers lock free ( typically a
> volatile read, only requiring the lock when updating the cache ).
>
> We would also not want to hold the lock while performing the service lookup,
> in which case we may end up with two synchronization blocks, so as to keep
> the service lookup as lazy as possible. Or have I missed our point?
>
It was specifically to do with the optimistic call to
defaultFactory.createURLStreamHandle the result of which might be thrown away,
but probably not. The method handlerFromSettableFactory is anyway called from
within the synchronized so it did not seem a big deal to pull in default
factory call and simplify the logic.
Paul.
Re: RFR 8064924: Update java.net.URL to work with modules
On 02/02/15 11:00, Paul Sandoz wrote:
On Jan 30, 2015, at 10:10 PM, Alan Bateman wrote:
On 30/01/2015 15:35, Chris Hegarty wrote:
:
Update webrev and spec diffs:
http://cr.openjdk.java.net/~chegar/8064924/01/
I think the javadoc looks much better now, thanks.
Minor comments in URL:
Java doc on URL constructor:
269 * If the previous step fails to find a protocol handler, then the
270 * {@linkplain java.util.ServiceLoader ServiceLoader} mechanism
is used
271 * to locate a {@code URLStreamHandlerFactory} provider using the
system
"to locate {@code URLStreamHandlerFactory} providers using..."
Thanks Paul, Updated in the latest version ( see other mail ).
Using the plural reads better given what follows:
272 * class loader. The ordering that providers are located is
273 * implementation specific, and an implementation is free to
cache the
274 * located providers. A {@linkplain
java.util.ServiceConfigurationError
275 * ServiceConfigurationError}, {@code Error} or {@code
RuntimeException}
276 * thrown from the {@code createURLStreamHandler}, if
encountered, will
277 * be propagated to the calling thread. The {@code
278 * createURLStreamHandler} method of each provider, if
instantiated, is
279 * invoked, with the protocol string, until a provider returns
non-null,
280 * or all providers have been exhausted.
In getURLStreamHandler method:
1313 if (handler == null) {
1314 // Try the built-in protocol handler
1315 handler = defaultFactory.createURLStreamHandler(protocol);
1316 }
1317
1318 synchronized (streamHandlerLock) {
1319
1320 URLStreamHandler handler2 = null;
1321
1322 // Check again with hashtable just in case another
1323 // thread created a handler since we last checked
1324 handler2 = handlers.get(protocol);
1325
1326 if (handler2 != null) {
1327 return handler2;
1328 }
1329
1330 // Check with factory if another thread set a
1331 // factory since our last check
1332 if (!checkedWithFactory) {
1333 handler2 = handlerFromSettableFactory(protocol);
1334 }
1335
1336 if (!(handler2 == null || handler2 == NULL_HANDLER)) {
1337 // The handler from the factory must be given more
1338 // importance. Discard the default handler that
1339 // this thread created.
1340 handler = handler2;
1341 }
1342
1343 // Insert this handler into the hashtable
1344 if (handler != null) {
1345 handlers.put(protocol, handler);
1346 }
1347
1348 }
The code might read better if you place the stuff above the synchronized block
within it (assuming that causes no issues):
if (!checkedFactory) {
handle = handlerFromSettableFactory(protocol);
if (handle == NULL_HANDLER) handle = null;
}
if (handle == null) {
handler = defaultFactory.createURLStreamHandler(protocol);
}
if (handle != null) {
handlers.put(protocol, handler);
}
That is a possibility, but great effort has been put into this area
recently, by Peter, to attempt to keep the lookup of handlers lock free
( typically a volatile read, only requiring the lock when updating the
cache ).
We would also not want to hold the lock while performing the service
lookup, in which case we may end up with two synchronization blocks, so
as to keep the service lookup as lazy as possible. Or have I missed our
point?
-Chris.
Paul.
Re: RFR 8064924: Update java.net.URL to work with modules
I agree with Alan, addURLStreamHandlerFactory is probably an attractive nuisance. It is not strictly necessary to achieve the goal here; replace the problematic ( with modules ) system property with a service lookup. For now, I'd like to move this issue forward without the additional new public method. We can have deploy use setURLSHF(), and document the compatibility issue if applets/Webstart apps also try to set a factory. We can revisit this later in 9, if it becomes an issue. Updated specdiff: http://cr.openjdk.java.net/~chegar/8064924/02/specdiff/ I this revision, I omitted the implementation changes, so we can agree the spec changes first. They are much simpler now. -Chris. On 02/02/15 09:15, Chris Hegarty wrote: Just catching up… On 2 Feb 2015, at 08:42, Alan Bateman wrote: On 01/02/2015 10:45, Peter Levart wrote: : I see. But if URLStreamHandlerFactories are only supposed to be located via the system class loader, is that different from what we have now when URLStreamHandlers are being located directly via class name construction (prefix + protocol + .Handler) and loaded via the system class loader? They have to be public classes with public default constructors, yes. But so have to be URLStreamHandlerFactories too, to be loadable by ServiceLoader. Are we just trying to get rid of old mechanism or is there something I'm missing? The legacy mechanism isn't going to work with modules as currently envisaged (the protocol handler factory class may be visible and be a public type but it doesn't mean that it will accessible when we have module boundaries). The intention is that ServiceLoader will work with modules and so it should be possible to deploy modules that provide implementations of URLStreamHandlerFactory. Thanks Alan. This may not have been clear from my original mail. : If that's the reason for addURLStreamHandlerFactory (to support apps deployed to containers and which use setURLStreamHandlerFactory) then there should probably be some mechanism to allow those apps to call setURLStreamHandlerFactory but don't allow them to override handlers for protocols that container is trying to enforce (like jar). So factory set by setURLStreamHandlerFactory should not be evaluated 1st. What about the following order of evaluation: 1. default system factory if protocols are "file" or "jrt" 2. factories registered via ServiceLoader or addURLStreamHandlerFactory or equivalent 3. factory set by setURLStreamHandlerFactory if any 4. default system factory Applications using setURLStreamHandlerFactory expect their protocol handler factory to be used and we don't want to break this. So I think this has to be called first, the only exception is the core protocols (file and jrt mostly) that cannot be overridden. So in your order then I think #2 and #3 should be reserved. Right, and if you reverse #2 and #3 above, you get the same ordering as in the original proposal. I deliberately avoided any behavioural changes with setURLStreamHandlerFactory. I hope I have not missed other comments ( please send them again, if they have not already been addressed ), but I think the only outstanding issue, with regard to the spec changes, is the permission on addURLStreamHandlerFactory. Maybe this method should define a new permission, rather than reusing setFactory, it could be RuntimePermission(“addFactory”). It would help further differentiate the addURLSHF and setURLSHF methods. -Chris. -Alan
Re: RFR 8064924: Update java.net.URL to work with modules
On Jan 30, 2015, at 10:10 PM, Alan Bateman wrote:
> On 30/01/2015 15:35, Chris Hegarty wrote:
>> :
>>
>> Update webrev and spec diffs:
>>http://cr.openjdk.java.net/~chegar/8064924/01/
>>
> I think the javadoc looks much better now, thanks.
>
Minor comments in URL:
Java doc on URL constructor:
269 * If the previous step fails to find a protocol handler, then the
270 * {@linkplain java.util.ServiceLoader ServiceLoader} mechanism is
used
271 * to locate a {@code URLStreamHandlerFactory} provider using the
system
"to locate {@code URLStreamHandlerFactory} providers using..."
Using the plural reads better given what follows:
272 * class loader. The ordering that providers are located is
273 * implementation specific, and an implementation is free to cache
the
274 * located providers. A {@linkplain
java.util.ServiceConfigurationError
275 * ServiceConfigurationError}, {@code Error} or {@code
RuntimeException}
276 * thrown from the {@code createURLStreamHandler}, if encountered,
will
277 * be propagated to the calling thread. The {@code
278 * createURLStreamHandler} method of each provider, if
instantiated, is
279 * invoked, with the protocol string, until a provider returns
non-null,
280 * or all providers have been exhausted.
In getURLStreamHandler method:
1313 if (handler == null) {
1314 // Try the built-in protocol handler
1315 handler = defaultFactory.createURLStreamHandler(protocol);
1316 }
1317
1318 synchronized (streamHandlerLock) {
1319
1320 URLStreamHandler handler2 = null;
1321
1322 // Check again with hashtable just in case another
1323 // thread created a handler since we last checked
1324 handler2 = handlers.get(protocol);
1325
1326 if (handler2 != null) {
1327 return handler2;
1328 }
1329
1330 // Check with factory if another thread set a
1331 // factory since our last check
1332 if (!checkedWithFactory) {
1333 handler2 = handlerFromSettableFactory(protocol);
1334 }
1335
1336 if (!(handler2 == null || handler2 == NULL_HANDLER)) {
1337 // The handler from the factory must be given more
1338 // importance. Discard the default handler that
1339 // this thread created.
1340 handler = handler2;
1341 }
1342
1343 // Insert this handler into the hashtable
1344 if (handler != null) {
1345 handlers.put(protocol, handler);
1346 }
1347
1348 }
The code might read better if you place the stuff above the synchronized block
within it (assuming that causes no issues):
if (!checkedFactory) {
handle = handlerFromSettableFactory(protocol);
if (handle == NULL_HANDLER) handle = null;
}
if (handle == null) {
handler = defaultFactory.createURLStreamHandler(protocol);
}
if (handle != null) {
handlers.put(protocol, handler);
}
Paul.
Re: RFR 8064924: Update java.net.URL to work with modules
Just catching up… On 2 Feb 2015, at 08:42, Alan Bateman wrote: > On 01/02/2015 10:45, Peter Levart wrote: >> : >> >> I see. But if URLStreamHandlerFactories are only supposed to be located via >> the system class loader, is that different from what we have now when >> URLStreamHandlers are being located directly via class name construction >> (prefix + protocol + .Handler) and loaded via the system class loader? They >> have to be public classes with public default constructors, yes. But so have >> to be URLStreamHandlerFactories too, to be loadable by ServiceLoader. >> >> Are we just trying to get rid of old mechanism or is there something I'm >> missing? > > The legacy mechanism isn't going to work with modules as currently envisaged > (the protocol handler factory class may be visible and be a public type but > it doesn't mean that it will accessible when we have module boundaries). The > intention is that ServiceLoader will work with modules and so it should be > possible to deploy modules that provide implementations of > URLStreamHandlerFactory. Thanks Alan. This may not have been clear from my original mail. >> : >> >> If that's the reason for addURLStreamHandlerFactory (to support apps >> deployed to containers and which use setURLStreamHandlerFactory) then there >> should probably be some mechanism to allow those apps to call >> setURLStreamHandlerFactory but don't allow them to override handlers for >> protocols that container is trying to enforce (like jar). So factory set by >> setURLStreamHandlerFactory should not be evaluated 1st. What about the >> following order of evaluation: >> >> 1. default system factory if protocols are "file" or "jrt" >> 2. factories registered via ServiceLoader or addURLStreamHandlerFactory or >> equivalent >> 3. factory set by setURLStreamHandlerFactory if any >> 4. default system factory > > Applications using setURLStreamHandlerFactory expect their protocol handler > factory to be used and we don't want to break this. So I think this has to be > called first, the only exception is the core protocols (file and jrt mostly) > that cannot be overridden. So in your order then I think #2 and #3 should be > reserved. Right, and if you reverse #2 and #3 above, you get the same ordering as in the original proposal. I deliberately avoided any behavioural changes with setURLStreamHandlerFactory. I hope I have not missed other comments ( please send them again, if they have not already been addressed ), but I think the only outstanding issue, with regard to the spec changes, is the permission on addURLStreamHandlerFactory. Maybe this method should define a new permission, rather than reusing setFactory, it could be RuntimePermission(“addFactory”). It would help further differentiate the addURLSHF and setURLSHF methods. -Chris. > -Alan
Re: RFR 8064924: Update java.net.URL to work with modules
On 01/02/2015 10:45, Peter Levart wrote: : I see. But if URLStreamHandlerFactories are only supposed to be located via the system class loader, is that different from what we have now when URLStreamHandlers are being located directly via class name construction (prefix + protocol + .Handler) and loaded via the system class loader? They have to be public classes with public default constructors, yes. But so have to be URLStreamHandlerFactories too, to be loadable by ServiceLoader. Are we just trying to get rid of old mechanism or is there something I'm missing? The legacy mechanism isn't going to work with modules as currently envisaged (the protocol handler factory class may be visible and be a public type but it doesn't mean that it will accessible when we have module boundaries). The intention is that ServiceLoader will work with modules and so it should be possible to deploy modules that provide implementations of URLStreamHandlerFactory. : If that's the reason for addURLStreamHandlerFactory (to support apps deployed to containers and which use setURLStreamHandlerFactory) then there should probably be some mechanism to allow those apps to call setURLStreamHandlerFactory but don't allow them to override handlers for protocols that container is trying to enforce (like jar). So factory set by setURLStreamHandlerFactory should not be evaluated 1st. What about the following order of evaluation: 1. default system factory if protocols are "file" or "jrt" 2. factories registered via ServiceLoader or addURLStreamHandlerFactory or equivalent 3. factory set by setURLStreamHandlerFactory if any 4. default system factory Applications using setURLStreamHandlerFactory expect their protocol handler factory to be used and we don't want to break this. So I think this has to be called first, the only exception is the core protocols (file and jrt mostly) that cannot be overridden. So in your order then I think #2 and #3 should be reserved. -Alan
Re: RFR 8064924: Update java.net.URL to work with modules
On 02/01/2015 10:56 AM, Alan Bateman wrote: I see. But isn't URL.setURLStreamHandlerFactory() enough for that purpose? It can only be set once, but there can only be *one* container that wants it's jar protocol handler configured system-wide. This a good question as it brings up the scenario that Chris is trying to address by introducing addURLStreamHandlerFactory. The concern is where the container starts an application that also uses the legacy setURLStreamHandlerFactory. The container is trying not to cause the application to fail with an error. Looking at it again then I think addURLStreamHandlerFactory is going to be more an attractive nuisance that expected, despite the @apiNote and we need to drop this part of the solution. There are compatibility and migration concerns but I don't think they are significant in the overall context of a major release. -Alan Here's another idea: If URLStreamHandlerFactories could (optionally) announce supported protocols, then existing method setURLStreamHandlerFactory could allow setting multiple factories as long as they don't conflict. A container could set a factory that announces "jar" protocol only and that would not conflict with application deployed in container setting an URLStreamHandlerFactory that doesn't announce anything, so will be called for anything but "jar" in this situation. Here's what I mean: http://cr.openjdk.java.net/~plevart/jdk9-dev/URLStreamHandlerFactory/webrev.02/ Regards, Peter
Re: RFR 8064924: Update java.net.URL to work with modules
On 02/01/2015 10:56 AM, Alan Bateman wrote: On 31/01/2015 23:47, Peter Levart wrote: I agree. Putting the order on the SPI API is not the right solution. The order should be configured in one place. But there needs to be some kind of handle each service exposes for order configuration to reference. So one idea how to extend the ServiceLoader mechanism is this: I think this is a much bigger topic and one that the URL usage isn't one of the better examples to explore it (the reason being that providers of URLStreamHandlerFactory are proposed to only be located via the system class loader, no proposal to allow for bundling with an application which is where the real scope issues come to the fore). I see. But if URLStreamHandlerFactories are only supposed to be located via the system class loader, is that different from what we have now when URLStreamHandlers are being located directly via class name construction (prefix + protocol + .Handler) and loaded via the system class loader? They have to be public classes with public default constructors, yes. But so have to be URLStreamHandlerFactories too, to be loadable by ServiceLoader. Are we just trying to get rid of old mechanism or is there something I'm missing? I see. But isn't URL.setURLStreamHandlerFactory() enough for that purpose? It can only be set once, but there can only be *one* container that wants it's jar protocol handler configured system-wide. This a good question as it brings up the scenario that Chris is trying to address by introducing addURLStreamHandlerFactory. The concern is where the container starts an application that also uses the legacy setURLStreamHandlerFactory. The container is trying not to cause the application to fail with an error. Looking at it again then I think addURLStreamHandlerFactory is going to be more an attractive nuisance that expected, despite the @apiNote and we need to drop this part of the solution. There are compatibility and migration concerns but I don't think they are significant in the overall context of a major release. If that's the reason for addURLStreamHandlerFactory (to support apps deployed to containers and which use setURLStreamHandlerFactory) then there should probably be some mechanism to allow those apps to call setURLStreamHandlerFactory but don't allow them to override handlers for protocols that container is trying to enforce (like jar). So factory set by setURLStreamHandlerFactory should not be evaluated 1st. What about the following order of evaluation: 1. default system factory if protocols are "file" or "jrt" 2. factories registered via ServiceLoader or addURLStreamHandlerFactory or equivalent 3. factory set by setURLStreamHandlerFactory if any 4. default system factory Peter -Alan
Re: RFR 8064924: Update java.net.URL to work with modules
On 31/01/2015 23:47, Peter Levart wrote: I agree. Putting the order on the SPI API is not the right solution. The order should be configured in one place. But there needs to be some kind of handle each service exposes for order configuration to reference. So one idea how to extend the ServiceLoader mechanism is this: I think this is a much bigger topic and one that the URL usage isn't one of the better examples to explore it (the reason being that providers of URLStreamHandlerFactory are proposed to only be located via the system class loader, no proposal to allow for bundling with an application which is where the real scope issues come to the fore). I see. But isn't URL.setURLStreamHandlerFactory() enough for that purpose? It can only be set once, but there can only be *one* container that wants it's jar protocol handler configured system-wide. This a good question as it brings up the scenario that Chris is trying to address by introducing addURLStreamHandlerFactory. The concern is where the container starts an application that also uses the legacy setURLStreamHandlerFactory. The container is trying not to cause the application to fail with an error. Looking at it again then I think addURLStreamHandlerFactory is going to be more an attractive nuisance that expected, despite the @apiNote and we need to drop this part of the solution. There are compatibility and migration concerns but I don't think they are significant in the overall context of a major release. -Alan
Re: RFR 8064924: Update java.net.URL to work with modules
Hi Bernd,
On 02/01/2015 01:14 AM, Bernd wrote:
Hello,
What about simply rejecting overlapping programmatic registrations,
This can't work by registering URLStreamHandlerFactory objects as they
don't announce their supported protocols. Perhaps registering
URLStreamHandler objects directly would be an option as in:
public static void setURLStreamHandler(String protocol, URLStreamHandler
urlStreamHandler) throws IllegalStateException
But this per-protocol registration (or any programmatic registration
that overrides previous defaults) has a drawback that you can end up
with URL instances for the same protocol but different URLStreamHandler,
depending on when they are constructed, so it can happen that
URL("http://www.google.com/";) is not equal to
URL("http://www.google.com/";). Sometimes you actually want that (see
Alan's note about loading a "jar" URLStreamHandler deployed in a jar
file), but otherwise it gives you a very fragile system that depends on
initialization order. Programmatic registration should be reserved for
the master of the universe (to be performed before the universe is
born). Otherwise, special protocol handlers are better registered
declaratively.
and for the service loader using the natural order (class loader
search order). I guess most extensions register new schemes only, as
it was not easy before in such a central shared system component. an
ordering api might not really help. Its might be better to allow
specific versions like a factory or even something thread local?
(Similar to jndi enc)
If you want to have multiple universes in the same VM where each has
it's own view of per-protocol URLStreamHandlers, then you must ensure
that URL instances constructed in each universe, don't come in contact
with each other or you will be subjected to similar issues as when two
Class objects with same class name are not equal to each other, because
each is loaded by different ClassLoader.
Regards, Peter
Bernd
Am 01.02.2015 00:48 schrieb "Peter Levart" >:
Hi Alan,
On 01/31/2015 10:33 PM, Alan Bateman wrote:
On 31/01/2015 19:42, Peter Levart wrote:
Hi Chris,
I looked at your solution to make URLStreamHandlerFactory
interface a service provider using ServiceLoader API and in
addition adding new URL static method to programmaticaly
register URLStreamHandlerFactories. There are a couple of issues
with your approach I'd like to discuss.
The programmatic approach using static URL method does give you
some means of order in which registered
URLStreamHandlerFactories are tried to create URLStreamHandler
for particular protocol - the registration order. It means that
this method should only be called by one "party" in the VM or
else it is hard to control the order of registration.
ServiceLoader is a declarative approach, but does not give you
order by default. Also, your ServiceLoader approach gives a way
for URLStreamHandlerFactories to register in the system without
security checks. If a particular
META-INF/services/java.net.URLStreamHandlerFactory is found,
it's content is used to load a class and instantiate a factory
which is used in URL constructors then. Previously, one had to
have a "setFactory" permission to set the
URLStreamHandlerFactory or appropriate PropertyPermission for
seting the package prefix property. This can be fixed.
:
Anyway, I think there is an alternative to programmatic
registration approach of URLStreamHandlerFactories. Using just
ServiceLoader and a new default method on
URLStreamHandlerFactory interface to provide order. Here's what
I'm thinking about:
http://cr.openjdk.java.net/~plevart/jdk9-dev/URLStreamHandlerFactory/webrev.01/
Re: RFR 8064924: Update java.net.URL to work with modules
Hello,
What about simply rejecting overlapping programmatic registrations, and for
the service loader using the natural order (class loader search order). I
guess most extensions register new schemes only, as it was not easy before
in such a central shared system component. an ordering api might not really
help. Its might be better to allow specific versions like a factory or even
something thread local? (Similar to jndi enc)
Bernd
Am 01.02.2015 00:48 schrieb "Peter Levart" :
> Hi Alan,
>
> On 01/31/2015 10:33 PM, Alan Bateman wrote:
>
> On 31/01/2015 19:42, Peter Levart wrote:
>
> Hi Chris,
>
> I looked at your solution to make URLStreamHandlerFactory interface a
> service provider using ServiceLoader API and in addition adding new URL
> static method to programmaticaly register URLStreamHandlerFactories. There
> are a couple of issues with your approach I'd like to discuss.
>
> The programmatic approach using static URL method does give you some means
> of order in which registered URLStreamHandlerFactories are tried to create
> URLStreamHandler for particular protocol - the registration order. It means
> that this method should only be called by one "party" in the VM or else it
> is hard to control the order of registration.
>
> ServiceLoader is a declarative approach, but does not give you order by
> default. Also, your ServiceLoader approach gives a way for
> URLStreamHandlerFactories to register in the system without security
> checks. If a particular META-INF/services/java.net.URLStreamHandlerFactory
> is found, it's content is used to load a class and instantiate a factory
> which is used in URL constructors then. Previously, one had to have a
> "setFactory" permission to set the URLStreamHandlerFactory or appropriate
> PropertyPermission for seting the package prefix property. This can be
> fixed.
>
> :
>
> Anyway, I think there is an alternative to programmatic registration
> approach of URLStreamHandlerFactories. Using just ServiceLoader and a new
> default method on URLStreamHandlerFactory interface to provide order.
> Here's what I'm thinking about:
>
>
> http://cr.openjdk.java.net/~plevart/jdk9-dev/URLStreamHandlerFactory/webrev.01/
>
>
> I don't think we should we expose ordering values in
> URLStreamHandlerFactory, it looks a bit odd and not clear how an
> implementation can choose a useful value. There is a general issue that
> ServiceLoader doesn't currently support a means to order service providers
> but I think we can re-examine that when we move to modules and and linking.
> That is, have a consistent way to configure ordering that we can use
> everywhere that ServiceLoader is used rather than doing one-off solutions.
>
>
> I agree. Putting the order on the SPI API is not the right solution. The
> order should be configured in one place. But there needs to be some kind of
> handle each service exposes for order configuration to reference. So one
> idea how to extend the ServiceLoader mechanism is this:
>
> create a special class-scope runtime annotation...
>
> public @interface ServiceProvider {
> String name();
> }
>
> ...with which service implementation classes can optionally be annotated.
> This could enable ServiceLoader API extension like:
>
> public static ServiceLoader load(Class service, String
> serviceProviderName)
>
> that would return an Iterable over implementations that are annotated with
> a particular @ServiceProvider(name = ...) annotation (similar to security
> Providers but simpler).
>
> In addition one could specify a system property with the key being
> prefixed by service interface FQ class name, like:
>
>
> java.net.URLStreamHandlerFactory.serviceLoaderOrder=providerName1,providerName2,providerName3,...
>
>
>
>
> The other thing is that it's not clear how this would work for a factory
> for the jar protocol that is itself deployed in a JAR file on the class
> path. This is important for containers that want to do their own caching
> and they want their jar protocol handler configured system-wide before
> starting any applications. It's also part of the motive for the
> addURLStreamHandlerFactory in the original proposal.
>
>
> I see. But isn't URL.setURLStreamHandlerFactory() enough for that purpose?
> It can only be set once, but there can only be *one* container that wants
> it's jar protocol handler configured system-wide.
>
> Regards, Peter
>
>
> I think you have good point with the setFactory permission, that does need
> to be looked at.
>
> -Alan.
>
>
>
Re: RFR 8064924: Update java.net.URL to work with modules
Hi Alan,
On 01/31/2015 10:33 PM, Alan Bateman wrote:
On 31/01/2015 19:42, Peter Levart wrote:
Hi Chris,
I looked at your solution to make URLStreamHandlerFactory interface a
service provider using ServiceLoader API and in addition adding new
URL static method to programmaticaly register
URLStreamHandlerFactories. There are a couple of issues with your
approach I'd like to discuss.
The programmatic approach using static URL method does give you some
means of order in which registered URLStreamHandlerFactories are
tried to create URLStreamHandler for particular protocol - the
registration order. It means that this method should only be called
by one "party" in the VM or else it is hard to control the order of
registration.
ServiceLoader is a declarative approach, but does not give you order
by default. Also, your ServiceLoader approach gives a way for
URLStreamHandlerFactories to register in the system without security
checks. If a particular
META-INF/services/java.net.URLStreamHandlerFactory is found, it's
content is used to load a class and instantiate a factory which is
used in URL constructors then. Previously, one had to have a
"setFactory" permission to set the URLStreamHandlerFactory or
appropriate PropertyPermission for seting the package prefix
property. This can be fixed.
:
Anyway, I think there is an alternative to programmatic registration
approach of URLStreamHandlerFactories. Using just ServiceLoader and a
new default method on URLStreamHandlerFactory interface to provide
order. Here's what I'm thinking about:
http://cr.openjdk.java.net/~plevart/jdk9-dev/URLStreamHandlerFactory/webrev.01/
I don't think we should we expose ordering values in
URLStreamHandlerFactory, it looks a bit odd and not clear how an
implementation can choose a useful value. There is a general issue
that ServiceLoader doesn't currently support a means to order service
providers but I think we can re-examine that when we move to modules
and and linking. That is, have a consistent way to configure ordering
that we can use everywhere that ServiceLoader is used rather than
doing one-off solutions.
I agree. Putting the order on the SPI API is not the right solution. The
order should be configured in one place. But there needs to be some kind
of handle each service exposes for order configuration to reference. So
one idea how to extend the ServiceLoader mechanism is this:
create a special class-scope runtime annotation...
public @interface ServiceProvider {
String name();
}
...with which service implementation classes can optionally be
annotated. This could enable ServiceLoader API extension like:
public static ServiceLoader load(Class service, String
serviceProviderName)
that would return an Iterable over implementations that are annotated
with a particular @ServiceProvider(name = ...) annotation (similar to
security Providers but simpler).
In addition one could specify a system property with the key being
prefixed by service interface FQ class name, like:
java.net.URLStreamHandlerFactory.serviceLoaderOrder=providerName1,providerName2,providerName3,...
The other thing is that it's not clear how this would work for a
factory for the jar protocol that is itself deployed in a JAR file on
the class path. This is important for containers that want to do their
own caching and they want their jar protocol handler configured
system-wide before starting any applications. It's also part of the
motive for the addURLStreamHandlerFactory in the original proposal.
I see. But isn't URL.setURLStreamHandlerFactory() enough for that
purpose? It can only be set once, but there can only be *one* container
that wants it's jar protocol handler configured system-wide.
Regards, Peter
I think you have good point with the setFactory permission, that does
need to be looked at.
-Alan.
Re: RFR 8064924: Update java.net.URL to work with modules
On 31/01/2015 19:42, Peter Levart wrote: Hi Chris, I looked at your solution to make URLStreamHandlerFactory interface a service provider using ServiceLoader API and in addition adding new URL static method to programmaticaly register URLStreamHandlerFactories. There are a couple of issues with your approach I'd like to discuss. The programmatic approach using static URL method does give you some means of order in which registered URLStreamHandlerFactories are tried to create URLStreamHandler for particular protocol - the registration order. It means that this method should only be called by one "party" in the VM or else it is hard to control the order of registration. ServiceLoader is a declarative approach, but does not give you order by default. Also, your ServiceLoader approach gives a way for URLStreamHandlerFactories to register in the system without security checks. If a particular META-INF/services/java.net.URLStreamHandlerFactory is found, it's content is used to load a class and instantiate a factory which is used in URL constructors then. Previously, one had to have a "setFactory" permission to set the URLStreamHandlerFactory or appropriate PropertyPermission for seting the package prefix property. This can be fixed. : Anyway, I think there is an alternative to programmatic registration approach of URLStreamHandlerFactories. Using just ServiceLoader and a new default method on URLStreamHandlerFactory interface to provide order. Here's what I'm thinking about: http://cr.openjdk.java.net/~plevart/jdk9-dev/URLStreamHandlerFactory/webrev.01/ I don't think we should we expose ordering values in URLStreamHandlerFactory, it looks a bit odd and not clear how an implementation can choose a useful value. There is a general issue that ServiceLoader doesn't currently support a means to order service providers but I think we can re-examine that when we move to modules and and linking. That is, have a consistent way to configure ordering that we can use everywhere that ServiceLoader is used rather than doing one-off solutions. The other thing is that it's not clear how this would work for a factory for the jar protocol that is itself deployed in a JAR file on the class path. This is important for containers that want to do their own caching and they want their jar protocol handler configured system-wide before starting any applications. It's also part of the motive for the addURLStreamHandlerFactory in the original proposal. I think you have good point with the setFactory permission, that does need to be looked at. -Alan.
Re: RFR 8064924: Update java.net.URL to work with modules
Hi Chris, I looked at your solution to make URLStreamHandlerFactory interface a service provider using ServiceLoader API and in addition adding new URL static method to programmaticaly register URLStreamHandlerFactories. There are a couple of issues with your approach I'd like to discuss. The programmatic approach using static URL method does give you some means of order in which registered URLStreamHandlerFactories are tried to create URLStreamHandler for particular protocol - the registration order. It means that this method should only be called by one "party" in the VM or else it is hard to control the order of registration. ServiceLoader is a declarative approach, but does not give you order by default. Also, your ServiceLoader approach gives a way for URLStreamHandlerFactories to register in the system without security checks. If a particular META-INF/services/java.net.URLStreamHandlerFactory is found, it's content is used to load a class and instantiate a factory which is used in URL constructors then. Previously, one had to have a "setFactory" permission to set the URLStreamHandlerFactory or appropriate PropertyPermission for seting the package prefix property. This can be fixed. The java.protocol.handler.pkgs system property manipulation in HttpSOAPConnection has a couple of issues as it is now (without your changes): - first, the property can be set too late, when https protocol has already been used in URL constructions before any HttpSOAPConnection is used. The URLStreamHandler cached in URL will be used for ever regardless of HttpSOAPConnection setting the property later. - second, the HttpSOAPConnection.initHttps is racy - it bases it's decision to instantiate and add a security Provider on the presence/absence of a particular package in the value of java.protocol.handler.pkgs system property, which it modifies afterwards. Concurrent calls to initHttps (which is called from HttpSOAPConnection.doGet and .post) can lead to multiple security Providers instantiations and adds... Your question "why is jaxws using the old https client ??" is right on the spot. Is it just because nobody has updated it yet? Why does it have to choose the URLStreamHandlers based on JVM it is executing on (doesn't IBM JVM already arrange for it's own HTTPS support by default?). Is it maybe the security Provider that is dependent on these *old* HTTPS implementations because it is using their internal APIs? This needs to be cleared, I think. Anyway, I think there is an alternative to programmatic registration approach of URLStreamHandlerFactories. Using just ServiceLoader and a new default method on URLStreamHandlerFactory interface to provide order. Here's what I'm thinking about: http://cr.openjdk.java.net/~plevart/jdk9-dev/URLStreamHandlerFactory/webrev.01/ You see, even tests can be made to use this approach. I think declarative approach is better because it doesn't depend on initialization order. Additional ideas you can take from this for your patch are the following: - the way URLStreamHandlerFactories obtained from ServiceLoader are security-checked before used. The implementation class / code source has to be given "setFactory" permission if such factory is to be considered at all. - the way URLStreamHandlers are cached using ConcurrentHashMap instead of Hashtable which simplifies caching "atomicity" checks and does not use lock(s) on fast-path. So what do you think of ServiceLoader-only approach? Regards, Peter On 01/30/2015 02:36 PM, Chris Hegarty wrote: This is phase 1, of getting java.net.URL work with modules. Being able to effectively specify URL protocol handler factories as fully qualified class names, through the 'java.protocol.handler.pkgs' system property is problematic. It requires the protocol handler factory implementation class to be public and accessible, as the current implementation tries to instantiate it through core reflection. Since the protocol handler factory must be an implementation of a URLStreamHandlerFactory, it lends itself to being retrofitted with a ServiceLoader lookup. Note, the 'java.protocol.handler.pkgs' system property mechanism predates ServiceLoader. URL protocol handlers would most likely have used service loader if it were available at the time. Some URL protocol handlers are fundamental to the platform itself, like 'file' and 'jar', it is not appropriate to attempt a service loader lookup for these, as they may lead to recursive initialization issues. However, Java Plugin, Webstart, and possibly other containers, do override the 'jar' handler. A new API will be provided for this purpose. Providing an API solution should not interfere with system initialization as it will only be called after the system comes up and user code is executing. The 'file' protocol handler factory will no longer be overridable, and the system property will no longer be consulted. Webrev and spec diffs: h
Re: RFR 8064924: Update java.net.URL to work with modules
On 30/01/2015 15:35, Chris Hegarty wrote: : Update webrev and spec diffs: http://cr.openjdk.java.net/~chegar/8064924/01/ I think the javadoc looks much better now, thanks. -Alan
Re: RFR 8064924: Update java.net.URL to work with modules
Thanks for the comments Alan… On 30 Jan 2015, at 14:32, Alan Bateman wrote: > On 30/01/2015 13:36, Chris Hegarty wrote: >> This is phase 1, of getting java.net.URL work with modules. >> >> Being able to effectively specify URL protocol handler factories as fully >> qualified class names, through the 'java.protocol.handler.pkgs' system >> property is problematic. It requires the protocol handler factory >> implementation class to be public and accessible, as the current >> implementation tries to instantiate it through core reflection. Since the >> protocol handler factory must be an implementation of a >> URLStreamHandlerFactory, it lends itself to being retrofitted with a >> ServiceLoader lookup. Note, the 'java.protocol.handler.pkgs' system property >> mechanism predates ServiceLoader. URL protocol handlers would most likely >> have used service loader if it were available at the time. >> >> Some URL protocol handlers are fundamental to the platform itself, like >> 'file' and 'jar', it is not appropriate to attempt a service loader lookup >> for these, as they may lead to recursive initialization issues. However, >> Java Plugin, Webstart, and possibly other containers, do override the 'jar' >> handler. A new API will be provided for this purpose. Providing an API >> solution should not interfere with system initialization as it will only be >> called after the system comes up and user code is executing. >> >> The 'file' protocol handler factory will no longer be overridable, and the >> system property will no longer be consulted. >> >> Webrev and spec diffs: >> http://cr.openjdk.java.net/~chegar/8064924/00/ >> >> I want to agree the approach and spec first, so that the spec changes can be >> submitted for approval. A comprehensive test will be added before the code >> changes are finalised. >> >> > I've had a number of off-list discussions with Chris on this and I think the > proposal and the ordering is good. It preserves behavior for those setting > the system-wide URLStreamHandlerFactory with setURLStreamHandlerFactory and > only really creates a migration issue for those setting the JDK 1.0/1.1 era > pkgs property property. > > One thing that I wasn't aware of is the hairy in SAAJ. That is some thing > that will need to coordinated with upstream and hopefully Miroslav can help > on that. As upstream might still have a constraint that the code compile with > JDK 7 and JDK 8 then some adjustments might be required. Right. My changes are mainly a placeholder to remind me to revisit this. I suspect this code will continue to set the system property and call the new method reflectively, if available. I’ll agree this approach with Miran before finalising the changes. > In any, the focus now is agreeing the API changes before digging into the > code changes. > > I've read through the javadoc and it looks good. A couple of suggestions: > > - step 2: alternative wording: "The createURLStreamHandler method of each > factory is invoked, in registration order, with the protocol string, until a > factory returns non-null”. Thanks, updated. > - step 3: "ServiceLoader mechanism is used" might be better than > "ServiceLoader mechanism tries”. Updated. > - step 3: "ServiceConfigurationErrors", maybe this should be singular as the > first will cause the search to bail. Related to this is > createURLStreamHandler terminating with an error or runtime exception, you > may want to mention that too. I added a note about this. > In the @apiNote in addURLStreamHandlerFactory then "containers" might not be > understood by the reader. I don't have an alternative but maybe make it clear > that this is for system-wide configuration and not intended to be used by > transient applications. I made a change to indicate that this is system-wide. Update webrev and spec diffs: http://cr.openjdk.java.net/~chegar/8064924/01/ -Chris. > -Alan
Re: RFR 8064924: Update java.net.URL to work with modules
On 30/01/2015 13:36, Chris Hegarty wrote: This is phase 1, of getting java.net.URL work with modules. Being able to effectively specify URL protocol handler factories as fully qualified class names, through the 'java.protocol.handler.pkgs' system property is problematic. It requires the protocol handler factory implementation class to be public and accessible, as the current implementation tries to instantiate it through core reflection. Since the protocol handler factory must be an implementation of a URLStreamHandlerFactory, it lends itself to being retrofitted with a ServiceLoader lookup. Note, the 'java.protocol.handler.pkgs' system property mechanism predates ServiceLoader. URL protocol handlers would most likely have used service loader if it were available at the time. Some URL protocol handlers are fundamental to the platform itself, like 'file' and 'jar', it is not appropriate to attempt a service loader lookup for these, as they may lead to recursive initialization issues. However, Java Plugin, Webstart, and possibly other containers, do override the 'jar' handler. A new API will be provided for this purpose. Providing an API solution should not interfere with system initialization as it will only be called after the system comes up and user code is executing. The 'file' protocol handler factory will no longer be overridable, and the system property will no longer be consulted. Webrev and spec diffs: http://cr.openjdk.java.net/~chegar/8064924/00/ I want to agree the approach and spec first, so that the spec changes can be submitted for approval. A comprehensive test will be added before the code changes are finalised. I've had a number of off-list discussions with Chris on this and I think the proposal and the ordering is good. It preserves behavior for those setting the system-wide URLStreamHandlerFactory with setURLStreamHandlerFactory and only really creates a migration issue for those setting the JDK 1.0/1.1 era pkgs property property. One thing that I wasn't aware of is the hairy in SAAJ. That is some thing that will need to coordinated with upstream and hopefully Miroslav can help on that. As upstream might still have a constraint that the code compile with JDK 7 and JDK 8 then some adjustments might be required. In any, the focus now is agreeing the API changes before digging into the code changes. I've read through the javadoc and it looks good. A couple of suggestions: - step 2: alternative wording: "The createURLStreamHandler method of each factory is invoked, in registration order, with the protocol string, until a factory returns non-null". - step 3: "ServiceLoader mechanism is used" might be better than "ServiceLoader mechanism tries". - step 3: "ServiceConfigurationErrors", maybe this should be singular as the first will cause the search to bail. Related to this is createURLStreamHandler terminating with an error or runtime exception, you may want to mention that too. In the @apiNote in addURLStreamHandlerFactory then "containers" might not be understood by the reader. I don't have an alternative but maybe make it clear that this is for system-wide configuration and not intended to be used by transient applications. -Alan
RFR 8064924: Update java.net.URL to work with modules
This is phase 1, of getting java.net.URL work with modules. Being able to effectively specify URL protocol handler factories as fully qualified class names, through the 'java.protocol.handler.pkgs' system property is problematic. It requires the protocol handler factory implementation class to be public and accessible, as the current implementation tries to instantiate it through core reflection. Since the protocol handler factory must be an implementation of a URLStreamHandlerFactory, it lends itself to being retrofitted with a ServiceLoader lookup. Note, the 'java.protocol.handler.pkgs' system property mechanism predates ServiceLoader. URL protocol handlers would most likely have used service loader if it were available at the time. Some URL protocol handlers are fundamental to the platform itself, like 'file' and 'jar', it is not appropriate to attempt a service loader lookup for these, as they may lead to recursive initialization issues. However, Java Plugin, Webstart, and possibly other containers, do override the 'jar' handler. A new API will be provided for this purpose. Providing an API solution should not interfere with system initialization as it will only be called after the system comes up and user code is executing. The 'file' protocol handler factory will no longer be overridable, and the system property will no longer be consulted. Webrev and spec diffs: http://cr.openjdk.java.net/~chegar/8064924/00/ I want to agree the approach and spec first, so that the spec changes can be submitted for approval. A comprehensive test will be added before the code changes are finalised. -Chris.
