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

[Alexey Ivanov]

On Thu, 25 May 2023 11:39:33 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>>> I understand that, virtual folders don't have a filesystem path, this is why `fsv.isFileSystem(f)` should handle this situation. The implementation of `FileSystemView.isFileSystem` explicitly excludes such files which are both `isLink` and `isDirectory`:
>>> 
>>> https://github.com/openjdk/jdk/blob/d7245f70e7bac1236bbcdcd9b25346ca22ab8bb2/src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java#L382-L388
>>> 
>>> Let's look deeper:
>>> 
>>> https://github.com/openjdk/jdk/blob/d7245f70e7bac1236bbcdcd9b25346ca22ab8bb2/src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java#L591-L597
>>> 
>>> https://github.com/openjdk/jdk/blob/d7245f70e7bac1236bbcdcd9b25346ca22ab8bb2/src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java#L860-L866
>>> 
>>> The `hasAttribute` method uses `getAttributes0` which calls [`IShellFolder::GetAttributesOf`](https://learn.microsoft.com/en-us/windows/win32/api/shobjidl_core/nf-shobjidl_core-ishellfolder-getattributesof) which returns [`SFGAO`](https://learn.microsoft.com/en-us/windows/win32/shell/sfgao). The attribute `SFGAO_LINK` which corresponds to `ATTRIB_LINK` in Java is described as _“The specified items are shortcuts.”_ That is it is a shell folder shortcut with `.lnk` file extension, which used to be the case.
>>> 
>>> However, now this attribute is set for both [_symbolic links_](https://learn.microsoft.com/en-us/windows/win32/fileio/creating-symbolic-links) and [_junctions_](https://learn.microsoft.com/en-us/windows/win32/fileio/hard-links-and-junctions#junctions).
>>> 
>>> Here's what different methods return:
>>> 
>>> ```
>>> file: C:\test\symbolic-links\jdk - junction
>>> 	isFileSystem   false
>>> 	isLink         true
>>> 	isDirectory    true
>>> 	isSymbolicLink false
>>> Absolute Path : null
>>> 
>>> file: C:\test\symbolic-links\jdk - symbolic
>>> 	isFileSystem   false
>>> 	isLink         true
>>> 	isDirectory    true
>>> 	isSymbolicLink true
>>> Absolute Path : C:\test\symbolic-links\jdk - symbolic
>>> 
>>> file: C:\test\symbolic-links\jdk - shortcut.lnk
>>> 	isFileSystem   false
>>> 	isLink         true
>>> 	isDirectory    true
>>> 	isSymbolicLink false
>>> Absolute Path : null
>>> ```
>>> 
>>> In the three cases above, `isFileSystem` returns `false` even though all the three objects point to a filesystem directory, the first two are fully transparent for apps whereas the `.lnk` shortcut needs special handling.
>>> 
>>> If `JFileChooser` is in directory-only mode, selecting a junction or `.lnk` file results in ...
>
>> Since `FileSystemView.isFileSystem` is used in many places for particular Look and Feel, there might be chances of regression. So how about checking for shell folder first and if yes then can combine the `shell.isFileSystem()` with `isSymbolicLink()` in BasicFileChooser class.......? If its not a shell folder then directly we can check for `isSymbolicLink()`.
> 
> I guess what I proposed is riskier but _it's better_. It resolves the problem why junctions and symbolic links aren't accepted. It will also handle junctions correctly, now, as I showed above, selecting a junction results in the selected file being set to `null` just like it was for symbolic links before your first fix.
> 
> Another thing to try is to modify `Win32ShellFolder2.isLink` so that it returns `true` only for Windows shortcuts that is for `.lnk` files. In this case, the behaviour will be the same as it was on previous versions of Windows where neither junctions nor symbolic links were considered a link.

As for other Look-and-Feels, `isFileSystem` is used for the same purpose that it's used in the code you're modifying: if a selected object in the list is a file system object (a directory), the text field to the path of the selected object; otherwise, set the text field to `null`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13998#discussion_r1205541978