On Mon, 25 Nov 2024 01:29:14 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Can I please get a review of this change that proposes to address the issue >> noted in https://bugs.openjdk.org/browse/JDK-8344908? >> >> With this change, the URLClassPath will no longer propagate the >> `IllegalArgumentException` throwing when constructing a resource loader. The >> URL which caused the issue will continue to not be able to serve resources >> through that URLClassPath but the URLClassPath itself will now be >> functionally usable for locating resources through rest of the URLs. >> >> While at it, the internal class `FileURLMapper` which gets used in this code >> flow has been updated to be more confined by making it package private. >> >> For addressing the underlying issue with ParseUtil we have >> https://bugs.openjdk.org/browse/JDK-8258246 which I plan to look into >> separately. There are also a few other usages of `ParseUtil.decode()` in >> various other places outside of this code flow which I plan to look into >> separately. >> >> A new jtreg test has been introduced which reproduces the issue and verifies >> the fix. This test and other existing tests in tier1, tier2 and tier3 >> continue to pass with this change. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > minor code comment update in the test src/java.base/share/classes/sun/net/www/ParseUtil.java line 176: > 174: * @throws IllegalArgumentException if {@code s} could not be decoded > 175: */ > 176: public static String decode(String s) throws > IllegalArgumentException { I'm not sure if you meant to add this or not but "throws IAE" is not needed, usually don't put unchecked exceptions in the throws. src/java.base/unix/classes/jdk/internal/loader/FileURLMapper.java line 82: > 80: return false; > 81: } else { > 82: File f = new File(s); Lots of unrelated cleanup in this file, but okay. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22351#discussion_r1856030450 PR Review Comment: https://git.openjdk.org/jdk/pull/22351#discussion_r1856031412
