On Tue, 15 Apr 2025 12:00:10 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. 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.

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

> 53:             System.getProperty("sun.net.www.protocol.file.ftp-enabled", 
> "false")
> 54:     );
> 55: 

The code for defining and checking this system property would ideally belong in 
`Handler.java`.

I decided to put it in `FileURLConnection.java` instead, just to avoid 
increasing the already significant duplication of code between unix/windows 
implementations of `Handler.java`.

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

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

Reply via email to