OpenJDK JDK-7067885 code changes for community review
Alexey Ivanov
alexey.ivanov at oracle.com
Fri Sep 16 10:20:30 UTC 2016
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 jdk9-dev
mailing list