> 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 16 additional commits since the
last revision:
- merge latest from master branch
- intialize headers - use MessageHeader.set()
- merge latest from master branch
- 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
- ... and 6 more: https://git.openjdk.org/jdk/compare/a3560020...9b8f74f3
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/27633/files
- new: https://git.openjdk.org/jdk/pull/27633/files/05514212..9b8f74f3
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=27633&range=08
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=27633&range=07-08
Stats: 22368 lines in 551 files changed: 13248 ins; 5316 del; 3804 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
