8319436: Proxy.newProxyInstance throws NPE if loader is null and inte…

…rface not visible from class loader

Reviewed-by: alanb

src/java.base/share/classes/java/lang/System.java

@@ -2667,7 +2667,7 @@ public StackWalker newStackWalkerInstance(Set<StackWalker.Option> options,
             }
 
             public String getLoaderNameID(ClassLoader loader) {
-                return loader.nameAndId();
+                return loader != null ? loader.nameAndId() : "null";
             }
 
             @Override

test/jdk/java/lang/reflect/Proxy/ClassRestrictions.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -22,13 +22,13 @@
  */
 
 /* @test
- * @bug 4227192 8004928 8072656
+ * @bug 4227192 8004928 8072656 8319436
  * @summary This is a test of the restrictions on the parameters that may
  * be passed to the Proxy.getProxyClass method.
  * @author Peter Jones
  *
  * @build ClassRestrictions
- * @run main ClassRestrictions
+ * @run junit ClassRestrictions
  */
 
 import java.io.File;
@@ -37,6 +37,13 @@
 import java.net.URLClassLoader;
 import java.net.URL;
 import java.nio.file.Paths;
+import java.util.List;
+import java.util.stream.Stream;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+import static org.junit.jupiter.api.Assertions.*;
 
 public class ClassRestrictions {
 
@@ -52,129 +59,65 @@ interface Bashful {
         void foo();
     }
 
-    public static final String nonPublicIntrfaceName = "java.util.zip.ZipConstants";
-
-    public static void main(String[] args) {
-
-        System.err.println(
-            "\nTest of restrictions on parameters to Proxy.getProxyClass\n");
-
-        try {
-            ClassLoader loader = ClassRestrictions.class.getClassLoader();
-            Class<?>[] interfaces;
-            Class<?> proxyClass;
-
-            /*
-             * All of the Class objects in the interfaces array must represent
-             * interfaces, not classes or primitive types.
-             */
-            try {
-                interfaces = new Class<?>[] { Object.class };
-                proxyClass = Proxy.getProxyClass(loader, interfaces);
-                throw new Error(
-                    "proxy class created with java.lang.Object as interface");
-            } catch (IllegalArgumentException e) {
-                e.printStackTrace();
-                System.err.println();
-                // assume exception is for intended failure
-            }
-            try {
-                interfaces = new Class<?>[] { Integer.TYPE };
-                proxyClass = Proxy.getProxyClass(loader, interfaces);
-                throw new Error(
-                    "proxy class created with int.class as interface");
-            } catch (IllegalArgumentException e) {
-                e.printStackTrace();
-                System.err.println();
-                // assume exception is for intended failure
-            }
-
-            /*
-             * No two elements in the interfaces array may refer to identical
-             * Class objects.
-             */
-            try {
-                interfaces = new Class<?>[] { Bar.class, Bar.class };
-                proxyClass = Proxy.getProxyClass(loader, interfaces);
-                throw new Error(
-                    "proxy class created with repeated interfaces");
-            } catch (IllegalArgumentException e) {
-                e.printStackTrace();
-                System.err.println();
-                // assume exception is for intended failure
-            }
-
-            /*
-             * All of the interfaces types must be visible by name though the
-             * specified class loader.
-             */
-            String[] cpaths = System.getProperty("test.classes", ".")
-                                    .split(File.pathSeparator);
-            URL[] urls = new URL[cpaths.length];
-            for (int i=0; i < cpaths.length; i++) {
-                urls[i] = Paths.get(cpaths[i]).toUri().toURL();
-            }
-            ClassLoader altLoader = new URLClassLoader(urls, null);
-            Class altBarClass;
-            altBarClass = Class.forName(Bar.class.getName(), false, altLoader);
-            try {
-                interfaces = new Class<?>[] { altBarClass };
-                proxyClass = Proxy.getProxyClass(loader, interfaces);
-                throw new Error(
-                    "proxy class created with interface " +
-                    "not visible to class loader");
-            } catch (IllegalArgumentException e) {
-                e.printStackTrace();
-                System.err.println();
-                // assume exception is for intended failure
-            }
+    private static final String TEST_CLASSES = System.getProperty("test.classes", ".");
+    private static final ClassLoader LOADER = ClassRestrictions.class.getClassLoader();
+
+    static Stream<List<Class<?>>> badProxyInterfaces() {
+        return Stream.of(
+                List.of(Object.class),          // proxy interface cannot be a class
+                List.of(int.class),             // proxy interface can't be primitive type
+                List.of(Bar.class, Bar.class),  // cannot have repeated interfaces
+                // two proxy interfaces have the method of same method name but different return type
+                List.of(Bar.class, Baz.class)
+        );
+    }
 
-            /*
-             * All non-public interfaces must be in the same package.
-             */
-            Class<?> nonPublic1 = Bashful.class;
-            Class<?> nonPublic2 = Class.forName(nonPublicIntrfaceName);
-            if (Modifier.isPublic(nonPublic2.getModifiers())) {
-                throw new Error(
-                    "Interface " + nonPublicIntrfaceName +
-                    " is public and need to be changed!");
-            }
-            try {
-                interfaces = new Class<?>[] { nonPublic1, nonPublic2 };
-                proxyClass = Proxy.getProxyClass(loader, interfaces);
-                throw new Error(
-                    "proxy class created with two non-public interfaces " +
-                    "in different packages");
-            } catch (IllegalArgumentException e) {
-                e.printStackTrace();
-                System.err.println();
-                // assume exception is for intended failure
-            }
+    /*
+     * Test cases for illegal proxy interfaces
+     */
+    @ParameterizedTest
+    @MethodSource("badProxyInterfaces")
+    void testForName(List<Class<?>> interfaces) {
+        assertThrows(IllegalArgumentException.class, () -> Proxy.getProxyClass(LOADER, interfaces.toArray(Class[]::new)));
+    }
 
-            /*
-             * No two interfaces may each have a method with the same name and
-             * parameter signature but different return type.
-             */
-            try {
-                interfaces = new Class<?>[] { Bar.class, Baz.class };
-                proxyClass = Proxy.getProxyClass(loader, interfaces);
-                throw new Error(
-                    "proxy class created with conflicting methods");
-            } catch (IllegalArgumentException e) {
-                e.printStackTrace();
-                System.err.println();
-                // assume exception is for intended failure
-            }
+    private static final String nonPublicIntrfaceName = "java.util.zip.ZipConstants";
+
+    /*
+     * All non-public interfaces must be in the same package.
+     */
+    @Test
+    void testNonPublicIntfs() throws Exception {
+        var nonPublic1 = Bashful.class;
+        var nonPublic2 = Class.forName(nonPublicIntrfaceName);
+        assertFalse(Modifier.isPublic(nonPublic2.getModifiers()),
+            "Interface " + nonPublicIntrfaceName + " is public and need to be changed!");
+        var interfaces = new Class<?>[] { nonPublic1, nonPublic2 };
+        assertThrows(IllegalArgumentException.class, () -> Proxy.getProxyClass(LOADER, interfaces));
+    }
 
-            /*
-             * All components of this test have passed.
-             */
-            System.err.println("\nTEST PASSED");
+    static Stream<ClassLoader> loaders() {
+        return Stream.of(null,
+                         ClassLoader.getPlatformClassLoader(),
+                         ClassLoader.getSystemClassLoader(),
+                         LOADER);
+    }
 
-        } catch (Throwable e) {
-            System.err.println("\nTEST FAILED:");
-            e.printStackTrace();
-            throw new Error("TEST FAILED: ", e);
+    /*
+     * All of the interfaces types must be visible by name though the
+     * specified class loader.
+     */
+    @ParameterizedTest
+    @MethodSource("loaders")
+    void testNonVisibleInterface(ClassLoader loader) throws Exception {
+        String[] cpaths = TEST_CLASSES.split(File.pathSeparator);
+        URL[] urls = new URL[cpaths.length];
+        for (int i = 0; i < cpaths.length; i++) {
+            urls[i] = Paths.get(cpaths[i]).toUri().toURL();
         }
+        var altLoader = new URLClassLoader(urls, null);
+        var altBarClass = Class.forName(Bar.class.getName(), false, altLoader);
+        var interfaces = new Class<?>[]{ altBarClass };
+        assertThrows(IllegalArgumentException.class, () -> Proxy.getProxyClass(loader, interfaces));
     }
 }