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 test/jdk/jdk/internal/loader/URLClassPath/ClassPathUnusableURLs.java line 90: > 88: jaros.putNextEntry(jarEntry); > 89: jaros.write("hello".getBytes(US_ASCII)); > 90: jaros.closeEntry(); >From a readability point of view, I think renaming "jaros" to "jos" would make >it a bit easier to read. Also personally I would drop the uses of final from >the try statement, only because there it adds noise when looking at the 3 >resources in the statement. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22351#discussion_r1856039341
