Hello Eirik,
Looking at the commit history, that comment was added when, very long
back, the JarFileFactory used to return a java.security.Permission for
the cached JarFile. That code was then changed but the comment was left
around which I believe was an oversight.
Like you note, in its current form it is not accurate. It can be
removed. If we do introduce a new comment for that block (which I think
we should), then something like the following might be useful:
/*
* If we are using a cached jarFile, which could have been added to the
cache
* by someone else, then set our jarFileURLConnection to the one from
the cache
* and update it to use the same value for useCaches as the one we had
determined
* in our constructor.
*/
-Jaikiran
On 20/11/24 10:38 pm, Eirik Bjørsnøs wrote:
Hi,
At the end of s.n.w.p.j.JarURLConnection::connect, I see this stray,
permission related comment:
/* we also ask the factory the permission that was required
* to get the jarFile, and set it as our permission.
*/
if (useCaches) {
boolean oldUseCaches = jarFileURLConnection.getUseCaches();
jarFileURLConnection = factory.getConnection(jarFile);
jarFileURLConnection.setUseCaches(oldUseCaches);
}
The "factory" here is JarFileFactory, which recently went through a
SM-cleanup PR which removed permission checking.
The comment seems to have existed since the initial load, but have
moved around a bit since then.
It does not seem to make a lot of sense now, and digging through
history I'm also struggling a bit to understand how it made sense even
in the initial load.
What's the best action here? Delete the comment? Replace it with
something more appropriate? Wait for pending JEP486 cleanup to take
care of it?
Thanks,
Eirik.
