On Mon, 31 Mar 2025 14:46:32 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:

> Please help review this cleanup PR which consolidates 'local file' URL checks 
> across the 'file:' and 'jar:' URL scheme implementations and defines this 
> check in terms of RFC 8089, Section 2.
> 
> This PR:
> 
> * Moves `URLJarFile.isFileURL` to `sun.net.www.ParseUtil` where it is 
> documented according to RFC 8089 and given the more suitable name 
> `isLocalFileURL`
> * Updates `isLocalFileURL` to simplify an `if (x) return true;` statement to 
> `return x;`
> * Updates `URLJarFile.getJarFile` and `JarFileFactory.getOrCreate` to use 
> `isLocalFileURL`
> * Updates `JarFileFactory.urlFor` to use `isLocalFileURL` (while maintaining 
> and documenting the historical and subtly different non-treatment of '~' as 
> an alias for 'localhost')
> * Updates `sun.net.www.protocol.file.Handler.openConnection` to use 
> `isLocalFileURL` in windows/unix implementations
> 
> This is a pure cleanup / refactoring PR, no tests are updated here. Existing 
> testing in this area seems sparse, so I have tried to limit the number of 
> code tweaks to a minimium to make reviews easier.
> 
> Testing: GHA runs green, as well as tier2 on MacOS.

Hello Eirik, these changes look good to me and merely centralizing the logic to 
one single `ParseUtil.isLocalFileURL()` method. Thank you for cleaning this up.

The copyright year on `URLJarFile.java` will need an update.

-------------

Marked as reviewed by jpai (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24332#pullrequestreview-2746779221

Reply via email to