On Thu, 4 Mar 2021 22:16:10 GMT, Craig Andrews <github.com+194713+candr...@openjdk.org> wrote:
>> `java.net.URLClassLoader.getResource` can throw an undocumented >> `IllegalArgumentException`. >> >> According to the javadoc for the `getResource` and `findResource` methods, >> neither should be throwing `IllegalArgumentException` - they should return >> null if the resource can't be resolved. >> >> Quoting the javadoc for >> [`URLClassLoader.html#findResource`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/net/URLClassLoader.html#findResource(java.lang.String)) >>> Returns: >>> a URL for the resource, or null if the resource could not be found, or >>> if the loader is closed. >> >> And quoting the javadoc for >> [`ClassLoader.html#getResource`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/ClassLoader.html#getResource(java.lang.String)) >>> Returns: >>> URL object for reading the resource; null if the resource could not be >>> found, a URL could not be constructed to locate the resource, the resource >>> is in a package that is not opened unconditionally, or access to the >>> resource is denied by the security manager. >> >> Neither mentions throwing `IllegalArgumentException` and both are clear that >> when URL can't be constructed, `null` should be returned. >> >> Here's a stack trace: >> java.lang.IllegalArgumentException: name >> at >> java.base/jdk.internal.loader.URLClassPath$Loader.findResource(URLClassPath.java:600) >> at >> java.base/jdk.internal.loader.URLClassPath.findResource(URLClassPath.java:291) >> at java.base/java.net.URLClassLoader$2.run(URLClassLoader.java:655) >> at java.base/java.net.URLClassLoader$2.run(URLClassLoader.java:653) >> at java.base/java.security.AccessController.doPrivileged(Native >> Method) >> at >> java.base/java.net.URLClassLoader.findResource(URLClassLoader.java:652) >> >> Looking at >> [`URLClassPath.findResource`](https://github.com/openjdk/jdk/blob/2b00367e1154feb2c05b84a11d62fb5750e46acf/src/java.base/share/classes/jdk/internal/loader/URLClassPath.java#L603) >> URL findResource(final String name, boolean check) { >> URL url; >> try { >> url = new URL(base, ParseUtil.encodePath(name, false)); >> } catch (MalformedURLException e) { >> throw new IllegalArgumentException("name"); >> } >> >> Instead of throwing `IllegalArgumentException`, that line should simply >> return `null`. >> >> A similar issue exists at >> [`URLClassPath.getResource`](https://github.com/openjdk/jdk/blob/2b00367e1154feb2c05b84a11d62fb5750e46acf/src/java.base/share/classes/jdk/internal/loader/URLClassPath.java#L639) >> URL findResource(final String name, boolean check) { >> URL url; >> try { >> url = new URL(base, ParseUtil.encodePath(name, false)); >> } catch (MalformedURLException e) { >> throw new IllegalArgumentException("name"); >> } >> >> Instead of throwing `IllegalArgumentException`, that line should simply >> return `null`. > > Craig Andrews has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. The pull request contains one new > commit since the last revision: > > JDK-8262277: java.net.URLClassLoader.getResource throws undocumented > IllegalArgumentException test/jdk/java/net/URLClassLoader/TestBug8262277.java line 30: > 28: * @summary Test to see if URLClassLoader.getResource and > URLClassLoader.getResources > 29: * throw IllegalArgumentException > 30: */ I'd prefer if the test checked that findResource returned null and that findResources returned an empty enumeration. I think we should be able to find a better name for the test too. Do you really want the author tag in the test? I know they exist in some tests but they are impossible to remove, even when tests/code are significantly changed by someone else. ------------- PR: https://git.openjdk.java.net/jdk/pull/2662
