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

[Volkan Yazici]

On Tue, 7 Oct 2025 08:07:39 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 five additional commits since the last revision:
> 
>  - Daniel's suggestion - don't repeat/copy code from super()
>  - merge latest from master branch
>  - missed pushing the change
>  - 8367561: fix inputstream leak
>  - 8367561: introduce tests

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

> 123:             } else {
> 124:                 try (var _ = new FileInputStream(file.getPath())) {
> 125:                 }

*Nit:* You can consider shortening this to `new FileInputStream(file.getPath()).close()`.

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

> 256:             is = new BufferedInputStream(new FileInputStream(file.getPath()));
> 257:         }
> 258:         return is;

90% of this work – i.e., getting the file listing – is already done in `connect()`. `getInputStream()` only concatenates `directoryListing`. Moving this last 10% logic to `connect()`, making it populate a

    private Supplier<InputStream> inputStreamSupplier;

field, can simplify the code. Have you considered this route?

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

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