On Fri, 25 Apr 2025 18:41:19 GMT, Eirik Bjørsnøs <eir...@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 incrementally with two additional 
> commits since the last revision:
> 
>  - Add test for MUE on non-local URL with an empty path component
>  - Use Boolean::getBoolean

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

> 60:             // Non-empty path
> 61:             URL localURL = file.toUri().toURL();
> 62:             URL nonLocalURL = new URI("file", hostname, 
> localURL.getFile(), "").toURL();

Actually we should take that out of the assertThrows since 
MalformedURLException could be thrown while constructing the URL...

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24657#discussion_r2060730678

Reply via email to