OpenJDK JDK-7067885 code changes for community review
Alan Bateman
Alan.Bateman at oracle.com
Mon Sep 12 16:08:47 UTC 2016
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