RFR: 8353440: Disable FTP fallback for non-local file URLs by default [v10]

[Daniel Fuchs]

On Fri, 25 Apr 2025 11:24:11 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> wrote:

>> Please help review this PR which disables the unspecified but long-standing feature where an `FtpURLConnection` is opened as a fallback for non-local file URLs.
>> 
>> Before this change, if a file URL has a non-local host component, say `file://remotehost/folder/data.txt`, then the  implementation would attempt opening an FTP connection to `remotehost`. After this change, such URLs will be rejected with a `MalformedURLException`, unless the FTP fallback feature is explicitly re-enabled via a system property.
>> 
>> This change was initially discussed here: https://mail.openjdk.org/pipermail/net-dev/2025-March/025988.html
>> 
>> See the above discussion and CSR draft JDK-8354678 for the motivation for this change.
>> 
>> This PR:
>> 
>> * Changes file URL `Handler::openConnection` implementation for unix/windows to throw `MalformedURLException`, unless the FTP fallback feature is explicitly enabled by configuration.
>> * Introduces a new system property `jdk.net.file.ftpenabled` which when set to `true` re-enables the feature.
>> * Documents the new property in `net-properties.html` 
>> * Updates the existing test `NonLocalFtpFallback` to enable the feature via said system property.
>> * Adds a new test `NonLocalFtpFallbackDisabled` verifying that a `MalformedURLException` is thrown by default for a non-local URL host component.
>> * Moves testing of exceptional behavior in FtpConnection when using non-local file URLs with FTP fallback enabled from `OpenStream` to `NonLocalFtpFallback`
>> 
>> I have added a Release Note as a subtask in the JBS issue, this also needs a review.
>
> Eirik Bjørsnøs 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 12 additional commits since the last revision:
> 
>  - Merge branch 'master' into disable-non-local-file-urls
>  - Add @comment referencing testing of non-local file URLs in NonLocalFtpFallback
>  - verifyNonLocalFileURLRejected is a better name for this test which expects the FTP fallback to be disabled
>  - Move testing of exceptional behavior of a non-local, FTP-fallback enabled file URL from OpenStream to NonLocalFtpFallback
>  - Add a test verifying that local file URLs such as 'file://localhost/folder/data.xml' are not affected by the FTP fallback feature being enabled
>  - Merge branch 'master' into disable-non-local-file-urls
>  - Fix repeated "should should"
>  - Move file URL properties out of the Proxy section, replace URI with URL
>  - Be explicit about explicit {@code ftp://} URIs
>  - Document jdk.net.file.ftpfallback as a Networking Property
>  - ... and 2 more: https://git.openjdk.org/jdk/compare/03e34e30...af73727f

Sorry - rewording the release note got me thinking again...

src/java.base/share/classes/sun/net/www/protocol/file/FileURLConnection.java line 55:

> 53:             System.getProperty("jdk.net.file.ftpfallback", "false")
> 54:     );
> 55: 

Maybe we should use `Boolean.getBoolean("jdk.net.file.ftpfallback")` here.

test/jdk/sun/net/www/protocol/file/NonLocalFtpFallbackDisabled.java line 58:

> 56:         // We can use a fake host name here, no actual FTP request will be made
> 57:         String hostname = "remotehost";
> 58:         URL nonLocalURL = new URL("file", hostname, localURL.getFile());

Should we also check that `new URL("file", hostname, "").openConnection()` will throw MUE?

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

PR Review: https://git.openjdk.org/jdk/pull/24657#pullrequestreview-2794825750
PR Review Comment: https://git.openjdk.org/jdk/pull/24657#discussion_r2060628151
PR Review Comment: https://git.openjdk.org/jdk/pull/24657#discussion_r2060634329