RFR: 8367561: Getting some "header" property from a file:// URL causes a file descriptor leak [v3]
Jaikiran Pai
jpai at openjdk.org
Tue Oct 7 08:11:12 UTC 2025
On Mon, 6 Oct 2025 14:30:07 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:
>> 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.
>
> The code duplication is a bit distateful. Have you thought about introducing a package protected method in s.n.www.URLConnection e.g. ensureConnectionEstablished() that would by default call getInputStream()?
> Then you could possibly just override this method in the s.n.www.FileURLConnection subclass, instead of having to override all the methods that call getInputStream() just for the sake or replacing getInputStream() with isReadable()?
>
> I haven't experimented with what I'm suggesting here, and maybe you already considered and rejected it.
Hello Daniel, when I started on this fix, I intended to keep the changes only to the `sun.net.www.protocol.file.FileURLConnection` to reduce the chances of unforeseen problems. So I hadn't given it a thought to update the `sun.net.www.URLConnection` class. But what you suggest here is much more cleaner and at the same time allows us to fix this issue in `FileURLConnection`. So I've updated this PR to follow your suggestion.
I decided to name the new (internal) method as `ensureCanServeHeaders()` to be more precise about what its role is. I can rename it to something else, if necessary.
I verified that the tests pass and the original leak is addressed even after these latest changes.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27633#discussion_r2409775501
More information about the net-dev
mailing list