Brent - the null check in Class::getResourceXXX looks really odd,
particularly because resolveName accepts null. Since we've changed this
in the jake forest to do the right thing then I'm wondering if it would
be better if you dropped this part from your patch, would that be okay?
-Alan
On 16/11/2016 23:22, Brent Christian wrote:
Hi,
Please review my fix for 8136831 - Undefined null behavior in
Class[Loader].getResourceXXXX().
Bug:
https://bugs.openjdk.java.net/browse/JDK-8136831
Webrev:
http://cr.openjdk.java.net/~bchristi/8136831/webrev.00/
Class and ClassLoader have the following public methods for locating
resources by name:
Class.getResource(String name)
Class.getResourceAsStream(String name)
ClassLoader.getResource(String name)
ClassLoader.getResourceAsStream(String name)
ClassLoader.getResources(String name)
Of these, only Class.getResourceAsStream() specifies the behavior when
passed a null 'name' argument - throw a NullPointerException.
All methods throw an NPE in this case (going back at least to JDK 7u),
with ClassLoader.getResources() being something of an exception. As
described in the bug report, it returns an Enumeration object with a
buggy implementation which throws an NPE from hasMoreElements().
As of the module system going into JDK9b111, these methods no longer
throw an NPE, but return null (again, with the exception of
ClassLoader.getResources(), which now returns a non-buggy Enumeration
with working hasMoreElements() method).
I believe this issue should be resolved as follows:
1. Restore the historical NPE behavior by adding code to ensure this
behavior (instead of relying on it happening incidentally deeper in
the code - see the stack traces in the bug report).
2. Specify @throws NPE in the JavaDoc for these methods.
For ClassLoader.getResources() this is a change in behavior, though
the old behavior of returning a buggy Enumeration is not worth
keeping. Better to fail fast, and behave like other related methods.
Thanks,
-Brent