RFR: 8319436: Proxy.newProxyInstance throws NPE if loader is null and interface not visible from class loader

[Alan Bateman]

On Mon, 6 Nov 2023 19:12:28 GMT, Mandy Chung <mchung at openjdk.org> wrote:

> This is a regression caused by JDK-8302791.   IAE should be thrown when an interface is not visible to the given class loader but NPE is thrown instead when the loader is null.   The boot loader has no name and so the fix will print `null` in the exception message.   `test/jdk/java/lang/reflect/Proxy/ClassRestrictions.java` is converted to junit and updated to test IAE thrown because proxy interface is not visible to null loader.

test/jdk/java/lang/reflect/Proxy/ClassRestrictions.java line 61:

> 59:     }
> 60: 
> 61:     static Stream<Arguments> proxyInterfaces() {

These are cases where IAE is expected so maybe the method source should be badProxyInterfaces to make it clearer that they are expected to fail?

test/jdk/java/lang/reflect/Proxy/ClassRestrictions.java line 90:

> 88:         } catch (IllegalArgumentException e) {
> 89:             System.err.println(e.getMessage());
> 90:             // assume exception is for intended failure

The "throw new Error(message)" should be replaced with fail(message) or use assertThrows(IllegalArgument.class, () -> Proxy.getProxyClass(...)).

test/jdk/java/lang/reflect/Proxy/ClassRestrictions.java line 105:

> 103:         if (Modifier.isPublic(nonPublic2.getModifiers())) {
> 104:             throw new Error("Interface " + nonPublicIntrfaceName +
> 105:                             " is public and need to be changed!");

You could use assertEquals here, or at least replace the throw new Error with fail, up to you.

test/jdk/java/lang/reflect/Proxy/ClassRestrictions.java line 125:

> 123: 
> 124:     private static final String[] CPATHS = System.getProperty("test.classes", ".")
> 125:                                                  .split(File.pathSeparator);

It might be a bit clearer to define TEST_CLASSES to be the value System.getProperty("test.classes", "."), and split it in testNonVisibleInterface.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16525#discussion_r1383882661
PR Review Comment: https://git.openjdk.org/jdk/pull/16525#discussion_r1383885042
PR Review Comment: https://git.openjdk.org/jdk/pull/16525#discussion_r1383886740
PR Review Comment: https://git.openjdk.org/jdk/pull/16525#discussion_r1383896386