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

[Jaikiran Pai]

On Tue, 28 Oct 2025 13:23:29 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 16 additional commits since the last revision:
> 
>  - merge latest from master branch
>  - intialize headers - use MessageHeader.set()
>  - merge latest from master branch
>  - Daniel's review - comment to test code
>  - merge latest from master branch
>  - Daniel's review, use a class level field for URLConnection
>  - Volkan's review - add a comment in test
>  - merge latest from master branch
>  - Daniel's suggestion - add reachability fence
>  - Volkan's review
>  - ... and 6 more: https://git.openjdk.org/jdk/compare/da28266b...9b8f74f3

Thank you all for the reviews.

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

PR Comment: https://git.openjdk.org/jdk/pull/27633#issuecomment-3460996312