Hi Claes,
I wonder if initialize() should check the state of
the readOnly() flag - and if that's true, call
perms.setReadOnly() ?
see SecureClassLoader::getProtectionDomain(..)
best regards,
-- daniel
On 15/08/2019 13:44, Claes Redestad wrote:
Hi,
On 2019-08-15 12:56, Alan Bateman wrote:
On 15/08/2019 11:03, Claes Redestad wrote:
Hi,
by resolving permissions for code source URLs lazily, we can reduce
early class loading during bootstrap, which improves footprint, startup
and reduces the typical bootstrap dependency graph.
Bug: https://bugs.openjdk.java.net/browse/JDK-8229773
Webrev: ...
:
I think the approach is okay as URL::openConnection doesn't actually
open the connection to the resource and the creation of the
URLStreamdHandler shouldn't depend on permissions granted to the
caller. If a handler needs permissions when creating the URLConnection
then it should do so in a privileged block.
thanks!
I checked most of the handlers and the openConnection implementations
and couldn't find any path that isn't either free of permission checks
or doing permission sensitive steps in a privileged block already. Many
handlers are already potentially created lazily after a SM has already
been installed, so I don't think we need additional tests for this.
I think it would be a bit cleaner if the supporting class would lazily
add the permissions for a CodeSource to the collection. That is,
create it with a permissions collection and code source rather than a
URL to match SecureClassLoader::getPermissions. You could potentially
use it in URLClassLoader getPermission(CodeSource) method too.
That'd be cool. The logic in URLClassLoader is mostly a super-set of the
logic I'm hoisting from BuiltinClassLoader here, but the logic in
URLClassLoader also has a security check acting on the ACC of the
classloader which would be hard to move to the supporting class, and it
seems that check need to happen eagerly.
I'll readjust the API to wrap the CodeSource rather than the URL, but I
think we should leave URLClassLoader alone for now.
In System.setSecurityManager then you might need
DefaultFileSystemProvider.theFileSystem() to ensure that the default
file system is fully initialized before setting the SM.
Right, doesn't make much of a difference since all providers currently
set up their singleton file system on creation, but using theFileSystem
better captures intent.
A minor nit this adds a spurious import BuiltinClassLoader. Also it
can import the sun.security.uti class to be consistent with the
existing code.
Cleaned up imports, too:
http://cr.openjdk.java.net/~redestad/8229773/webrev.01/
/Claes
