RFR: 8307105: JFileChooser InvalidPathException when selecting some system folders on Windows [v3]
Tejesh R
tr at openjdk.org
Thu May 25 07:17:58 UTC 2023
On Wed, 24 May 2023 15:48:34 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 selected file being to `null`; symbolic links are support...
>
> Handling `.lnk` files on Windows doesn't seem right: `JFileChooser` allows selecting such a file.
>
> * If it's a shortcut to a directory, clicking Open in the file chooser or double-clicking it in the list navigates to the target.
> * If it's a shortcut to a regular file, `JFileChooser` returns this file which isn't helpful, it's not what the user usually wants, the target of the shortcut should be returned. This is what happens if the user selects a shortcut file (`.lnk`) in the native file chooser.
>
> In addition to that, the shortcuts are filtered based on the file types. For example, if select the *Open* command in Notepad, the file chooser shows only shortcuts to `.txt` files (as well as `.txt` files), it hides shortcuts to other file types.
>
> But `JFileChooser` doesn't show `.lnk` files at all if an extension filter is set.
> 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 selected file being to `null`; symbolic links are supported with your fix.
>
> So, **I propose modifying the implementation of `FileSystemView.isFileSystem`** so that it returns only `sf.isFileSystem()` or filters out the `.lnk` shortcuts only.
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()`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13998#discussion_r1205092918
More information about the client-libs-dev
mailing list