On Mon, 22 May 2023 16:07:30 GMT, Tejesh R <[email protected]> wrote: >> 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. > > `Files.isSymbolicLink` takes path as argument to check if its a symbolic > link. For certain windows specific files as explained the path won't be exact > path, rather a hex values. So directly validating with path is causing an > exception. So `fsv.isLink()` is used as a first level validation which uses > File to check if its valid. I didn't find any other alternatives to validate > the file path, so used `fsv.isLink` as first level validation along with > Symbolic Link check.
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. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13998#discussion_r1204238269
