From bfb7fba79b2a9a67505c25ab57007bfdb8e915a8 Mon Sep 17 00:00:00 2001 From: Alan Bateman Date: Wed, 25 May 2016 20:12:32 +0100 Subject: [PATCH] 8156142: ModuleReader instances don't always throw NPE for passed null args Reviewed-by: chegar, mchung --- .../java/lang/module/ModuleReader.java | 8 +- .../java/lang/module/SystemModuleFinder.java | 3 +- .../module/ModuleReader/ModuleReaderTest.java | 100 ++++++++++++++++-- 3 files changed, 98 insertions(+), 13 deletions(-) diff --git a/jdk/src/java.base/share/classes/java/lang/module/ModuleReader.java b/jdk/src/java.base/share/classes/java/lang/module/ModuleReader.java index 1b77bfd8618..4d3d2bf15a0 100644 --- a/jdk/src/java.base/share/classes/java/lang/module/ModuleReader.java +++ b/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 diff --git a/jdk/src/java.base/share/classes/java/lang/module/SystemModuleFinder.java b/jdk/src/java.base/share/classes/java/lang/module/SystemModuleFinder.java index 77d5392821a..9dd6694267c 100644 --- a/jdk/src/java.base/share/classes/java/lang/module/SystemModuleFinder.java +++ b/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); } diff --git a/jdk/test/java/lang/module/ModuleReader/ModuleReaderTest.java b/jdk/test/java/lang/module/ModuleReader/ModuleReaderTest.java index ecaf6f45dff..cdeb61b899a 100644 --- a/jdk/test/java/lang/module/ModuleReader/ModuleReaderTest.java +++ b/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) { } }