8156142: ModuleReader instances don't always throw NPE for passed null args

Reviewed-by: chegar, mchung

jdk/src/java.base/share/classes/java/lang/module/ModuleReader.java

@@ -30,6 +30,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.net.URI;
 import java.nio.ByteBuffer;
+import java.util.Objects;
 import java.util.Optional;
 
 
@@ -163,9 +164,12 @@ public interface ModuleReader extends Closeable {
      * @param  bb
      *         The byte buffer to release
      *
-     * @implSpec The default implementation does nothing.
+     * @implSpec The default implementation doesn't do anything except check
+     * if the byte buffer is null.
      */
-    default void release(ByteBuffer bb) { }
+    default void release(ByteBuffer bb) {
+        Objects.requireNonNull(bb);
+    }
 
     /**
      * Closes the module reader. Once closed then subsequent calls to locate or

jdk/src/java.base/share/classes/java/lang/module/SystemModuleFinder.java

@@ -272,9 +272,9 @@ class SystemModuleFinder implements ModuleFinder {
          * if not found.
          */
         private ImageLocation findImageLocation(String name) throws IOException {
+            Objects.requireNonNull(name);
             if (closed)
                 throw new IOException("ModuleReader is closed");
-
             if (imageReader != null) {
                 return imageReader.findLocation(module, name);
             } else {
@@ -322,6 +322,7 @@ class SystemModuleFinder implements ModuleFinder {
 
         @Override
         public void release(ByteBuffer bb) {
+            Objects.requireNonNull(bb);
             ImageReader.releaseByteBuffer(bb);
         }
 

jdk/test/java/lang/module/ModuleReader/ModuleReaderTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2016, 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
@@ -38,6 +38,7 @@ import java.io.InputStream;
 import java.lang.module.ModuleFinder;
 import java.lang.module.ModuleReader;
 import java.lang.module.ModuleReference;
+import java.lang.reflect.Module;
 import java.net.URI;
 import java.net.URL;
 import java.net.URLConnection;
@@ -64,16 +65,24 @@ public class ModuleReaderTest {
     private static final Path SRC_DIR    = Paths.get(TEST_SRC, "src");
     private static final Path MODS_DIR   = Paths.get("mods");
 
+    // the module name of the base module
+    private static final String BASE_MODULE = "java.base";
+
     // the module name of the test module
     private static final String TEST_MODULE = "m";
 
+    // resources in the base module
+    private static final String[] BASE_RESOURCES = {
+        "java/lang/Object.class"
+    };
+
     // resources in test module (can't use module-info.class as a test
     // resource as it will be modified by the jmod tool)
-    private static final String[] RESOURCES = {
+    private static final String[] TEST_RESOURCES = {
         "p/Main.class"
     };
 
-    // a resource that is not in the test module
+    // a resource that is not in the base or test module
     private static final String NOT_A_RESOURCE = "NotAResource";
 
 
@@ -89,7 +98,74 @@ public class ModuleReaderTest {
 
 
     /**
-     * Test exploded module
+     * Test ModuleReader to module in runtime image
+     */
+    public void testImage() throws Exception {
+
+        ModuleFinder finder = ModuleFinder.ofSystem();
+        ModuleReference mref = finder.find(BASE_MODULE).get();
+        ModuleReader reader = mref.open();
+
+        try (reader) {
+
+            for (String name : BASE_RESOURCES) {
+                byte[] expectedBytes;
+                Module baseModule = Object.class.getModule();
+                try (InputStream in = baseModule.getResourceAsStream(name)) {
+                    expectedBytes = in.readAllBytes();
+                }
+
+                testFind(reader, name, expectedBytes);
+                testOpen(reader, name, expectedBytes);
+                testRead(reader, name, expectedBytes);
+
+            }
+
+            // test "not found"
+            assertFalse(reader.find(NOT_A_RESOURCE).isPresent());
+            assertFalse(reader.open(NOT_A_RESOURCE).isPresent());
+            assertFalse(reader.read(NOT_A_RESOURCE).isPresent());
+
+
+            // test nulls
+            try {
+                reader.find(null);
+                assertTrue(false);
+            } catch (NullPointerException expected) { }
+
+            try {
+                reader.open(null);
+                assertTrue(false);
+            } catch (NullPointerException expected) { }
+
+            try {
+                reader.read(null);
+                assertTrue(false);
+            } catch (NullPointerException expected) { }
+
+            try {
+                reader.release(null);
+                assertTrue(false);
+            } catch (NullPointerException expected) { }
+
+        }
+
+        // test closed ModuleReader
+        try {
+            reader.open(BASE_RESOURCES[0]);
+            assertTrue(false);
+        } catch (IOException expected) { }
+
+
+        try {
+            reader.read(BASE_RESOURCES[0]);
+            assertTrue(false);
+        } catch (IOException expected) { }
+    }
+
+
+    /**
+     * Test ModuleReader to exploded module
      */
     public void testExplodedModule() throws Exception {
         test(MODS_DIR);
@@ -97,7 +173,7 @@ public class ModuleReaderTest {
 
 
     /**
-     * Test modular JAR
+     * Test ModuleReader to modular JAR
      */
     public void testModularJar() throws Exception {
         Path dir = Files.createTempDirectory(USER_DIR, "mlib");
@@ -111,7 +187,7 @@ public class ModuleReaderTest {
 
 
     /**
-     * Test JMOD
+     * Test ModuleReader to JMOD
      */
     public void testJMod() throws Exception {
         Path dir = Files.createTempDirectory(USER_DIR, "mlib");
@@ -145,7 +221,7 @@ public class ModuleReaderTest {
         try (reader) {
 
             // test each of the known resources in the module
-            for (String name : RESOURCES) {
+            for (String name : TEST_RESOURCES) {
                 byte[] expectedBytes
                     = Files.readAllBytes(MODS_DIR
                         .resolve(TEST_MODULE)
@@ -157,6 +233,7 @@ public class ModuleReaderTest {
             }
 
             // test "not found"
+            assertFalse(reader.find(NOT_A_RESOURCE).isPresent());
             assertFalse(reader.open(NOT_A_RESOURCE).isPresent());
             assertFalse(reader.read(NOT_A_RESOURCE).isPresent());
 
@@ -176,19 +253,22 @@ public class ModuleReaderTest {
                 assertTrue(false);
             } catch (NullPointerException expected) { }
 
-            // should release(null) throw NPE?
+            try {
+                reader.release(null);
+                throw new RuntimeException();
+            } catch (NullPointerException expected) { }
 
         }
 
         // test closed ModuleReader
         try {
-            reader.open(RESOURCES[0]);
+            reader.open(TEST_RESOURCES[0]);
             assertTrue(false);
         } catch (IOException expected) { }
 
 
         try {
-            reader.read(RESOURCES[0]);
+            reader.read(TEST_RESOURCES[0]);
             assertTrue(false);
         } catch (IOException expected) { }
     }