Thanks Alexander for you review.
Mercurial diff for updated source change:
diff -r 021369229cfd
src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java
--- a/src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java Tue Sep
06 13:09:29 2016 -0400
+++ b/src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java Fri Sep
23 12:15:51 2016 +0530
@@ -32,6 +32,7 @@
import java.io.FileNotFoundException;
import java.util.*;
import java.util.concurrent.Callable;
+import java.nio.file.*;
/**
* @author Michael Martak
@@ -240,10 +241,11 @@
* @exception FileNotFoundException if file does not exist
*/
public static ShellFolder getShellFolder(File file) throws
FileNotFoundException {
+ Path path = Paths.get(file.getPath());
if (file instanceof ShellFolder) {
return (ShellFolder)file;
}
- if (!file.exists()) {
+ if (!Files.exists(path,LinkOption.NOFOLLOW_LINKS)) {
throw new FileNotFoundException();
}
return shellFolderManager.createShellFolder(file);
Below is the explanation for fix.
Problem description:
FileChooser behavior when there is a soft link to a non-existent file/directory.
Soft link to be displayed with an error/exception when attempt is made to
open it. Instead the soft link name is not displayed in the file chooser.
Analysis:
JFileChooser() internally uses java.io.exists() to check whether file exists.
In this scenario JFileChooser() checks dangling symlink existence using
java.io.exists().
java.io.exists() api does not handle dangling symlink and returns false.
JFileChooser() expects exists() api to return true for dangling symlink but,
java.io.exists() returns false.
Code changes:
We have made changes in JFilechooser to use Non-Blocking java.nio.exists() api
which check for file existence.
java.nio.exists() handles dangling symlinks it accepts second argument link
option which
determines whether to follow the link or not and returns true for dangling
symlinks.
Regards,
Alok
-----Original Message-----
From: Alexandr Scherbatiy [mailto:alexandr.scherba...@oracle.com]
Sent: Tuesday, September 20, 2016 8:19 PM
To: Sharma, Alok Kumar (OSTL) <alok.sha...@hpe.com>
Cc: swing-dev@openjdk.java.net
Subject: Re: <Swing Dev> OpenJDK JDK-7067885 code changes for community review
Hello Alok,
Is it possible to update the line where File.exists() is used just
converting the file to a Path and using java.nio.exists() check?
Thanks,
Alexandr.
On 9/16/2016 1:20 PM, Alexey Ivanov wrote:
> Hi Alok,
>
> This change should be discussed on swing-dev mailing list because you
> modify behavior of javax.swing.JFileChooser, and on core-libs-dev
> because you also modify java.io.File.
>
> I agree with Alan, using the new API appears to be a better
> alternative than changing java.io.File.
>
>
> Regards,
> Alexey
>
> On 12.09.2016 19:08, Alan Bateman wrote:
>> Best to follow-up on swing-dev as I assume the right thing is to
>> update JFileChooser rather than changing java.io.File to be mutable.
>> You did acknowledge near the end of your mail that the new file
>> system API supports sym links so it may be that JFileChooser could
>> use that.
>>
>> -Alan
>>
>>
>> On 12/09/2016 08:41, Sharma, Alok Kumar (OSTL) wrote:
>>> Following is the fix for JDK-7067885. I am not sure which mailing ID
>>> to post this.
>>>
>>> Bug: FileChooser does not display soft link name if link is to
>>> nonexistent file/directory
>>> https://bugs.openjdk.java.net/browse/JDK-7067885
>>>
>>> This bug is unassigned. Can someone please look into these changes
>>> and get back to me? Explanation of fix is at the end of the source
>>> code diff.
>>>
>>> Mercurial diff for source change:
>>> -------------------------------------------------------------------
>>> diff -r 021369229cfd src/java.base/share/classes/java/io/File.java
>>> --- a/src/java.base/share/classes/java/io/File.java Tue Sep 06
>>> 13:09:29 2016 -0400
>>> +++ b/src/java.base/share/classes/java/io/File.java Mon Sep 12
>>> 11:27:07 2016 +0530
>>> @@ -164,6 +164,27 @@
>>> private final String path;
>>>
>>> /**
>>> + * The flag indicates whether to follow the symlink.
>>> + */
>>> + private boolean follow_link = true;
>>> +
>>> + /**
>>> + * Sets the follow_link of file.
>>> + * description: Sets follow_link on or off.
>>> + * @param follow_link the value that determines whether to
>>> follow the symlink or not.
>>> + * TRUE: file.io.exists() checks the file existence using
>>> stat() which follows the symlink.
>>> + *
>>> + * FALSE: file.io.exists() checks the file existence using
>>> lstat() if stat() fails to retrieve
>>> + * the file info. lstat() if file is a symbolic link, then
>>> it returns information
>>> + * about the link itself.
>>> + * @return Returns ZERO at success
>>> + */
>>> + public int set_follow_link(boolean follow_link) {
>>> + this.follow_link=follow_link;
>>> + return 0;
>>> + }
>>> +
>>> + /**
>>> * Enum type that indicates the status of a file path.
>>> */
>>> private static enum PathStatus { INVALID, CHECKED }; diff -r
>>> 021369229cfd src/java.base/unix/native/libjava/UnixFileSystem_md.c
>>> --- a/src/java.base/unix/native/libjava/UnixFileSystem_md.c Tue Sep
>>> 06 13:09:29 2016 -0400
>>> +++ b/src/java.base/unix/native/libjava/UnixFileSystem_md.c Mon Sep
>>> 12 11:27:07 2016 +0530
>>> @@ -51,6 +51,7 @@
>>> #define dirent64 dirent
>>> #define readdir64_r readdir_r
>>> #define stat64 stat
>>> +#define lstat64 lstat
>>> #ifndef MACOSX
>>> #define statvfs64 statvfs
>>> #endif
>>> @@ -62,6 +63,7 @@
>>> jfieldID path;
>>> } ids;
>>>
>>> +jfieldID ufs_follow_link;
>>>
>>> JNIEXPORT void JNICALL
>>> Java_java_io_UnixFileSystem_initIDs(JNIEnv *env, jclass cls) @@
>>> -70,6 +72,8 @@
>>> if (!fileClass) return;
>>> ids.path = (*env)->GetFieldID(env, fileClass,
>>> "path", "Ljava/lang/String;");
>>> + ufs_follow_link = (*env)->GetFieldID(env, fileClass,
>>> + "follow_link", "Z");
>>> }
>>>
>>> /* -- Path operations -- */
>>> @@ -113,20 +117,42 @@
>>> return JNI_FALSE;
>>> }
>>>
>>> +static jboolean
>>> +lstatMode(const char *path, int *mode) {
>>> + struct stat64 sb;
>>> + if (lstat64(path, &sb) == 0) {
>>> + *mode = sb.st_mode;
>>> + return JNI_TRUE;
>>> + }
>>> + return JNI_FALSE;
>>> +}
>>>
>>> JNIEXPORT jint JNICALL
>>> Java_java_io_UnixFileSystem_getBooleanAttributes0(JNIEnv *env,
>>> jobject this,
>>> jobject file) {
>>> jint rv = 0;
>>> + jint follow_link = 0;
>>>
>>> WITH_FIELD_PLATFORM_STRING(env, file, ids.path, path) {
>>> int mode;
>>> - if (statMode(path, &mode)) {
>>> - int fmt = mode & S_IFMT;
>>> - rv = (jint) (java_io_FileSystem_BA_EXISTS
>>> - | ((fmt == S_IFREG) ?
>>> java_io_FileSystem_BA_REGULAR : 0)
>>> - | ((fmt == S_IFDIR) ?
>>> java_io_FileSystem_BA_DIRECTORY : 0));
>>> + follow_link = (*env)->GetBooleanField(env, file,
>>> ufs_follow_link);
>>> + if ( follow_link ) {
>>> + if (statMode(path, &mode)) {
>>> + int fmt = mode & S_IFMT;
>>> + rv = (jint) (java_io_FileSystem_BA_EXISTS
>>> + | ((fmt == S_IFREG) ?
>>> java_io_FileSystem_BA_REGULAR : 0)
>>> + | ((fmt == S_IFDIR) ?
>>> java_io_FileSystem_BA_DIRECTORY : 0));
>>> + }
>>> + }
>>> + else {
>>> + if (lstatMode(path, &mode)) {
>>> + int fmt = mode & S_IFMT;
>>> + rv = (jint) (java_io_FileSystem_BA_EXISTS
>>> + | ((fmt == S_IFREG) ?
>>> java_io_FileSystem_BA_REGULAR : 0)
>>> + | ((fmt == S_IFDIR) ?
>>> java_io_FileSystem_BA_DIRECTORY : 0));
>>> + }
>>> }
>>> } END_PLATFORM_STRING(env, path);
>>> return rv;
>>> diff -r 021369229cfd
>>> src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java
>>> ---
>>> a/src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java
>>>
>>> Tue Sep 06 13:09:29 2016 -0400
>>> +++
>>> b/src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java
>>>
>>> Mon Sep 12 11:27:07 2016 +0530
>>> @@ -521,6 +521,7 @@
>>> f = createFileSystemRoot(f);
>>> }
>>> try {
>>> + f.set_follow_link(false);
>>> f = ShellFolder.getShellFolder(f);
>>> } catch (FileNotFoundException e) {
>>> // Not a valid file (wouldn't show in native
>>> file chooser)
>>> ------------------------------------------
>>>
>>> Below is the explanation for fix.
>>>
>>> Problem description:
>>> FileChooser behavior when there is a soft link to a non-existent
>>> file/directory.
>>> Soft link to be displayed with an error/exception when attempt is
>>> made to
>>> open it. Instead the soft link name is not displayed in the file
>>> chooser.
>>>
>>> Analysis:
>>> JFileChooser() internally uses java.io.exists() to check whether
>>> file exists.
>>> In this scenario JFileChooser() checks dangling symlink existence
>>> using java.io.exists().
>>> java.io.exists() api does not handle dangling symlink and returns
>>> false.
>>>
>>> JFileChooser() expects exists() api to return true for dangling
>>> symlink but, java.io.exists() returns false.
>>>
>>> We see that there are two exists() apis in JAVA java.io.exists() and
>>> java.nio.exists().
>>>
>>> java.nio.exists() handles dangling symlinks it accepts second
>>> argument link option which
>>> determines whether to follow the link or not and returns true for
>>> dangling symlinks.
>>> java.io.exists() follows the symlink and returns false if target
>>> file doesn't exist.
>>>
>>> We have enhanced the java.io.exists() api to return true if it is a
>>> dangling symlink.
>>>
>>> Changes in code:
>>>
>>> New variable "follow_link" is introduced in
>>> "jdk/src/java.base/share/classes/java/io/File.java", we check for
>>> this flag in exists() code
>>> if its set then we handle symlinks. In case of dangling symlink
>>> java.io.exists() api checks file existence
>>> symlink itself not the target file and returns true.
>>>
>>> JFileChooser() issue (JDK-7067885) gets resolved with these changes.
>>>
>>> Testing:
>>> I have covered the testing for the below apis.
>>> Jfilechooser
>>> getShellFolder
>>> FileSystemView
>>> and ran /openjdk9/jdk/test/javax/swing/JFileChooser/* tests.
>>>
>>
>
