<Swing Dev> OpenJDK JDK-7067885 code changes for community review

Alexandr Scherbatiy alexandr.scherbatiy at oracle.com
Tue Sep 20 14:48:54 UTC 2016


   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.
>>>
>>
>




More information about the swing-dev mailing list