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

[Jesse Glick]

On Mon, 22 May 2023 17:40:01 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>>> 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
 .java#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.
>
> That seems like it could cause other issues down the road though. `FileURLConnection::connect` has this comment:
> 
> 
>     /*
>      * Note: the semantics of FileURLConnection object is that the
>      * results of the various URLConnection calls, such as
>      * getContentType, getInputStream or getContentLength reflect
>      * whatever was true when connect was called.
>      */
>     public void connect() throws IOException {

This patch appears to be work and may be preferable to what is currently proposed:


diff --git src/java.base/share/classes/sun/net/www/protocol/file/FileURLConnection.java src/java.base/share/classes/sun/net/www/protocol/file/FileURLConnection.java
index a9ce7cee9a1..55783474c22 100644
--- src/java.base/share/classes/sun/net/www/protocol/file/FileURLConnection.java
+++ src/java.base/share/classes/sun/net/www/protocol/file/FileURLConnection.java
@@ -53,7 +53,6 @@ public class FileURLConnection extends URLConnection {
     File file;
     String filename;
     boolean isDirectory = false;
-    boolean exists = false;
     List<String> files;
 
     long length = -1;
@@ -93,16 +92,12 @@ public class FileURLConnection extends URLConnection {
     private boolean initializedHeaders = false;
 
     private void initializeHeaders() {
-        try {
-            connect();
-            exists = file.exists();
-        } catch (IOException e) {
-        }
-        if (!initializedHeaders || !exists) {
+        if (!initializedHeaders || !file.exists()) {
             length = file.length();
             lastModified = file.lastModified();
+            filename = file.toString();
 
-            if (!isDirectory) {
+            if (!file.isDirectory()) {
                 FileNameMap map = java.net.URLConnection.getFileNameMap();
                 contentType = map.getContentTypeFor(filename);
                 if (contentType != null) {
@@ -131,17 +126,17 @@ public class FileURLConnection extends URLConnection {
 
     public Map<String,List<String>> getHeaderFields() {
         initializeHeaders();
-        return super.getHeaderFields();
+        return properties.getHeaders();
     }
 
     public String getHeaderField(String name) {
         initializeHeaders();
-        return super.getHeaderField(name);
+        return properties.findValue(name);
     }
 
     public String getHeaderField(int n) {
         initializeHeaders();
-        return super.getHeaderField(n);
+        return properties.getKey(n);
     }
 
     public int getContentLength() {

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

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