RFR: JDK-6956385: URLConnection.getLastModified() leaks file handles for jar:file and file: URLs [v3]

[Jesse Glick]

On Mon, 22 May 2023 16:31:11 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> It does not address the question if it needs to close more than the file: protocol.
>> 
>> (And it still feels like a layering violation, should maybe the metadata function initializeHeaders close its own stream - balanced with connect())?
>
> If we wanted to make JarURLConnection call AutoCloseable::close (or Closeable::close) when the underlying URLConnection is `AutoCloseable` - that would be a change of behaviour that would be observable by custom URLConnection implementations provided by custom URL Stream Handler.
> 
> That would be a change of behavior requiring a CSR, and it might not be appropriate for backport, due to potential backward compatibility issue (well - that would be a matter to discuss in the CSR anyway).
> 
> That's why I would advise to not try to do this in this PR. If we wanted to do it - we should probably log an RFE for that. Also examine what it means for `URL.openConnection()` to return an object that implements `AutoCloseable`. A the very least, an `@apiNote` would be needed in URL API documentation. 
> 
> Unless this really solves some real hard issue (as opposed to theoretical issues) for which we have no solution today, I would be very prudent with making wider changes to this old, intricate, and hard to maintain piece of code.

> should maybe the metadata function initializeHeaders close its own stream

Another possibility is for `FileURLConnection.getHeaderField(String)` and related methods to never open a `FileInputStream` to begin with, amending https://github.com/openjdk/jdk/blob/8474e693b4404ba62927fe0e43e68b904d66fbde/src/java.base/share/classes/sun/net/www/protocol/file/FileURLConnection.java#L96-L100 That would require overriding at least https://github.com/openjdk/jdk/blob/8474e693b4404ba62927fe0e43e68b904d66fbde/src/java.base/share/classes/sun/net/www/URLConnection.java#L104-L131 since https://github.com/openjdk/jdk/blob/8474e693b4404ba62927fe0e43e68b904d66fbde/src/java.base/share/classes/sun/net/www/protocol/file/FileURLConnection.java#L137-L140 etc. call `super`. The legality of such a change in light of https://github.com/openjdk/jdk/blob/8474e693b4404ba62927fe0e43e68b904d66fbde/src/java.base/share/classes/java/net/URLConnection.java#L57-L66 https://github.com/openjdk/jdk/blob/8474e693b4404ba62927fe0e43e68b904d66fbde/src/java.base/share/classes/java/net/URLConnection.ja
 va#L92-L96 is unclear; the Javadoc implies but does not state that you _cannot_ access header fields before `connect()` is called. On the other hand https://github.com/openjdk/jdk/blob/8474e693b4404ba62927fe0e43e68b904d66fbde/src/java.base/share/classes/sun/net/www/protocol/jar/JarURLConnection.java#L232-L235 apparently presumes that any request for a header field implicitly `connect()`s if it needed to.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/12871#discussion_r1200807234