RFR: 8307105: JFileChooser InvalidPathException when selecting some system folders on Windows [v3]

[Alexey Ivanov]

On Wed, 17 May 2023 16:47:56 GMT, Tejesh R <tr at openjdk.org> wrote:

>> This is a regression from fix [JDK-8281966](https://bugs.openjdk.org/browse/JDK-8281966): Absolute path of symlink is null in JFileChooser. The fix checks whether the file path is a symbolic link using `Files.isSymbolicLink()` method with path as input. In windows for specific folders like "This PC"/"Network"/"Libraries" the path value will be a hex values which causes InvalidPathException. In order to resolve the issue, since no other checks are available to validate the path of these folders, checking if the file is link firstly and then for symbolic link resolves the problem (since File.isLink() doesn't take path as input rather file is a parameter). Since every symbolic link is a link, this fix seems logical to me.
>> The fix is tested in CI for regression and is green. The regression fix is also tested for confirmation and works fine.
>
> Tejesh R has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Updated based on review comments

Changes requested by aivanov (Reviewer).

src/java.desktop/share/classes/javax/swing/plaf/basic/BasicFileChooserUI.java line 725:

> 723:                                     || (chooser.isDirectorySelectionEnabled()
> 724:                                         && (fsv.isFileSystem(f)
> 725:                                         || (fsv.isLink(f) && Files.isSymbolicLink(f.toPath())))

I don't think it's correct.

`Files.isSymbolicLink` should only be called for objects for which `fsv.isFileSystem(f)` returns `true`.

`fsv.isLink(f)` returns `true` for `.lnk` files which are the common Windows shortcuts; such a file can also be a symbolic link.

test/jdk/javax/swing/JFileChooser/FileChooserIPETest.java line 45:

> 43:  */
> 44: 
> 45: public class FileChooserIPETest {

Suggestion:

public class FileChooserInvalidPathExceptionTest {

Isn't it more descriptive this way? IPE isn't as common as NPE.

test/jdk/javax/swing/JFileChooser/FileChooserIPETest.java line 68:

> 66:                 Network.
> 67:                 2. Select and traverse through those folders.
> 68:                 3. If InvalidPathException occurs then test FAIL else test is PASS.

You may be more specific: if exception occurs, the test fails automatically.

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

PR Review: https://git.openjdk.org/jdk/pull/13998#pullrequestreview-1436912464
PR Review Comment: https://git.openjdk.org/jdk/pull/13998#discussion_r1200690007
PR Review Comment: https://git.openjdk.org/jdk/pull/13998#discussion_r1200711027
PR Review Comment: https://git.openjdk.org/jdk/pull/13998#discussion_r1200708730