[12] RFR: 8214827: Incorrect call ClassLoaders.toFileURL("jrt:/java.compiler")

[Calvin Cheung]

Hi Jiangli,

1) systemDictionaryShared.cpp:

Should all those CHECK_(protection_domain) be simply CHECK_NH?
In get_shared_jar_manifest() of the same file, CHECK_NH is used. But in 
other functions, CHECK_(something) is being used.
Not sure which one is the preferred way?

2) ProtectionDomain.java:

58     TestCommon.run("-cp", appJar, "ProtDomain").assertNormalExit();

     You can also check the output by doing:
     TestCommon.run("-cp", appJar, 
"ProtDomain").assertNormalExit("Protection Domains match");
     Similarly for line 61.

65     TestCommon.run("-cp", appJar, 
"JimageClassProtDomain").assertNormalExit();

      You can check the output "shouldNotContain" by doing:
      TestCommon.run("-cp", appJar, "JimageClassProtDomain")
          .assertNormalExit(output -> output.shouldNotContain("Failed: 
Protection Domains do not match");

The remaining 3 files look good.

thanks,
Calvin

On 1/9/19, 2:15 PM, Jiangli Zhou wrote:
> Please review the bug fix for 
> SystemDictionaryShared::get_shared_protection_domain(). When 
> jrt:/<module> location is given, a null URL (due to the error in 
> ClassLoaders.toFileURL) is used for the CodeSource, which results a 
> wrong ProtectionDomain being used for the archived app/platform class 
> from the runtime image. The granted permission is minimum in that case.
>
> This turns out to be an issue that's hidden for a long time. The 
> JimageClassProtDomain.java test has been outdated and fails to detect 
> the issue (I'm the guilt one). Please see the comments in the bug for 
> more details. Thanks Martin for finding the issue, and David, Alan and 
> Mandy for discussions.
>
> webrev: http://cr.openjdk.java.net/~jiangli/8214827/webrev.01/
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8214827
>
> Tested with appcds tests locally. Tested tier1-2. Tier3 is still in 
> progress.
>
> Thanks,
>
> Jiangli
>