RFR: 8353440: Disable FTP fallback for non-local file URLs by default [v5]
Daniel Fuchs
dfuchs at openjdk.org
Wed Apr 23 16:09:43 UTC 2025
On Wed, 23 Apr 2025 12:29:03 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. I plan to update the CSR pending an initial round of review of this PR.
>>
>> 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 `sun.net.www.protocol.file.ftp-enabled` which when set to `true` re-enables the feature.
>> * 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.
>>
>> 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 incrementally with one additional commit since the last revision:
>
> Move file URL properties out of the Proxy section, replace URI with URL
This looks quite good. I will run it through our CI to see if anything falls from the tree.
src/java.base/share/classes/java/net/doc-files/net-properties.html line 174:
> 172: <a id="FileHandler"></a>
> 173: <H2>File URL stream protocol handler properties</H2>
> 174: <P>The following properties are used to configure the handler for URLs with the {@code file://} scheme:</P>
I wonder if it should say the `{@code file:}` scheme instead. Although I understand that the case that interests us here is "file://some.host/some/path". Don't change anything right now - let's see if we have other opinions.
-------------
PR Review: https://git.openjdk.org/jdk/pull/24657#pullrequestreview-2787864603
PR Review Comment: https://git.openjdk.org/jdk/pull/24657#discussion_r2056384398
More information about the net-dev
mailing list