Re: RFR: 8283340: Remove support for -Dsun.misc.URLClassPath.disableJarChecking
On Fri, 18 Mar 2022 18:59:16 GMT, Sean Mullan wrote: >> I don't know what the motivation for this change is but there is more to >> this story and I think will require agreement from the security area before >> removing it. > > I agree with @AlanBateman. Please don't proceed with this fix until further > notice. @seanjmullan are there any updates here? Or is the suggestion to drop the PR, because the code is definitely still needed? Thanks! - PR: https://git.openjdk.java.net/jdk/pull/7861
Re: RFR: 8283340: Remove support for -Dsun.misc.URLClassPath.disableJarChecking
On Fri, 18 Mar 2022 08:39:46 GMT, Alan Bateman wrote: >> This change removes support for >> `-Dsun.misc.URLClassPath.disableJarChecking`. As discussed in the bug the >> feature doesn't current work, and only ever supported scenarios where a >> security manager is installed, so it seems safe to remove. > > I don't know what the motivation for this change is but there is more to this > story and I think will require agreement from the security area before > removing it. I agree with @AlanBateman. Please don't proceed with this fix until further notice. - PR: https://git.openjdk.java.net/jdk/pull/7861
Re: RFR: 8283340: Remove support for -Dsun.misc.URLClassPath.disableJarChecking
On Fri, 18 Mar 2022 08:39:46 GMT, Alan Bateman wrote: > I don't know what the motivation for this change is but there is more to this > story and I think will require agreement from the security area before > removing it. Thanks @AlanBateman. The motivation is that I was investigating an unrelated change to this code (a potential performance optimization), and it accidentally removed the logic that's catching and ignoring the `IOException`, and when I investigated this logic it looked unused and seemed like a cleanup opportunity. There don't seem to be any public tests for this feature. If it's security related I understand there may be other internal tests, and this PR may be a non-starter. I'd be interested in anything you can share about the story here, but if this code needs to stay for security reasons feel free to close the PR. Do you expect this will still be turned down once the `SecurityManager` remove is complete? Or does it make sense to think about generalize this logic so it works even if there isn't a `SecurityManager` enabled? - PR: https://git.openjdk.java.net/jdk/pull/7861
Re: RFR: 8283340: Remove support for -Dsun.misc.URLClassPath.disableJarChecking
On Thu, 17 Mar 2022 23:44:32 GMT, Liam Miller-Cushon wrote: > This change removes support for `-Dsun.misc.URLClassPath.disableJarChecking`. > As discussed in the bug the feature doesn't current work, and only ever > supported scenarios where a security manager is installed, so it seems safe > to remove. Also, I belatedly realized that I don't think this is actually a no-op: the exception is never shown to the user, but it will cause the jar to be skipped. It should be possible to write a test to exercise that, so I may look in to contributing that if there's interest. - PR: https://git.openjdk.java.net/jdk/pull/7861
Re: RFR: 8283340: Remove support for -Dsun.misc.URLClassPath.disableJarChecking
On Fri, 18 Mar 2022 04:01:05 GMT, Jiangli Zhou wrote:
>> This change removes support for
>> `-Dsun.misc.URLClassPath.disableJarChecking`. As discussed in the bug the
>> feature doesn't current work, and only ever supported scenarios where a
>> security manager is installed, so it seems safe to remove.
>
> src/java.base/share/classes/jdk/internal/loader/URLClassPath.java line 806:
>
>> 804:
>> 805: /* Throws if the given jar file is does not start with the
>> correct LOC */
>> 806: @SuppressWarnings("removal")
>
> I noticed the @SuppressWarnings("removal") when looking at the PR. It is
> added by
> https://github.com/openjdk/jdk/commit/6765f902505fbdd02f25b599f942437cd805cad1
> for [JDK-8266459](https://bugs.openjdk.java.net/browse/JDK-8266459). So
> looks like the method has been targeted for deprcating/removal.
Right, the suppression is for the use of `getSecurityManager` here.
-
PR: https://git.openjdk.java.net/jdk/pull/7861
Re: RFR: 8283340: Remove support for -Dsun.misc.URLClassPath.disableJarChecking
On Thu, 17 Mar 2022 23:44:32 GMT, Liam Miller-Cushon wrote: > This change removes support for `-Dsun.misc.URLClassPath.disableJarChecking`. > As discussed in the bug the feature doesn't current work, and only ever > supported scenarios where a security manager is installed, so it seems safe > to remove. I don't know what the motivation for this change is but there is more to this story and I think will require agreement from the security area before removing it. - PR: https://git.openjdk.java.net/jdk/pull/7861
Re: RFR: 8283340: Remove support for -Dsun.misc.URLClassPath.disableJarChecking
On Thu, 17 Mar 2022 23:44:32 GMT, Liam Miller-Cushon wrote:
> This change removes support for `-Dsun.misc.URLClassPath.disableJarChecking`.
> As discussed in the bug the feature doesn't current work, and only ever
> supported scenarios where a security manager is installed, so it seems safe
> to remove.
src/java.base/share/classes/jdk/internal/loader/URLClassPath.java line 806:
> 804:
> 805: /* Throws if the given jar file is does not start with the
> correct LOC */
> 806: @SuppressWarnings("removal")
I noticed the @SuppressWarnings("removal") when looking at the PR. It is added
by
https://github.com/openjdk/jdk/commit/6765f902505fbdd02f25b599f942437cd805cad1
for [JDK-8266459](https://bugs.openjdk.java.net/browse/JDK-8266459). So looks
like the method has been targeted for deprcating/removal.
-
PR: https://git.openjdk.java.net/jdk/pull/7861
RFR: 8283340: Remove support for -Dsun.misc.URLClassPath.disableJarChecking
This change removes support for `-Dsun.misc.URLClassPath.disableJarChecking`. As discussed in the bug the feature doesn't current work, and only ever supported scenarios where a security manager is installed, so it seems safe to remove. - Commit messages: - 8283340: Remove support for -Dsun.misc.URLClassPath.disableJarChecking Changes: https://git.openjdk.java.net/jdk/pull/7861/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7861=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8283340 Stats: 25 lines in 1 file changed: 0 ins; 20 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/7861.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7861/head:pull/7861 PR: https://git.openjdk.java.net/jdk/pull/7861
