> 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("foobar");
> // ... some other header...
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 13 additional commits since the
last revision:
- Daniel's review - comment to test code
- merge latest from master branch
- Daniel's review, use a class level field for URLConnection
- Volkan's review - add a comment in test
- merge latest from master branch
- Daniel's suggestion - add reachability fence
- Volkan's review
- merge latest from master branch
- Daniel's suggestion - don't repeat/copy code from super()
- merge latest from master branch
- ... and 3 more: https://git.openjdk.org/jdk/compare/43f08329...05514212
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/27633/files
- new: https://git.openjdk.org/jdk/pull/27633/files/f2938a1b..05514212
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=27633&range=07
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=27633&range=06-07
Stats: 18725 lines in 456 files changed: 10757 ins; 6594 del; 1374 mod
Patch: https://git.openjdk.org/jdk/pull/27633.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/27633/head:pull/27633
PR: https://git.openjdk.org/jdk/pull/27633
