RFR: 8367561: Getting some "header" property from a file:// URL causes a file descriptor leak [v5]

[Daniel Fuchs]

On Wed, 8 Oct 2025 10:26:24 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Can I please get a review of this change which proposes to fix a file descriptor leak in `sun.net.www.protocol.file.FileURLConnection`?
>> 
>> The bug affects all JDK versions since Java 8 through mainline JDK. Furthermore, the issue isn't in any way related to "jar:" URLs or the JAR resources caching involved in the `JarURLConnection`.
>> 
>> `sun.net.www.protocol.file.FileURLConnection` is a `java.net.URLConnection`. For this issue, the APIs on `URLConnection` that are of interest are `connect()`, header related APIs (the getHeaderXXXX(), getLastModified() and similar APIs) and `getInputStream()`.
>> 
>> The existing implementation in `FileURLConnection` in its `connect()` implementation does readability checks on the underlying `File` instance. If the `File` is a directory then the readability check is done by verifying that a `File.list()` call does not return null. On the other hand, if the File is not a directory, then connect() constructs a temporary `java.io.FileInputStream` for the File and lets the FileInputStream's constructor implementation do the necessary readability checks. In either case, if the readability checks fail, then an IOException is thrown from this method and the FileURLConnection stays unconnected.
>> 
>> One important detail of the implementation in `FileURLConnection.connect()` is that the `FileInputStream` that it creates for the regular-file readability check, it keeps it open when it returns from `connect()`. `connect()` itself doesn't return any value, so this `FileInputStream` that was created for readability check remains open without the application having access to it, unless the application calls `(File)URLConnection.getInputStream()`. The implementation of `FileURLConnection.getInputStream()` returns this previously constructed `InputStream` if `connect()` had previously suceeded with its readability checks. The application, as usual, is then expected to `close()` that `InputStream` that was returned from `URLConnection.getInputStream()`. This is the "normal" case, where the application at some point calls the `URLConnection.getInputStream()` and closes that stream.
>> 
>> As noted previously, `URLConnection` has a few other APIs, for example the APIs that provide header values. An example:
>> 
>> 
>> Path regularFile = Path.of("hello.txt");
>> URLConnection conn = regularFile.toUri().toURL().openConnection();
>> // either of the following header related APIs
>> long val = conn.getLastModified();
>> String val = conn.getHeaderField...
>
> Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Daniel's suggestion - add reachability fence

Marked as reviewed by dfuchs (Reviewer).

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

PR Review: https://git.openjdk.org/jdk/pull/27633#pullrequestreview-3314936881