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

[Jaikiran Pai]

On Sat, 4 Oct 2025 16:32:16 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:
> 
>   missed pushing the change

src/java.base/share/classes/sun/net/www/protocol/file/FileURLConnection.java line 145:

> 143:     public String getHeaderField(String name) {
> 144:         initializeHeaders();
> 145:         return super.getHeaderField(name);

Several header related method implementations in this class have been updated to avoid the `super.XXXX()` calls. The `super.XXX()` implementation was calling `getInputStream()` and ignoring/not using the returned `InputStream`. That call to `getInputStream()` was a way to initiate a readability check and if the File wasn't readable then it would return a different result compared to when it was readable. The use of `getInputStream()` has been replaced by a call to `isReadable()` which does the same checks without leaving around a `InputStream` instance. Apart from that change, the rest of the code that resided in the `super.XXX()` implementation has been literally copy/pasted in these methods to avoid any kind of behavioural change.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27633#discussion_r2404079257