From e48481323373f879f7f3a556e93c32b95ace2908 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Fri, 7 Jul 2023 16:19:16 +0200 Subject: [PATCH 01/33] Added check for deletion of hideaway --- .../common/mount/MountWithinParentUtil.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index 2aa9feadc..f203a359f 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -72,12 +72,24 @@ public final class MountWithinParentUtil { Path hideaway = getHideaway(mountPoint); try { waitForMountpointRestoration(mountPoint); + if (Files.notExists(hideaway, LinkOption.NOFOLLOW_LINKS)) { + LOG.error("Unable to restore hidden directory to mountpoint \"{}\": Directory does not exist. (Deleted by user?)", mountPoint); + return; + } + if (!Files.isDirectory(hideaway, LinkOption.NOFOLLOW_LINKS)) { + LOG.error("Unable to restore hidden directory to mountpoint \"{}\": Not a directory.", mountPoint); + if (SystemUtils.IS_OS_WINDOWS) { + Files.setAttribute(hideaway, WIN_HIDDEN_ATTR, false); + } + return; + } + Files.move(hideaway, mountPoint); if (SystemUtils.IS_OS_WINDOWS) { Files.setAttribute(mountPoint, WIN_HIDDEN_ATTR, false); } } catch (IOException e) { - LOG.error("Unable to restore hidden directory to mountpoint {}.", mountPoint, e); + LOG.error("Unable to restore hidden directory to mountpoint \"{}\".", mountPoint, e); } } From 92b77baf40c241023a6d05075fe2e78afbcf5650 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Mon, 10 Jul 2023 15:42:49 +0200 Subject: [PATCH 02/33] Improved handling of existing hideaways --- .../common/mount/MountWithinParentUtil.java | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index f203a359f..2ec5ad862 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -28,9 +28,7 @@ public final class MountWithinParentUtil { var hideExists = Files.exists(hideaway, LinkOption.NOFOLLOW_LINKS); //TODO: possible improvement by just deleting an _empty_ hideaway - if (mpExists && hideExists) { //both resources exist (whatever type) - throw new MountPointPreparationException(new FileAlreadyExistsException(hideaway.toString())); - } else if (!mpExists && !hideExists) { //neither mountpoint nor hideaway exist + if (!mpExists && !hideExists) { //neither mountpoint nor hideaway exist throw new MountPointPreparationException(new NoSuchFileException(mountPoint.toString())); } else if (!mpExists) { //only hideaway exists checkIsDirectory(hideaway); @@ -42,8 +40,13 @@ public final class MountWithinParentUtil { } catch (IOException e) { throw new MountPointPreparationException(e); } - } else { //only mountpoint exists + } else { //mountpoint exists... try { + if (hideExists) { //... with hideaway + removeResidualHideaway(hideaway); + } + + //... (now) without hideaway checkIsDirectory(mountPoint); checkIsEmpty(mountPoint); @@ -68,6 +71,16 @@ public final class MountWithinParentUtil { } } + private static void removeResidualHideaway(Path hideaway) throws IOException { + if (!Files.isDirectory(hideaway, LinkOption.NOFOLLOW_LINKS)) { + if (SystemUtils.IS_OS_WINDOWS) { + Files.setAttribute(hideaway, WIN_HIDDEN_ATTR, false); + } + throw new MountPointPreparationException(new NotDirectoryException(hideaway.toString())); + } + Files.delete(hideaway); + } + static void cleanup(Path mountPoint) { Path hideaway = getHideaway(mountPoint); try { From 328f2f89a82ce87b3e192f8105968983fd9bb270 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Mon, 10 Jul 2023 16:01:39 +0200 Subject: [PATCH 03/33] Removed unnecessary checks --- .../common/mount/MountWithinParentUtil.java | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index 2ec5ad862..3ce6d1c39 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -73,12 +73,9 @@ public final class MountWithinParentUtil { private static void removeResidualHideaway(Path hideaway) throws IOException { if (!Files.isDirectory(hideaway, LinkOption.NOFOLLOW_LINKS)) { - if (SystemUtils.IS_OS_WINDOWS) { - Files.setAttribute(hideaway, WIN_HIDDEN_ATTR, false); - } throw new MountPointPreparationException(new NotDirectoryException(hideaway.toString())); } - Files.delete(hideaway); + Files.delete(hideaway); //Fails if not empty } static void cleanup(Path mountPoint) { @@ -89,13 +86,6 @@ public final class MountWithinParentUtil { LOG.error("Unable to restore hidden directory to mountpoint \"{}\": Directory does not exist. (Deleted by user?)", mountPoint); return; } - if (!Files.isDirectory(hideaway, LinkOption.NOFOLLOW_LINKS)) { - LOG.error("Unable to restore hidden directory to mountpoint \"{}\": Not a directory.", mountPoint); - if (SystemUtils.IS_OS_WINDOWS) { - Files.setAttribute(hideaway, WIN_HIDDEN_ATTR, false); - } - return; - } Files.move(hideaway, mountPoint); if (SystemUtils.IS_OS_WINDOWS) { From 5e2e8c35d5f3c19f803196ea49e5dd1bb1d19ceb Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Mon, 10 Jul 2023 17:38:30 +0200 Subject: [PATCH 04/33] Added cleanup of junctions on Win --- .../common/mount/MountWithinParentUtil.java | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index 3ce6d1c39..04e84f64e 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -12,6 +12,7 @@ import java.nio.file.LinkOption; import java.nio.file.NoSuchFileException; import java.nio.file.NotDirectoryException; import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributes; public final class MountWithinParentUtil { @@ -24,7 +25,7 @@ public final class MountWithinParentUtil { static void prepareParentNoMountPoint(Path mountPoint) throws MountPointPreparationException { Path hideaway = getHideaway(mountPoint); - var mpExists = Files.exists(mountPoint, LinkOption.NOFOLLOW_LINKS); + var mpExists = removeResidualJunction(mountPoint); //Handle junction as not existing var hideExists = Files.exists(hideaway, LinkOption.NOFOLLOW_LINKS); //TODO: possible improvement by just deleting an _empty_ hideaway @@ -71,6 +72,25 @@ public final class MountWithinParentUtil { } } + private static boolean removeResidualJunction(Path path) throws MountPointPreparationException { + try { + if (!Files.exists(path, LinkOption.NOFOLLOW_LINKS)) { + return false; + } + if (!SystemUtils.IS_OS_WINDOWS) { //So far this is only a problem on Windows + return true; + } + if (Files.readAttributes(path, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS).isOther()) { + LOG.info("Mountpoint \"{}\" is still a junction. Deleting it.", path); + Files.delete(path); //Throws if path is also a non-empty folder + return false; + } + return true; + } catch (IOException e) { + throw new MountPointPreparationException(e); + } + } + private static void removeResidualHideaway(Path hideaway) throws IOException { if (!Files.isDirectory(hideaway, LinkOption.NOFOLLOW_LINKS)) { throw new MountPointPreparationException(new NotDirectoryException(hideaway.toString())); From 12f5f41968b897878a3169cd47b3f0d0b0fb3bfc Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Tue, 11 Jul 2023 20:28:40 +0200 Subject: [PATCH 05/33] Redefined IllegalMountPointException and MountPointPreparationException --- .../mount/IllegalMountPointException.java | 24 ++++++++++++++++--- .../mount/MountPointInUseException.java | 6 +++-- .../mount/MountPointNotExistsException.java | 6 +++-- .../MountPointNotSupportedException.java | 6 +++-- .../mount/MountPointPreparationException.java | 16 +++++++++---- .../common/mount/MountWithinParentUtil.java | 19 +++++++-------- .../org/cryptomator/common/mount/Mounter.java | 8 +++---- 7 files changed, 56 insertions(+), 29 deletions(-) diff --git a/src/main/java/org/cryptomator/common/mount/IllegalMountPointException.java b/src/main/java/org/cryptomator/common/mount/IllegalMountPointException.java index 5fdb1d91c..a96e4f0bb 100644 --- a/src/main/java/org/cryptomator/common/mount/IllegalMountPointException.java +++ b/src/main/java/org/cryptomator/common/mount/IllegalMountPointException.java @@ -1,9 +1,27 @@ package org.cryptomator.common.mount; +import java.nio.file.Path; + +/** + * Indicates that validation or preparation of a mountpoint failed due to a configuration error or an invalid system state.
+ * Instances of this exception are usually caught and displayed to the user in an appropriate fashion, e.g. by {@link org.cryptomator.ui.unlock.UnlockInvalidMountPointController UnlockInvalidMountPointController.} + * + * @see MountPointPreparationException MountPointPreparationException for wrapping exceptions which occur while preparing the mountpoint. + */ public class IllegalMountPointException extends IllegalArgumentException { - public IllegalMountPointException(String msg) { - super(msg); + private final Path mountpoint; + + public IllegalMountPointException(Path mountpoint) { + this(mountpoint, "The provided mountpoint has a problem: " + mountpoint.toString()); } -} + public IllegalMountPointException(Path mountpoint, String msg) { + super(msg); + this.mountpoint = mountpoint; + } + + public Path getMountpoint() { + return mountpoint; + } +} \ No newline at end of file diff --git a/src/main/java/org/cryptomator/common/mount/MountPointInUseException.java b/src/main/java/org/cryptomator/common/mount/MountPointInUseException.java index 9f7f11174..d104cff4d 100644 --- a/src/main/java/org/cryptomator/common/mount/MountPointInUseException.java +++ b/src/main/java/org/cryptomator/common/mount/MountPointInUseException.java @@ -1,8 +1,10 @@ package org.cryptomator.common.mount; +import java.nio.file.Path; + public class MountPointInUseException extends IllegalMountPointException { - public MountPointInUseException(String msg) { - super(msg); + public MountPointInUseException(Path path) { + super(path); } } diff --git a/src/main/java/org/cryptomator/common/mount/MountPointNotExistsException.java b/src/main/java/org/cryptomator/common/mount/MountPointNotExistsException.java index e90523bc2..677e2fb01 100644 --- a/src/main/java/org/cryptomator/common/mount/MountPointNotExistsException.java +++ b/src/main/java/org/cryptomator/common/mount/MountPointNotExistsException.java @@ -1,8 +1,10 @@ package org.cryptomator.common.mount; +import java.nio.file.Path; + public class MountPointNotExistsException extends IllegalMountPointException { - public MountPointNotExistsException(String msg) { - super(msg); + public MountPointNotExistsException(Path path, String msg) { + super(path, msg); } } diff --git a/src/main/java/org/cryptomator/common/mount/MountPointNotSupportedException.java b/src/main/java/org/cryptomator/common/mount/MountPointNotSupportedException.java index e321f23b1..94e59121c 100644 --- a/src/main/java/org/cryptomator/common/mount/MountPointNotSupportedException.java +++ b/src/main/java/org/cryptomator/common/mount/MountPointNotSupportedException.java @@ -1,8 +1,10 @@ package org.cryptomator.common.mount; +import java.nio.file.Path; + public class MountPointNotSupportedException extends IllegalMountPointException { - public MountPointNotSupportedException(String msg) { - super(msg); + public MountPointNotSupportedException(Path path, String msg) { + super(path, msg); } } diff --git a/src/main/java/org/cryptomator/common/mount/MountPointPreparationException.java b/src/main/java/org/cryptomator/common/mount/MountPointPreparationException.java index e4734e011..12a4a1c29 100644 --- a/src/main/java/org/cryptomator/common/mount/MountPointPreparationException.java +++ b/src/main/java/org/cryptomator/common/mount/MountPointPreparationException.java @@ -1,12 +1,18 @@ package org.cryptomator.common.mount; +/** + * Used for wrapping exceptions which occur while preparing the mountpoint.
+ * Instances of this exception are usually treated as generic error. + * + * @see IllegalMountPointException IllegalMountPointException for indicating configuration errors. + */ public class MountPointPreparationException extends RuntimeException { - public MountPointPreparationException(String msg) { - super(msg); - } - public MountPointPreparationException(Throwable cause) { super(cause); } -} + + public MountPointPreparationException(String msg, Throwable cause) { + super(msg, cause); + } +} \ No newline at end of file diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index 2aa9feadc..1727d9374 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -5,12 +5,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.IOException; -import java.nio.file.DirectoryNotEmptyException; import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.LinkOption; -import java.nio.file.NoSuchFileException; -import java.nio.file.NotDirectoryException; import java.nio.file.Path; public final class MountWithinParentUtil { @@ -22,16 +19,16 @@ public final class MountWithinParentUtil { private MountWithinParentUtil() {} - static void prepareParentNoMountPoint(Path mountPoint) throws MountPointPreparationException { + static void prepareParentNoMountPoint(Path mountPoint) throws IllegalMountPointException { Path hideaway = getHideaway(mountPoint); var mpExists = Files.exists(mountPoint, LinkOption.NOFOLLOW_LINKS); var hideExists = Files.exists(hideaway, LinkOption.NOFOLLOW_LINKS); //TODO: possible improvement by just deleting an _empty_ hideaway if (mpExists && hideExists) { //both resources exist (whatever type) - throw new MountPointPreparationException(new FileAlreadyExistsException(hideaway.toString())); + throw new IllegalMountPointException(mountPoint); } else if (!mpExists && !hideExists) { //neither mountpoint nor hideaway exist - throw new MountPointPreparationException(new NoSuchFileException(mountPoint.toString())); + throw new IllegalMountPointException(mountPoint); } else if (!mpExists) { //only hideaway exists checkIsDirectory(hideaway); LOG.info("Mountpoint {} seems to be not properly cleaned up. Will be fixed on unmount.", mountPoint); @@ -54,7 +51,7 @@ public final class MountWithinParentUtil { int attempts = 0; while (!Files.notExists(mountPoint)) { if (attempts >= 10) { - throw new MountPointPreparationException("Path " + mountPoint + " could not be cleared"); + throw new IllegalMountPointException(mountPoint, "Path could not be cleared: " + mountPoint); } Thread.sleep(1000); attempts++; @@ -99,16 +96,16 @@ public final class MountWithinParentUtil { } } - private static void checkIsDirectory(Path toCheck) throws MountPointPreparationException { + private static void checkIsDirectory(Path toCheck) throws IllegalMountPointException { if (!Files.isDirectory(toCheck, LinkOption.NOFOLLOW_LINKS)) { - throw new MountPointPreparationException(new NotDirectoryException(toCheck.toString())); + throw new IllegalMountPointException(toCheck); } } - private static void checkIsEmpty(Path toCheck) throws MountPointPreparationException, IOException { + private static void checkIsEmpty(Path toCheck) throws IllegalMountPointException, IOException { try (var dirStream = Files.list(toCheck)) { if (dirStream.findFirst().isPresent()) { - throw new MountPointPreparationException(new DirectoryNotEmptyException(toCheck.toString())); + throw new IllegalMountPointException(toCheck); } } } diff --git a/src/main/java/org/cryptomator/common/mount/Mounter.java b/src/main/java/org/cryptomator/common/mount/Mounter.java index 593cb6666..cb89aa50e 100644 --- a/src/main/java/org/cryptomator/common/mount/Mounter.java +++ b/src/main/java/org/cryptomator/common/mount/Mounter.java @@ -99,7 +99,7 @@ public class Mounter { var mpIsDriveLetter = userChosenMountPoint.toString().matches("[A-Z]:\\\\"); if (mpIsDriveLetter) { if (driveLetters.getOccupied().contains(userChosenMountPoint)) { - throw new MountPointInUseException(userChosenMountPoint.toString()); + throw new MountPointInUseException(userChosenMountPoint); } } else if (canMountToParent && !canMountToDir) { MountWithinParentUtil.prepareParentNoMountPoint(userChosenMountPoint); @@ -115,13 +115,13 @@ public class Mounter { || (!canMountToParent && !mpIsDriveLetter) // || (!canMountToDir && !canMountToParent && !canMountToSystem && !canMountToDriveLetter); if (configNotSupported) { - throw new MountPointNotSupportedException(e.getMessage()); + throw new MountPointNotSupportedException(userChosenMountPoint, e.getMessage()); } else if (canMountToDir && !canMountToParent && !Files.exists(userChosenMountPoint)) { //mountpoint must exist - throw new MountPointNotExistsException(e.getMessage()); + throw new MountPointNotExistsException(userChosenMountPoint, e.getMessage()); } else { //TODO: add specific exception for !canMountToDir && canMountToParent && !Files.notExists(userChosenMountPoint) - throw new IllegalMountPointException(e.getMessage()); + throw new IllegalMountPointException(userChosenMountPoint, e.getMessage()); } } } From 08a1e1ec7d1d0e70900d4ab3620263fdf153dc36 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Tue, 11 Jul 2023 20:48:46 +0200 Subject: [PATCH 06/33] Added/Refactored exceptions to account for more cases --- .../common/mount/HideawayAlreadyExistsException.java | 10 ++++++++++ .../mount/MountPointCouldNotBeClearedException.java | 10 ++++++++++ .../mount/MountPointNotEmptyDirectoryException.java | 10 ++++++++++ .../common/mount/MountPointNotExistsException.java | 4 ++++ .../common/mount/MountWithinParentUtil.java | 12 ++++++------ 5 files changed, 40 insertions(+), 6 deletions(-) create mode 100644 src/main/java/org/cryptomator/common/mount/HideawayAlreadyExistsException.java create mode 100644 src/main/java/org/cryptomator/common/mount/MountPointCouldNotBeClearedException.java create mode 100644 src/main/java/org/cryptomator/common/mount/MountPointNotEmptyDirectoryException.java diff --git a/src/main/java/org/cryptomator/common/mount/HideawayAlreadyExistsException.java b/src/main/java/org/cryptomator/common/mount/HideawayAlreadyExistsException.java new file mode 100644 index 000000000..57f255b2e --- /dev/null +++ b/src/main/java/org/cryptomator/common/mount/HideawayAlreadyExistsException.java @@ -0,0 +1,10 @@ +package org.cryptomator.common.mount; + +import java.nio.file.Path; + +public class HideawayAlreadyExistsException extends IllegalMountPointException { + + public HideawayAlreadyExistsException(Path path, String msg) { + super(path, msg); + } +} \ No newline at end of file diff --git a/src/main/java/org/cryptomator/common/mount/MountPointCouldNotBeClearedException.java b/src/main/java/org/cryptomator/common/mount/MountPointCouldNotBeClearedException.java new file mode 100644 index 000000000..03150176c --- /dev/null +++ b/src/main/java/org/cryptomator/common/mount/MountPointCouldNotBeClearedException.java @@ -0,0 +1,10 @@ +package org.cryptomator.common.mount; + +import java.nio.file.Path; + +public class MountPointCouldNotBeClearedException extends IllegalMountPointException { + + public MountPointCouldNotBeClearedException(Path path) { + super(path, "Mountpoint could not be cleared: " + path.toString()); + } +} \ No newline at end of file diff --git a/src/main/java/org/cryptomator/common/mount/MountPointNotEmptyDirectoryException.java b/src/main/java/org/cryptomator/common/mount/MountPointNotEmptyDirectoryException.java new file mode 100644 index 000000000..79801613f --- /dev/null +++ b/src/main/java/org/cryptomator/common/mount/MountPointNotEmptyDirectoryException.java @@ -0,0 +1,10 @@ +package org.cryptomator.common.mount; + +import java.nio.file.Path; + +public class MountPointNotEmptyDirectoryException extends IllegalMountPointException { + + public MountPointNotEmptyDirectoryException(Path path, String msg) { + super(path, msg); + } +} \ No newline at end of file diff --git a/src/main/java/org/cryptomator/common/mount/MountPointNotExistsException.java b/src/main/java/org/cryptomator/common/mount/MountPointNotExistsException.java index 677e2fb01..f1c41b0b7 100644 --- a/src/main/java/org/cryptomator/common/mount/MountPointNotExistsException.java +++ b/src/main/java/org/cryptomator/common/mount/MountPointNotExistsException.java @@ -7,4 +7,8 @@ public class MountPointNotExistsException extends IllegalMountPointException { public MountPointNotExistsException(Path path, String msg) { super(path, msg); } + + public MountPointNotExistsException(Path path) { + super(path, "Mountpoint does not exist: " + path); + } } diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index 1727d9374..4c829d68d 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -19,16 +19,16 @@ public final class MountWithinParentUtil { private MountWithinParentUtil() {} - static void prepareParentNoMountPoint(Path mountPoint) throws IllegalMountPointException { + static void prepareParentNoMountPoint(Path mountPoint) throws IllegalMountPointException, MountPointPreparationException { Path hideaway = getHideaway(mountPoint); var mpExists = Files.exists(mountPoint, LinkOption.NOFOLLOW_LINKS); var hideExists = Files.exists(hideaway, LinkOption.NOFOLLOW_LINKS); //TODO: possible improvement by just deleting an _empty_ hideaway if (mpExists && hideExists) { //both resources exist (whatever type) - throw new IllegalMountPointException(mountPoint); + throw new HideawayAlreadyExistsException(mountPoint, "Hideaway (" + hideaway + ") already exists for mountpoint: " + mountPoint); } else if (!mpExists && !hideExists) { //neither mountpoint nor hideaway exist - throw new IllegalMountPointException(mountPoint); + throw new MountPointNotExistsException(mountPoint); } else if (!mpExists) { //only hideaway exists checkIsDirectory(hideaway); LOG.info("Mountpoint {} seems to be not properly cleaned up. Will be fixed on unmount.", mountPoint); @@ -51,7 +51,7 @@ public final class MountWithinParentUtil { int attempts = 0; while (!Files.notExists(mountPoint)) { if (attempts >= 10) { - throw new IllegalMountPointException(mountPoint, "Path could not be cleared: " + mountPoint); + throw new MountPointCouldNotBeClearedException(mountPoint); } Thread.sleep(1000); attempts++; @@ -98,14 +98,14 @@ public final class MountWithinParentUtil { private static void checkIsDirectory(Path toCheck) throws IllegalMountPointException { if (!Files.isDirectory(toCheck, LinkOption.NOFOLLOW_LINKS)) { - throw new IllegalMountPointException(toCheck); + throw new MountPointNotEmptyDirectoryException(toCheck, "Mountpoint is not a directory: " + toCheck); } } private static void checkIsEmpty(Path toCheck) throws IllegalMountPointException, IOException { try (var dirStream = Files.list(toCheck)) { if (dirStream.findFirst().isPresent()) { - throw new IllegalMountPointException(toCheck); + throw new MountPointNotEmptyDirectoryException(toCheck, "Mountpoint directory is not empty: " + toCheck); } } } From becc5e316a5e8e95c214d2e9421c5bb8ac095023 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Tue, 11 Jul 2023 21:00:33 +0200 Subject: [PATCH 07/33] Made interaction between Unlock logic & UI more consistent --- .../ui/unlock/UnlockInvalidMountPointController.java | 5 +++-- src/main/java/org/cryptomator/ui/unlock/UnlockModule.java | 3 ++- .../java/org/cryptomator/ui/unlock/UnlockWorkflow.java | 8 ++++---- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java b/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java index 83bd80df4..1af21a5e0 100644 --- a/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java +++ b/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java @@ -1,5 +1,6 @@ package org.cryptomator.ui.unlock; +import org.cryptomator.common.mount.IllegalMountPointException; import org.cryptomator.common.mount.MountPointInUseException; import org.cryptomator.common.mount.MountPointNotExistsException; import org.cryptomator.common.mount.MountPointNotSupportedException; @@ -30,13 +31,13 @@ public class UnlockInvalidMountPointController implements FxController { public FormattedLabel dialogDescription; @Inject - UnlockInvalidMountPointController(@UnlockWindow Stage window, @UnlockWindow Vault vault, @UnlockWindow AtomicReference unlockException, FxApplicationWindows appWindows, ResourceBundle resourceBundle) { + UnlockInvalidMountPointController(@UnlockWindow Stage window, @UnlockWindow Vault vault, @UnlockWindow AtomicReference illegalMountPointException, FxApplicationWindows appWindows, ResourceBundle resourceBundle) { this.window = window; this.vault = vault; this.appWindows = appWindows; this.resourceBundle = resourceBundle; - var exc = unlockException.get(); + var exc = illegalMountPointException.get(); this.exceptionType = getExceptionType(exc); this.exceptionMessage = exc.getMessage(); } diff --git a/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java b/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java index b59fa272d..c84c12db1 100644 --- a/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java +++ b/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java @@ -4,6 +4,7 @@ import dagger.Binds; import dagger.Module; import dagger.Provides; import dagger.multibindings.IntoMap; +import org.cryptomator.common.mount.IllegalMountPointException; import org.cryptomator.common.vaults.Vault; import org.cryptomator.ui.common.DefaultSceneFactory; import org.cryptomator.ui.common.FxController; @@ -61,7 +62,7 @@ abstract class UnlockModule { @Provides @UnlockWindow @UnlockScoped - static AtomicReference unlockException() { + static AtomicReference illegalMountPointException() { return new AtomicReference<>(); } diff --git a/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java b/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java index cd4f5fcba..ea91a3651 100644 --- a/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java +++ b/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java @@ -40,10 +40,10 @@ public class UnlockWorkflow extends Task { private final Lazy invalidMountPointScene; private final FxApplicationWindows appWindows; private final KeyLoadingStrategy keyLoadingStrategy; - private final AtomicReference unlockFailedException; + private final AtomicReference illegalMountPointException; @Inject - UnlockWorkflow(@UnlockWindow Stage window, @UnlockWindow Vault vault, VaultService vaultService, @FxmlScene(FxmlFile.UNLOCK_SUCCESS) Lazy successScene, @FxmlScene(FxmlFile.UNLOCK_INVALID_MOUNT_POINT) Lazy invalidMountPointScene, FxApplicationWindows appWindows, @UnlockWindow KeyLoadingStrategy keyLoadingStrategy, @UnlockWindow AtomicReference unlockFailedException) { + UnlockWorkflow(@UnlockWindow Stage window, @UnlockWindow Vault vault, VaultService vaultService, @FxmlScene(FxmlFile.UNLOCK_SUCCESS) Lazy successScene, @FxmlScene(FxmlFile.UNLOCK_INVALID_MOUNT_POINT) Lazy invalidMountPointScene, FxApplicationWindows appWindows, @UnlockWindow KeyLoadingStrategy keyLoadingStrategy, @UnlockWindow AtomicReference illegalMountPointException) { this.window = window; this.vault = vault; this.vaultService = vaultService; @@ -51,7 +51,7 @@ public class UnlockWorkflow extends Task { this.invalidMountPointScene = invalidMountPointScene; this.appWindows = appWindows; this.keyLoadingStrategy = keyLoadingStrategy; - this.unlockFailedException = unlockFailedException; + this.illegalMountPointException = illegalMountPointException; } @Override @@ -79,7 +79,7 @@ public class UnlockWorkflow extends Task { private void handleIllegalMountPointError(IllegalMountPointException impe) { Platform.runLater(() -> { - unlockFailedException.set(impe); + illegalMountPointException.set(impe); window.setScene(invalidMountPointScene.get()); window.show(); }); From 9361a75cd88a6c6393c9c9d8561dc29d796f13c3 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Tue, 11 Jul 2023 21:21:16 +0200 Subject: [PATCH 08/33] Refactored UnlockInvalidMountPointController to use improved Exceptions --- .../ui/unlock/UnlockInvalidMountPointController.java | 6 +++++- src/main/resources/i18n/strings.properties | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java b/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java index 1af21a5e0..22812ec0f 100644 --- a/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java +++ b/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java @@ -14,6 +14,7 @@ import org.cryptomator.ui.vaultoptions.SelectedVaultOptionsTab; import javax.inject.Inject; import javafx.fxml.FXML; import javafx.stage.Stage; +import java.nio.file.Path; import java.util.ResourceBundle; import java.util.concurrent.atomic.AtomicReference; @@ -26,6 +27,7 @@ public class UnlockInvalidMountPointController implements FxController { private final FxApplicationWindows appWindows; private final ResourceBundle resourceBundle; private final ExceptionType exceptionType; + private final Path exceptionPath; private final String exceptionMessage; public FormattedLabel dialogDescription; @@ -39,13 +41,15 @@ public class UnlockInvalidMountPointController implements FxController { var exc = illegalMountPointException.get(); this.exceptionType = getExceptionType(exc); + this.exceptionPath = exc.getMountpoint(); this.exceptionMessage = exc.getMessage(); } @FXML public void initialize() { dialogDescription.setFormat(resourceBundle.getString(exceptionType.translationKey)); - dialogDescription.setArg1(exceptionMessage); + dialogDescription.setArg1(exceptionPath); + dialogDescription.setArg2(exceptionMessage); } @FXML diff --git a/src/main/resources/i18n/strings.properties b/src/main/resources/i18n/strings.properties index ca051269a..a9942eb51 100644 --- a/src/main/resources/i18n/strings.properties +++ b/src/main/resources/i18n/strings.properties @@ -131,7 +131,7 @@ unlock.error.customPath.message=Unable to mount vault to custom path unlock.error.customPath.description.notSupported=If you wish to keep using the custom path, please go to the preferences and select a volume type that supports it. Otherwise, go to the vault options and choose a supported mount point. unlock.error.customPath.description.notExists=The custom mount path does not exist. Either create it in your local filesystem or change it in the vault options. unlock.error.customPath.description.inUse=Drive letter "%s" is already in use. -unlock.error.customPath.description.generic=You have selected a custom mount path for this vault, but using it failed with the message: %s +unlock.error.customPath.description.generic=You have selected a custom mount path for this vault, but using it failed with the message: %2$s ## Hub hub.noKeychain.message=Unable to access device key hub.noKeychain.description=In order to unlock Hub vaults, a device key is required, which is secured using a keychain. To proceed, enable “%s” and select a keychain in the preferences. From 2a9d1e3fba8d118475e2df9fbd87a6063e983450 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Tue, 11 Jul 2023 22:33:27 +0200 Subject: [PATCH 09/33] Added messages for MountPointCouldNotBeClearedException and MountPointNotEmptyDirectoryException --- .../ui/unlock/UnlockInvalidMountPointController.java | 6 ++++++ src/main/resources/i18n/strings.properties | 2 ++ 2 files changed, 8 insertions(+) diff --git a/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java b/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java index 22812ec0f..509df30dd 100644 --- a/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java +++ b/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java @@ -1,7 +1,9 @@ package org.cryptomator.ui.unlock; import org.cryptomator.common.mount.IllegalMountPointException; +import org.cryptomator.common.mount.MountPointCouldNotBeClearedException; import org.cryptomator.common.mount.MountPointInUseException; +import org.cryptomator.common.mount.MountPointNotEmptyDirectoryException; import org.cryptomator.common.mount.MountPointNotExistsException; import org.cryptomator.common.mount.MountPointNotSupportedException; import org.cryptomator.common.vaults.Vault; @@ -74,6 +76,8 @@ public class UnlockInvalidMountPointController implements FxController { case MountPointNotSupportedException x -> ExceptionType.NOT_SUPPORTED; case MountPointNotExistsException x -> ExceptionType.NOT_EXISTING; case MountPointInUseException x -> ExceptionType.IN_USE; + case MountPointCouldNotBeClearedException x -> ExceptionType.COULD_NOT_BE_CLEARED; + case MountPointNotEmptyDirectoryException x -> ExceptionType.NOT_EMPTY_DIRECTORY; default -> ExceptionType.GENERIC; }; } @@ -83,6 +87,8 @@ public class UnlockInvalidMountPointController implements FxController { NOT_SUPPORTED("unlock.error.customPath.description.notSupported", ButtonAction.SHOW_PREFERENCES), NOT_EXISTING("unlock.error.customPath.description.notExists", ButtonAction.SHOW_VAULT_OPTIONS), IN_USE("unlock.error.customPath.description.inUse", ButtonAction.SHOW_VAULT_OPTIONS), + COULD_NOT_BE_CLEARED("unlock.error.customPath.description.couldNotBeCleaned", ButtonAction.SHOW_VAULT_OPTIONS), + NOT_EMPTY_DIRECTORY("unlock.error.customPath.description.notEmptyDir", ButtonAction.SHOW_VAULT_OPTIONS), GENERIC("unlock.error.customPath.description.generic", ButtonAction.SHOW_PREFERENCES); private final String translationKey; diff --git a/src/main/resources/i18n/strings.properties b/src/main/resources/i18n/strings.properties index a9942eb51..730609137 100644 --- a/src/main/resources/i18n/strings.properties +++ b/src/main/resources/i18n/strings.properties @@ -131,6 +131,8 @@ unlock.error.customPath.message=Unable to mount vault to custom path unlock.error.customPath.description.notSupported=If you wish to keep using the custom path, please go to the preferences and select a volume type that supports it. Otherwise, go to the vault options and choose a supported mount point. unlock.error.customPath.description.notExists=The custom mount path does not exist. Either create it in your local filesystem or change it in the vault options. unlock.error.customPath.description.inUse=Drive letter "%s" is already in use. +unlock.error.customPath.description.couldNotBeCleaned=Your vault could not be mounted to the path "%s". Please try again or choose a different path. +unlock.error.customPath.description.notEmptyDir=The custom mount path "%s" is not an empty folder. Please choose an empty folder and try again. unlock.error.customPath.description.generic=You have selected a custom mount path for this vault, but using it failed with the message: %2$s ## Hub hub.noKeychain.message=Unable to access device key From 614756c7408f511bea4d92ac265538c85c68b1ae Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Tue, 11 Jul 2023 22:35:08 +0200 Subject: [PATCH 10/33] Added message for HideawayAlreadyExistsException --- .../common/mount/HideawayAlreadyExistsException.java | 9 ++++++++- .../cryptomator/common/mount/MountWithinParentUtil.java | 2 +- .../ui/unlock/UnlockInvalidMountPointController.java | 7 +++++++ src/main/resources/i18n/strings.properties | 1 + 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/cryptomator/common/mount/HideawayAlreadyExistsException.java b/src/main/java/org/cryptomator/common/mount/HideawayAlreadyExistsException.java index 57f255b2e..b2a938b1c 100644 --- a/src/main/java/org/cryptomator/common/mount/HideawayAlreadyExistsException.java +++ b/src/main/java/org/cryptomator/common/mount/HideawayAlreadyExistsException.java @@ -4,7 +4,14 @@ import java.nio.file.Path; public class HideawayAlreadyExistsException extends IllegalMountPointException { - public HideawayAlreadyExistsException(Path path, String msg) { + private final Path hideaway; + + public HideawayAlreadyExistsException(Path path, Path hideaway, String msg) { super(path, msg); + this.hideaway = hideaway; + } + + public Path getHideaway() { + return hideaway; } } \ No newline at end of file diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index 4c829d68d..1ee1b37cf 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -26,7 +26,7 @@ public final class MountWithinParentUtil { //TODO: possible improvement by just deleting an _empty_ hideaway if (mpExists && hideExists) { //both resources exist (whatever type) - throw new HideawayAlreadyExistsException(mountPoint, "Hideaway (" + hideaway + ") already exists for mountpoint: " + mountPoint); + throw new HideawayAlreadyExistsException(mountPoint, hideaway, "Hideaway (" + hideaway + ") already exists for mountpoint: " + mountPoint); } else if (!mpExists && !hideExists) { //neither mountpoint nor hideaway exist throw new MountPointNotExistsException(mountPoint); } else if (!mpExists) { //only hideaway exists diff --git a/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java b/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java index 509df30dd..37ae85b9b 100644 --- a/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java +++ b/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java @@ -1,5 +1,6 @@ package org.cryptomator.ui.unlock; +import org.cryptomator.common.mount.HideawayAlreadyExistsException; import org.cryptomator.common.mount.IllegalMountPointException; import org.cryptomator.common.mount.MountPointCouldNotBeClearedException; import org.cryptomator.common.mount.MountPointInUseException; @@ -31,6 +32,7 @@ public class UnlockInvalidMountPointController implements FxController { private final ExceptionType exceptionType; private final Path exceptionPath; private final String exceptionMessage; + private final Path hideawayPath; public FormattedLabel dialogDescription; @@ -45,6 +47,7 @@ public class UnlockInvalidMountPointController implements FxController { this.exceptionType = getExceptionType(exc); this.exceptionPath = exc.getMountpoint(); this.exceptionMessage = exc.getMessage(); + this.hideawayPath = exc instanceof HideawayAlreadyExistsException haeExc ? haeExc.getHideaway() : null; } @FXML @@ -52,6 +55,7 @@ public class UnlockInvalidMountPointController implements FxController { dialogDescription.setFormat(resourceBundle.getString(exceptionType.translationKey)); dialogDescription.setArg1(exceptionPath); dialogDescription.setArg2(exceptionMessage); + dialogDescription.setArg3(hideawayPath); } @FXML @@ -76,6 +80,7 @@ public class UnlockInvalidMountPointController implements FxController { case MountPointNotSupportedException x -> ExceptionType.NOT_SUPPORTED; case MountPointNotExistsException x -> ExceptionType.NOT_EXISTING; case MountPointInUseException x -> ExceptionType.IN_USE; + case HideawayAlreadyExistsException x -> ExceptionType.HIDEAWAY_EXISTS; case MountPointCouldNotBeClearedException x -> ExceptionType.COULD_NOT_BE_CLEARED; case MountPointNotEmptyDirectoryException x -> ExceptionType.NOT_EMPTY_DIRECTORY; default -> ExceptionType.GENERIC; @@ -87,6 +92,7 @@ public class UnlockInvalidMountPointController implements FxController { NOT_SUPPORTED("unlock.error.customPath.description.notSupported", ButtonAction.SHOW_PREFERENCES), NOT_EXISTING("unlock.error.customPath.description.notExists", ButtonAction.SHOW_VAULT_OPTIONS), IN_USE("unlock.error.customPath.description.inUse", ButtonAction.SHOW_VAULT_OPTIONS), + HIDEAWAY_EXISTS("unlock.error.customPath.description.hideawayExists", ButtonAction.SHOW_VAULT_OPTIONS), COULD_NOT_BE_CLEARED("unlock.error.customPath.description.couldNotBeCleaned", ButtonAction.SHOW_VAULT_OPTIONS), NOT_EMPTY_DIRECTORY("unlock.error.customPath.description.notEmptyDir", ButtonAction.SHOW_VAULT_OPTIONS), GENERIC("unlock.error.customPath.description.generic", ButtonAction.SHOW_PREFERENCES); @@ -102,6 +108,7 @@ public class UnlockInvalidMountPointController implements FxController { private enum ButtonAction { + //TODO Add option to show filesystem, e.g. for ExceptionType.HIDEAWAY_EXISTS SHOW_PREFERENCES, SHOW_VAULT_OPTIONS; diff --git a/src/main/resources/i18n/strings.properties b/src/main/resources/i18n/strings.properties index 730609137..d0d770329 100644 --- a/src/main/resources/i18n/strings.properties +++ b/src/main/resources/i18n/strings.properties @@ -131,6 +131,7 @@ unlock.error.customPath.message=Unable to mount vault to custom path unlock.error.customPath.description.notSupported=If you wish to keep using the custom path, please go to the preferences and select a volume type that supports it. Otherwise, go to the vault options and choose a supported mount point. unlock.error.customPath.description.notExists=The custom mount path does not exist. Either create it in your local filesystem or change it in the vault options. unlock.error.customPath.description.inUse=Drive letter "%s" is already in use. +unlock.error.customPath.description.hideawayExists=The folder "%3$s", which is used to preserve folder properties set by you, has not been automatically cleaned up since your last mount to "%1$s". Please delete it manually and restore any properties to the original mount path if you set any. unlock.error.customPath.description.couldNotBeCleaned=Your vault could not be mounted to the path "%s". Please try again or choose a different path. unlock.error.customPath.description.notEmptyDir=The custom mount path "%s" is not an empty folder. Please choose an empty folder and try again. unlock.error.customPath.description.generic=You have selected a custom mount path for this vault, but using it failed with the message: %2$s From 96ebb67085c17960772bdaddb70faf58c81f9c90 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Wed, 12 Jul 2023 19:17:37 +0200 Subject: [PATCH 11/33] Applied suggestions from code review See: https://github.com/cryptomator/cryptomator/pull/2996#discussion_r1260955250 --- .../cryptomator/common/mount/MountWithinParentUtil.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index 04e84f64e..cb9992359 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -4,6 +4,7 @@ import org.apache.commons.lang3.SystemUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.FileNotFoundException; import java.io.IOException; import java.nio.file.DirectoryNotEmptyException; import java.nio.file.FileAlreadyExistsException; @@ -74,18 +75,14 @@ public final class MountWithinParentUtil { private static boolean removeResidualJunction(Path path) throws MountPointPreparationException { try { - if (!Files.exists(path, LinkOption.NOFOLLOW_LINKS)) { - return false; - } - if (!SystemUtils.IS_OS_WINDOWS) { //So far this is only a problem on Windows - return true; - } if (Files.readAttributes(path, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS).isOther()) { LOG.info("Mountpoint \"{}\" is still a junction. Deleting it.", path); Files.delete(path); //Throws if path is also a non-empty folder return false; } return true; + } catch (FileNotFoundException e) { + return false; } catch (IOException e) { throw new MountPointPreparationException(e); } From f5e035fa3b5d536f7450c66cccb9ba7475d5ab89 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Thu, 13 Jul 2023 15:22:21 +0200 Subject: [PATCH 12/33] Applied suggestions from code review Updated log message Replaced caught exception See: https://github.com/cryptomator/cryptomator/pull/2996#discussion_r1262297316 https://github.com/cryptomator/cryptomator/pull/2996#discussion_r1262298832 --- .../org/cryptomator/common/mount/MountWithinParentUtil.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index cb9992359..9b08a10cd 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -81,7 +81,7 @@ public final class MountWithinParentUtil { return false; } return true; - } catch (FileNotFoundException e) { + } catch (NoSuchFileException e) { return false; } catch (IOException e) { throw new MountPointPreparationException(e); @@ -100,7 +100,7 @@ public final class MountWithinParentUtil { try { waitForMountpointRestoration(mountPoint); if (Files.notExists(hideaway, LinkOption.NOFOLLOW_LINKS)) { - LOG.error("Unable to restore hidden directory to mountpoint \"{}\": Directory does not exist. (Deleted by user?)", mountPoint); + LOG.error("Unable to restore hidden directory to mountpoint \"{}\": Directory does not exist.", mountPoint); return; } From a259d554fa18a3c25d2492321347f976dfd76258 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Fri, 14 Jul 2023 14:53:00 +0200 Subject: [PATCH 13/33] Removed unused import See: https://github.com/cryptomator/cryptomator/pull/2996#discussion_r1263556873 --- .../java/org/cryptomator/common/mount/MountWithinParentUtil.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index 9b08a10cd..6a7d96d5c 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -4,7 +4,6 @@ import org.apache.commons.lang3.SystemUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.FileNotFoundException; import java.io.IOException; import java.nio.file.DirectoryNotEmptyException; import java.nio.file.FileAlreadyExistsException; From aa2e63acb0f86e9b036585cdca0c2884cbf4ec18 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Fri, 14 Jul 2023 14:58:56 +0200 Subject: [PATCH 14/33] Added `@PropertyKey` See: https://github.com/cryptomator/cryptomator/pull/3001#discussion_r1263039593 --- .../ui/unlock/UnlockInvalidMountPointController.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java b/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java index 37ae85b9b..9144f0a74 100644 --- a/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java +++ b/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java @@ -13,6 +13,7 @@ import org.cryptomator.ui.controls.FormattedLabel; import org.cryptomator.ui.fxapp.FxApplicationWindows; import org.cryptomator.ui.preferences.SelectedPreferencesTab; import org.cryptomator.ui.vaultoptions.SelectedVaultOptionsTab; +import org.jetbrains.annotations.PropertyKey; import javax.inject.Inject; import javafx.fxml.FXML; @@ -100,7 +101,7 @@ public class UnlockInvalidMountPointController implements FxController { private final String translationKey; private final ButtonAction action; - ExceptionType(String translationKey, ButtonAction action) { + ExceptionType(@PropertyKey(resourceBundle = "i18n.strings") String translationKey, ButtonAction action) { this.translationKey = translationKey; this.action = action; } From a90e75ceba86d41d2dcedc094dc321de29273720 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Fri, 14 Jul 2023 15:09:36 +0200 Subject: [PATCH 15/33] Renamed exceptions See: https://github.com/cryptomator/cryptomator/pull/3001#discussion_r1263563166 --- ...ception.java => ExistingHideawayException.java} | 4 ++-- .../mount/MountPointCleanupFailedException.java | 10 ++++++++++ .../MountPointCouldNotBeClearedException.java | 10 ---------- .../mount/MountPointNotExistingException.java | 14 ++++++++++++++ .../common/mount/MountPointNotExistsException.java | 14 -------------- .../common/mount/MountWithinParentUtil.java | 6 +++--- .../java/org/cryptomator/common/mount/Mounter.java | 2 +- .../unlock/UnlockInvalidMountPointController.java | 14 +++++++------- 8 files changed, 37 insertions(+), 37 deletions(-) rename src/main/java/org/cryptomator/common/mount/{HideawayAlreadyExistsException.java => ExistingHideawayException.java} (55%) create mode 100644 src/main/java/org/cryptomator/common/mount/MountPointCleanupFailedException.java delete mode 100644 src/main/java/org/cryptomator/common/mount/MountPointCouldNotBeClearedException.java create mode 100644 src/main/java/org/cryptomator/common/mount/MountPointNotExistingException.java delete mode 100644 src/main/java/org/cryptomator/common/mount/MountPointNotExistsException.java diff --git a/src/main/java/org/cryptomator/common/mount/HideawayAlreadyExistsException.java b/src/main/java/org/cryptomator/common/mount/ExistingHideawayException.java similarity index 55% rename from src/main/java/org/cryptomator/common/mount/HideawayAlreadyExistsException.java rename to src/main/java/org/cryptomator/common/mount/ExistingHideawayException.java index b2a938b1c..aa720d42c 100644 --- a/src/main/java/org/cryptomator/common/mount/HideawayAlreadyExistsException.java +++ b/src/main/java/org/cryptomator/common/mount/ExistingHideawayException.java @@ -2,11 +2,11 @@ package org.cryptomator.common.mount; import java.nio.file.Path; -public class HideawayAlreadyExistsException extends IllegalMountPointException { +public class ExistingHideawayException extends IllegalMountPointException { private final Path hideaway; - public HideawayAlreadyExistsException(Path path, Path hideaway, String msg) { + public ExistingHideawayException(Path path, Path hideaway, String msg) { super(path, msg); this.hideaway = hideaway; } diff --git a/src/main/java/org/cryptomator/common/mount/MountPointCleanupFailedException.java b/src/main/java/org/cryptomator/common/mount/MountPointCleanupFailedException.java new file mode 100644 index 000000000..6246e124c --- /dev/null +++ b/src/main/java/org/cryptomator/common/mount/MountPointCleanupFailedException.java @@ -0,0 +1,10 @@ +package org.cryptomator.common.mount; + +import java.nio.file.Path; + +public class MountPointCleanupFailedException extends IllegalMountPointException { + + public MountPointCleanupFailedException(Path path) { + super(path, "Mountpoint could not be cleared: " + path.toString()); + } +} \ No newline at end of file diff --git a/src/main/java/org/cryptomator/common/mount/MountPointCouldNotBeClearedException.java b/src/main/java/org/cryptomator/common/mount/MountPointCouldNotBeClearedException.java deleted file mode 100644 index 03150176c..000000000 --- a/src/main/java/org/cryptomator/common/mount/MountPointCouldNotBeClearedException.java +++ /dev/null @@ -1,10 +0,0 @@ -package org.cryptomator.common.mount; - -import java.nio.file.Path; - -public class MountPointCouldNotBeClearedException extends IllegalMountPointException { - - public MountPointCouldNotBeClearedException(Path path) { - super(path, "Mountpoint could not be cleared: " + path.toString()); - } -} \ No newline at end of file diff --git a/src/main/java/org/cryptomator/common/mount/MountPointNotExistingException.java b/src/main/java/org/cryptomator/common/mount/MountPointNotExistingException.java new file mode 100644 index 000000000..bebf63ee3 --- /dev/null +++ b/src/main/java/org/cryptomator/common/mount/MountPointNotExistingException.java @@ -0,0 +1,14 @@ +package org.cryptomator.common.mount; + +import java.nio.file.Path; + +public class MountPointNotExistingException extends IllegalMountPointException { + + public MountPointNotExistingException(Path path, String msg) { + super(path, msg); + } + + public MountPointNotExistingException(Path path) { + super(path, "Mountpoint does not exist: " + path); + } +} diff --git a/src/main/java/org/cryptomator/common/mount/MountPointNotExistsException.java b/src/main/java/org/cryptomator/common/mount/MountPointNotExistsException.java deleted file mode 100644 index f1c41b0b7..000000000 --- a/src/main/java/org/cryptomator/common/mount/MountPointNotExistsException.java +++ /dev/null @@ -1,14 +0,0 @@ -package org.cryptomator.common.mount; - -import java.nio.file.Path; - -public class MountPointNotExistsException extends IllegalMountPointException { - - public MountPointNotExistsException(Path path, String msg) { - super(path, msg); - } - - public MountPointNotExistsException(Path path) { - super(path, "Mountpoint does not exist: " + path); - } -} diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index 1ee1b37cf..e7a703e43 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -26,9 +26,9 @@ public final class MountWithinParentUtil { //TODO: possible improvement by just deleting an _empty_ hideaway if (mpExists && hideExists) { //both resources exist (whatever type) - throw new HideawayAlreadyExistsException(mountPoint, hideaway, "Hideaway (" + hideaway + ") already exists for mountpoint: " + mountPoint); + throw new ExistingHideawayException(mountPoint, hideaway, "Hideaway (" + hideaway + ") already exists for mountpoint: " + mountPoint); } else if (!mpExists && !hideExists) { //neither mountpoint nor hideaway exist - throw new MountPointNotExistsException(mountPoint); + throw new MountPointNotExistingException(mountPoint); } else if (!mpExists) { //only hideaway exists checkIsDirectory(hideaway); LOG.info("Mountpoint {} seems to be not properly cleaned up. Will be fixed on unmount.", mountPoint); @@ -51,7 +51,7 @@ public final class MountWithinParentUtil { int attempts = 0; while (!Files.notExists(mountPoint)) { if (attempts >= 10) { - throw new MountPointCouldNotBeClearedException(mountPoint); + throw new MountPointCleanupFailedException(mountPoint); } Thread.sleep(1000); attempts++; diff --git a/src/main/java/org/cryptomator/common/mount/Mounter.java b/src/main/java/org/cryptomator/common/mount/Mounter.java index cb89aa50e..101524ea3 100644 --- a/src/main/java/org/cryptomator/common/mount/Mounter.java +++ b/src/main/java/org/cryptomator/common/mount/Mounter.java @@ -118,7 +118,7 @@ public class Mounter { throw new MountPointNotSupportedException(userChosenMountPoint, e.getMessage()); } else if (canMountToDir && !canMountToParent && !Files.exists(userChosenMountPoint)) { //mountpoint must exist - throw new MountPointNotExistsException(userChosenMountPoint, e.getMessage()); + throw new MountPointNotExistingException(userChosenMountPoint, e.getMessage()); } else { //TODO: add specific exception for !canMountToDir && canMountToParent && !Files.notExists(userChosenMountPoint) throw new IllegalMountPointException(userChosenMountPoint, e.getMessage()); diff --git a/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java b/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java index 9144f0a74..6e11ed131 100644 --- a/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java +++ b/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java @@ -1,11 +1,11 @@ package org.cryptomator.ui.unlock; -import org.cryptomator.common.mount.HideawayAlreadyExistsException; +import org.cryptomator.common.mount.ExistingHideawayException; import org.cryptomator.common.mount.IllegalMountPointException; -import org.cryptomator.common.mount.MountPointCouldNotBeClearedException; +import org.cryptomator.common.mount.MountPointCleanupFailedException; import org.cryptomator.common.mount.MountPointInUseException; import org.cryptomator.common.mount.MountPointNotEmptyDirectoryException; -import org.cryptomator.common.mount.MountPointNotExistsException; +import org.cryptomator.common.mount.MountPointNotExistingException; import org.cryptomator.common.mount.MountPointNotSupportedException; import org.cryptomator.common.vaults.Vault; import org.cryptomator.ui.common.FxController; @@ -48,7 +48,7 @@ public class UnlockInvalidMountPointController implements FxController { this.exceptionType = getExceptionType(exc); this.exceptionPath = exc.getMountpoint(); this.exceptionMessage = exc.getMessage(); - this.hideawayPath = exc instanceof HideawayAlreadyExistsException haeExc ? haeExc.getHideaway() : null; + this.hideawayPath = exc instanceof ExistingHideawayException haeExc ? haeExc.getHideaway() : null; } @FXML @@ -79,10 +79,10 @@ public class UnlockInvalidMountPointController implements FxController { private ExceptionType getExceptionType(Throwable unlockException) { return switch (unlockException) { case MountPointNotSupportedException x -> ExceptionType.NOT_SUPPORTED; - case MountPointNotExistsException x -> ExceptionType.NOT_EXISTING; + case MountPointNotExistingException x -> ExceptionType.NOT_EXISTING; case MountPointInUseException x -> ExceptionType.IN_USE; - case HideawayAlreadyExistsException x -> ExceptionType.HIDEAWAY_EXISTS; - case MountPointCouldNotBeClearedException x -> ExceptionType.COULD_NOT_BE_CLEARED; + case ExistingHideawayException x -> ExceptionType.HIDEAWAY_EXISTS; + case MountPointCleanupFailedException x -> ExceptionType.COULD_NOT_BE_CLEARED; case MountPointNotEmptyDirectoryException x -> ExceptionType.NOT_EMPTY_DIRECTORY; default -> ExceptionType.GENERIC; }; From c3ac043c68d4add2c2ef687f34b541219cca81e2 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Fri, 14 Jul 2023 21:13:11 +0200 Subject: [PATCH 16/33] Removed MountPointPreparationException See: https://github.com/cryptomator/cryptomator/pull/3001#discussion_r1263572178 --- .../mount/IllegalMountPointException.java | 2 -- .../mount/MountPointPreparationException.java | 18 ------------------ .../common/mount/MountWithinParentUtil.java | 9 +++++---- 3 files changed, 5 insertions(+), 24 deletions(-) delete mode 100644 src/main/java/org/cryptomator/common/mount/MountPointPreparationException.java diff --git a/src/main/java/org/cryptomator/common/mount/IllegalMountPointException.java b/src/main/java/org/cryptomator/common/mount/IllegalMountPointException.java index a96e4f0bb..30419d85c 100644 --- a/src/main/java/org/cryptomator/common/mount/IllegalMountPointException.java +++ b/src/main/java/org/cryptomator/common/mount/IllegalMountPointException.java @@ -5,8 +5,6 @@ import java.nio.file.Path; /** * Indicates that validation or preparation of a mountpoint failed due to a configuration error or an invalid system state.
* Instances of this exception are usually caught and displayed to the user in an appropriate fashion, e.g. by {@link org.cryptomator.ui.unlock.UnlockInvalidMountPointController UnlockInvalidMountPointController.} - * - * @see MountPointPreparationException MountPointPreparationException for wrapping exceptions which occur while preparing the mountpoint. */ public class IllegalMountPointException extends IllegalArgumentException { diff --git a/src/main/java/org/cryptomator/common/mount/MountPointPreparationException.java b/src/main/java/org/cryptomator/common/mount/MountPointPreparationException.java deleted file mode 100644 index 12a4a1c29..000000000 --- a/src/main/java/org/cryptomator/common/mount/MountPointPreparationException.java +++ /dev/null @@ -1,18 +0,0 @@ -package org.cryptomator.common.mount; - -/** - * Used for wrapping exceptions which occur while preparing the mountpoint.
- * Instances of this exception are usually treated as generic error. - * - * @see IllegalMountPointException IllegalMountPointException for indicating configuration errors. - */ -public class MountPointPreparationException extends RuntimeException { - - public MountPointPreparationException(Throwable cause) { - super(cause); - } - - public MountPointPreparationException(String msg, Throwable cause) { - super(msg, cause); - } -} \ No newline at end of file diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index e7a703e43..64f382e02 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -5,6 +5,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.LinkOption; @@ -19,7 +20,7 @@ public final class MountWithinParentUtil { private MountWithinParentUtil() {} - static void prepareParentNoMountPoint(Path mountPoint) throws IllegalMountPointException, MountPointPreparationException { + static void prepareParentNoMountPoint(Path mountPoint) throws IllegalMountPointException { Path hideaway = getHideaway(mountPoint); var mpExists = Files.exists(mountPoint, LinkOption.NOFOLLOW_LINKS); var hideExists = Files.exists(hideaway, LinkOption.NOFOLLOW_LINKS); @@ -37,7 +38,7 @@ public final class MountWithinParentUtil { Files.setAttribute(hideaway, WIN_HIDDEN_ATTR, true, LinkOption.NOFOLLOW_LINKS); } } catch (IOException e) { - throw new MountPointPreparationException(e); + throw new UncheckedIOException(e); } } else { //only mountpoint exists try { @@ -57,10 +58,10 @@ public final class MountWithinParentUtil { attempts++; } } catch (IOException e) { - throw new MountPointPreparationException(e); + throw new UncheckedIOException(e); } catch (InterruptedException e) { Thread.currentThread().interrupt(); - throw new MountPointPreparationException(e); + throw new RuntimeException(e); } } } From 0b6782d44bc8c8efdae20fb9626e770fa9c65bd1 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Tue, 18 Jul 2023 20:06:50 +0200 Subject: [PATCH 17/33] Refactored error message to use ObservableValues --- .../UnlockInvalidMountPointController.java | 87 ++++++++++++++----- .../cryptomator/ui/unlock/UnlockModule.java | 7 +- .../cryptomator/ui/unlock/UnlockWorkflow.java | 6 +- .../fxml/unlock_invalid_mount_point.fxml | 2 +- 4 files changed, 71 insertions(+), 31 deletions(-) diff --git a/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java b/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java index 6e11ed131..14b74e377 100644 --- a/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java +++ b/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java @@ -1,5 +1,6 @@ package org.cryptomator.ui.unlock; +import org.cryptomator.common.ObservableUtil; import org.cryptomator.common.mount.ExistingHideawayException; import org.cryptomator.common.mount.IllegalMountPointException; import org.cryptomator.common.mount.MountPointCleanupFailedException; @@ -16,11 +17,12 @@ import org.cryptomator.ui.vaultoptions.SelectedVaultOptionsTab; import org.jetbrains.annotations.PropertyKey; import javax.inject.Inject; +import javafx.beans.property.ObjectProperty; +import javafx.beans.value.ObservableValue; import javafx.fxml.FXML; import javafx.stage.Stage; import java.nio.file.Path; import java.util.ResourceBundle; -import java.util.concurrent.atomic.AtomicReference; //At the current point in time only the CustomMountPointChooser may cause this window to be shown. @UnlockScoped @@ -29,34 +31,31 @@ public class UnlockInvalidMountPointController implements FxController { private final Stage window; private final Vault vault; private final FxApplicationWindows appWindows; - private final ResourceBundle resourceBundle; - private final ExceptionType exceptionType; - private final Path exceptionPath; - private final String exceptionMessage; - private final Path hideawayPath; + + private final ObservableValue exceptionType; + private final ObservableValue exceptionPath; + private final ObservableValue exceptionMessage; + private final ObservableValue hideawayPath; + private final ObservableValue format; + private final ObservableValue showPreferences; + private final ObservableValue showVaultOptions; public FormattedLabel dialogDescription; @Inject - UnlockInvalidMountPointController(@UnlockWindow Stage window, @UnlockWindow Vault vault, @UnlockWindow AtomicReference illegalMountPointException, FxApplicationWindows appWindows, ResourceBundle resourceBundle) { + UnlockInvalidMountPointController(@UnlockWindow Stage window, @UnlockWindow Vault vault, @UnlockWindow ObjectProperty illegalMountPointException, FxApplicationWindows appWindows, ResourceBundle resourceBundle) { this.window = window; this.vault = vault; this.appWindows = appWindows; - this.resourceBundle = resourceBundle; - var exc = illegalMountPointException.get(); - this.exceptionType = getExceptionType(exc); - this.exceptionPath = exc.getMountpoint(); - this.exceptionMessage = exc.getMessage(); - this.hideawayPath = exc instanceof ExistingHideawayException haeExc ? haeExc.getHideaway() : null; - } + this.exceptionType = illegalMountPointException.map(this::getExceptionType); + this.exceptionPath = illegalMountPointException.map(IllegalMountPointException::getMountpoint); + this.exceptionMessage = illegalMountPointException.map(IllegalMountPointException::getMessage); + this.hideawayPath = illegalMountPointException.map(e -> e instanceof ExistingHideawayException haeExc ? haeExc.getHideaway() : null); - @FXML - public void initialize() { - dialogDescription.setFormat(resourceBundle.getString(exceptionType.translationKey)); - dialogDescription.setArg1(exceptionPath); - dialogDescription.setArg2(exceptionMessage); - dialogDescription.setArg3(hideawayPath); + this.format = ObservableUtil.mapWithDefault(exceptionType, type -> resourceBundle.getString(type.translationKey), ""); + this.showPreferences = ObservableUtil.mapWithDefault(exceptionType, type -> type.action == ButtonAction.SHOW_PREFERENCES, false); + this.showVaultOptions = ObservableUtil.mapWithDefault(exceptionType, type -> type.action == ButtonAction.SHOW_VAULT_OPTIONS, false); } @FXML @@ -117,11 +116,51 @@ public class UnlockInvalidMountPointController implements FxController { /* Getter */ - public boolean isShowPreferences() { - return exceptionType.action == ButtonAction.SHOW_PREFERENCES; + public Path getExceptionPath() { + return exceptionPath.getValue(); } - public boolean isShowVaultOptions() { - return exceptionType.action == ButtonAction.SHOW_VAULT_OPTIONS; + public ObservableValue exceptionPathProperty() { + return exceptionPath; + } + + public String getFormat() { + return format.getValue(); + } + + public ObservableValue formatProperty() { + return format; + } + + public String getExceptionMessage() { + return exceptionMessage.getValue(); + } + + public ObservableValue exceptionMessageProperty() { + return exceptionMessage; + } + + public Path getHideawayPath() { + return hideawayPath.getValue(); + } + + public ObservableValue hideawayPathProperty() { + return hideawayPath; + } + + public Boolean getShowPreferences() { + return showPreferences.getValue(); + } + + public ObservableValue showPreferencesProperty() { + return showPreferences; + } + + public Boolean getShowVaultOptions() { + return showVaultOptions.getValue(); + } + + public ObservableValue showVaultOptionsProperty() { + return showVaultOptions; } } \ No newline at end of file diff --git a/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java b/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java index c84c12db1..f93999d21 100644 --- a/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java +++ b/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java @@ -19,12 +19,13 @@ import org.jetbrains.annotations.Nullable; import javax.inject.Named; import javax.inject.Provider; +import javafx.beans.property.ObjectProperty; +import javafx.beans.property.SimpleObjectProperty; import javafx.scene.Scene; import javafx.stage.Modality; import javafx.stage.Stage; import java.util.Map; import java.util.ResourceBundle; -import java.util.concurrent.atomic.AtomicReference; @Module(subcomponents = {KeyLoadingComponent.class}) abstract class UnlockModule { @@ -62,8 +63,8 @@ abstract class UnlockModule { @Provides @UnlockWindow @UnlockScoped - static AtomicReference illegalMountPointException() { - return new AtomicReference<>(); + static ObjectProperty illegalMountPointException() { + return new SimpleObjectProperty<>(); } @Provides diff --git a/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java b/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java index ea91a3651..564d57ab6 100644 --- a/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java +++ b/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java @@ -17,11 +17,11 @@ import org.slf4j.LoggerFactory; import javax.inject.Inject; import javafx.application.Platform; +import javafx.beans.property.ObjectProperty; import javafx.concurrent.Task; import javafx.scene.Scene; import javafx.stage.Stage; import java.io.IOException; -import java.util.concurrent.atomic.AtomicReference; /** * A multi-step task that consists of background activities as well as user interaction. @@ -40,10 +40,10 @@ public class UnlockWorkflow extends Task { private final Lazy invalidMountPointScene; private final FxApplicationWindows appWindows; private final KeyLoadingStrategy keyLoadingStrategy; - private final AtomicReference illegalMountPointException; + private final ObjectProperty illegalMountPointException; @Inject - UnlockWorkflow(@UnlockWindow Stage window, @UnlockWindow Vault vault, VaultService vaultService, @FxmlScene(FxmlFile.UNLOCK_SUCCESS) Lazy successScene, @FxmlScene(FxmlFile.UNLOCK_INVALID_MOUNT_POINT) Lazy invalidMountPointScene, FxApplicationWindows appWindows, @UnlockWindow KeyLoadingStrategy keyLoadingStrategy, @UnlockWindow AtomicReference illegalMountPointException) { + UnlockWorkflow(@UnlockWindow Stage window, @UnlockWindow Vault vault, VaultService vaultService, @FxmlScene(FxmlFile.UNLOCK_SUCCESS) Lazy successScene, @FxmlScene(FxmlFile.UNLOCK_INVALID_MOUNT_POINT) Lazy invalidMountPointScene, FxApplicationWindows appWindows, @UnlockWindow KeyLoadingStrategy keyLoadingStrategy, @UnlockWindow ObjectProperty illegalMountPointException) { this.window = window; this.vault = vault; this.vaultService = vaultService; diff --git a/src/main/resources/fxml/unlock_invalid_mount_point.fxml b/src/main/resources/fxml/unlock_invalid_mount_point.fxml index c6f1a31f2..dbf6fad05 100644 --- a/src/main/resources/fxml/unlock_invalid_mount_point.fxml +++ b/src/main/resources/fxml/unlock_invalid_mount_point.fxml @@ -40,7 +40,7 @@ - + From bb0b1b35926771e55832a05a6206abd14dba866f Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Tue, 11 Jul 2023 20:48:46 +0200 Subject: [PATCH 18/33] Cleaned up merge --- .../common/mount/ExistingHideawayException.java | 17 ----------------- .../mount/HideawayNotDirectoryException.java | 17 +++++++++++++++++ .../common/mount/MountWithinParentUtil.java | 15 +++++---------- .../UnlockInvalidMountPointController.java | 8 ++++---- src/main/resources/i18n/strings.properties | 2 +- 5 files changed, 27 insertions(+), 32 deletions(-) delete mode 100644 src/main/java/org/cryptomator/common/mount/ExistingHideawayException.java create mode 100644 src/main/java/org/cryptomator/common/mount/HideawayNotDirectoryException.java diff --git a/src/main/java/org/cryptomator/common/mount/ExistingHideawayException.java b/src/main/java/org/cryptomator/common/mount/ExistingHideawayException.java deleted file mode 100644 index aa720d42c..000000000 --- a/src/main/java/org/cryptomator/common/mount/ExistingHideawayException.java +++ /dev/null @@ -1,17 +0,0 @@ -package org.cryptomator.common.mount; - -import java.nio.file.Path; - -public class ExistingHideawayException extends IllegalMountPointException { - - private final Path hideaway; - - public ExistingHideawayException(Path path, Path hideaway, String msg) { - super(path, msg); - this.hideaway = hideaway; - } - - public Path getHideaway() { - return hideaway; - } -} \ No newline at end of file diff --git a/src/main/java/org/cryptomator/common/mount/HideawayNotDirectoryException.java b/src/main/java/org/cryptomator/common/mount/HideawayNotDirectoryException.java new file mode 100644 index 000000000..506f0f10b --- /dev/null +++ b/src/main/java/org/cryptomator/common/mount/HideawayNotDirectoryException.java @@ -0,0 +1,17 @@ +package org.cryptomator.common.mount; + +import java.nio.file.Path; + +public class HideawayNotDirectoryException extends IllegalMountPointException { + + private final Path hideaway; + + public HideawayNotDirectoryException(Path path, Path hideaway) { + super(path, "Existing hideaway (" + hideaway.toString() + ") for mountpoint is not a directory: " + path.toString()); + this.hideaway = hideaway; + } + + public Path getHideaway() { + return hideaway; + } +} \ No newline at end of file diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index cabaf6775..38bc437de 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -10,7 +10,6 @@ import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.LinkOption; import java.nio.file.NoSuchFileException; -import java.nio.file.NotDirectoryException; import java.nio.file.Path; import java.nio.file.attribute.BasicFileAttributes; @@ -28,8 +27,6 @@ public final class MountWithinParentUtil { var mpExists = removeResidualJunction(mountPoint); //Handle junction as not existing var hideExists = Files.exists(hideaway, LinkOption.NOFOLLOW_LINKS); - //TODO: possible improvement by just deleting an _empty_ hideaway - //TODO: Remove "ExistingHideawayException" if (!mpExists && !hideExists) { //neither mountpoint nor hideaway exist throw new MountPointNotExistingException(mountPoint); } else if (!mpExists) { //only hideaway exists @@ -45,7 +42,7 @@ public final class MountWithinParentUtil { } else { //mountpoint exists... try { if (hideExists) { //... with hideaway - removeResidualHideaway(hideaway); + removeResidualHideaway(mountPoint, hideaway); } //... (now) without hideaway @@ -73,8 +70,7 @@ public final class MountWithinParentUtil { } } - //TODO Remove MountPointPreparationException - private static boolean removeResidualJunction(Path path) throws MountPointPreparationException { + private static boolean removeResidualJunction(Path path) { try { if (Files.readAttributes(path, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS).isOther()) { LOG.info("Mountpoint \"{}\" is still a junction. Deleting it.", path); @@ -85,14 +81,13 @@ public final class MountWithinParentUtil { } catch (NoSuchFileException e) { return false; } catch (IOException e) { - throw new MountPointPreparationException(e); + throw new UncheckedIOException(e); } } - //TODO Remove MountPointPreparationException - private static void removeResidualHideaway(Path hideaway) throws IOException { + private static void removeResidualHideaway(Path mountPoint, Path hideaway) throws IOException { if (!Files.isDirectory(hideaway, LinkOption.NOFOLLOW_LINKS)) { - throw new MountPointPreparationException(new NotDirectoryException(hideaway.toString())); + throw new HideawayNotDirectoryException(mountPoint, hideaway); } Files.delete(hideaway); //Fails if not empty } diff --git a/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java b/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java index 6e11ed131..8a4fbf304 100644 --- a/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java +++ b/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java @@ -1,6 +1,6 @@ package org.cryptomator.ui.unlock; -import org.cryptomator.common.mount.ExistingHideawayException; +import org.cryptomator.common.mount.HideawayNotDirectoryException; import org.cryptomator.common.mount.IllegalMountPointException; import org.cryptomator.common.mount.MountPointCleanupFailedException; import org.cryptomator.common.mount.MountPointInUseException; @@ -48,7 +48,7 @@ public class UnlockInvalidMountPointController implements FxController { this.exceptionType = getExceptionType(exc); this.exceptionPath = exc.getMountpoint(); this.exceptionMessage = exc.getMessage(); - this.hideawayPath = exc instanceof ExistingHideawayException haeExc ? haeExc.getHideaway() : null; + this.hideawayPath = exc instanceof HideawayNotDirectoryException haeExc ? haeExc.getHideaway() : null; } @FXML @@ -81,7 +81,7 @@ public class UnlockInvalidMountPointController implements FxController { case MountPointNotSupportedException x -> ExceptionType.NOT_SUPPORTED; case MountPointNotExistingException x -> ExceptionType.NOT_EXISTING; case MountPointInUseException x -> ExceptionType.IN_USE; - case ExistingHideawayException x -> ExceptionType.HIDEAWAY_EXISTS; + case HideawayNotDirectoryException x -> ExceptionType.HIDEAWAY_NOT_DIR; case MountPointCleanupFailedException x -> ExceptionType.COULD_NOT_BE_CLEARED; case MountPointNotEmptyDirectoryException x -> ExceptionType.NOT_EMPTY_DIRECTORY; default -> ExceptionType.GENERIC; @@ -93,7 +93,7 @@ public class UnlockInvalidMountPointController implements FxController { NOT_SUPPORTED("unlock.error.customPath.description.notSupported", ButtonAction.SHOW_PREFERENCES), NOT_EXISTING("unlock.error.customPath.description.notExists", ButtonAction.SHOW_VAULT_OPTIONS), IN_USE("unlock.error.customPath.description.inUse", ButtonAction.SHOW_VAULT_OPTIONS), - HIDEAWAY_EXISTS("unlock.error.customPath.description.hideawayExists", ButtonAction.SHOW_VAULT_OPTIONS), + HIDEAWAY_NOT_DIR("unlock.error.customPath.description.hideawayNotDir", ButtonAction.SHOW_VAULT_OPTIONS), COULD_NOT_BE_CLEARED("unlock.error.customPath.description.couldNotBeCleaned", ButtonAction.SHOW_VAULT_OPTIONS), NOT_EMPTY_DIRECTORY("unlock.error.customPath.description.notEmptyDir", ButtonAction.SHOW_VAULT_OPTIONS), GENERIC("unlock.error.customPath.description.generic", ButtonAction.SHOW_PREFERENCES); diff --git a/src/main/resources/i18n/strings.properties b/src/main/resources/i18n/strings.properties index d0d770329..dca30a2f9 100644 --- a/src/main/resources/i18n/strings.properties +++ b/src/main/resources/i18n/strings.properties @@ -131,7 +131,7 @@ unlock.error.customPath.message=Unable to mount vault to custom path unlock.error.customPath.description.notSupported=If you wish to keep using the custom path, please go to the preferences and select a volume type that supports it. Otherwise, go to the vault options and choose a supported mount point. unlock.error.customPath.description.notExists=The custom mount path does not exist. Either create it in your local filesystem or change it in the vault options. unlock.error.customPath.description.inUse=Drive letter "%s" is already in use. -unlock.error.customPath.description.hideawayExists=The folder "%3$s", which is used to preserve folder properties set by you, has not been automatically cleaned up since your last mount to "%1$s". Please delete it manually and restore any properties to the original mount path if you set any. +unlock.error.customPath.description.hideawayNotDir=The path "%3$s", which is used to preserve folder properties set by you for your mount path "%1$s", is not a folder. Please delete it manually and restore any properties to the original mount path if you set any. unlock.error.customPath.description.couldNotBeCleaned=Your vault could not be mounted to the path "%s". Please try again or choose a different path. unlock.error.customPath.description.notEmptyDir=The custom mount path "%s" is not an empty folder. Please choose an empty folder and try again. unlock.error.customPath.description.generic=You have selected a custom mount path for this vault, but using it failed with the message: %2$s From 3ea6da3c6d8234c819bf87ab2cbc1ea37c96320b Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Wed, 19 Jul 2023 14:04:15 +0200 Subject: [PATCH 19/33] Fixed thrown exception --- .../common/mount/MountWithinParentUtil.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index 38bc437de..11671e4af 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -30,7 +30,7 @@ public final class MountWithinParentUtil { if (!mpExists && !hideExists) { //neither mountpoint nor hideaway exist throw new MountPointNotExistingException(mountPoint); } else if (!mpExists) { //only hideaway exists - checkIsDirectory(hideaway); + checkIsHideawayDirectory(mountPoint, hideaway); LOG.info("Mountpoint {} seems to be not properly cleaned up. Will be fixed on unmount.", mountPoint); try { if (SystemUtils.IS_OS_WINDOWS) { @@ -86,9 +86,7 @@ public final class MountWithinParentUtil { } private static void removeResidualHideaway(Path mountPoint, Path hideaway) throws IOException { - if (!Files.isDirectory(hideaway, LinkOption.NOFOLLOW_LINKS)) { - throw new HideawayNotDirectoryException(mountPoint, hideaway); - } + checkIsHideawayDirectory(mountPoint, hideaway); Files.delete(hideaway); //Fails if not empty } @@ -134,6 +132,12 @@ public final class MountWithinParentUtil { } } + private static void checkIsHideawayDirectory(Path mountPoint, Path hideawayToCheck) { + if (!Files.isDirectory(hideawayToCheck, LinkOption.NOFOLLOW_LINKS)) { + throw new HideawayNotDirectoryException(mountPoint, hideawayToCheck); + } + } + private static void checkIsEmpty(Path toCheck) throws IllegalMountPointException, IOException { try (var dirStream = Files.list(toCheck)) { if (dirStream.findFirst().isPresent()) { From f9f8a6b3576acc254a8e58de2dda1214a31db8d1 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Wed, 19 Jul 2023 15:39:14 +0200 Subject: [PATCH 20/33] Added unit tests for MountWithinParentUtil --- .../common/mount/MountWithinParentUtil.java | 6 +- .../mount/MountWithinParentUtilTest.java | 208 ++++++++++++++++++ 2 files changed, 212 insertions(+), 2 deletions(-) create mode 100644 src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index 11671e4af..467ffa9cf 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -70,7 +70,8 @@ public final class MountWithinParentUtil { } } - private static boolean removeResidualJunction(Path path) { + //visible for testing + static boolean removeResidualJunction(Path path) { try { if (Files.readAttributes(path, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS).isOther()) { LOG.info("Mountpoint \"{}\" is still a junction. Deleting it.", path); @@ -85,7 +86,8 @@ public final class MountWithinParentUtil { } } - private static void removeResidualHideaway(Path mountPoint, Path hideaway) throws IOException { + //visible for testing + static void removeResidualHideaway(Path mountPoint, Path hideaway) throws IOException { checkIsHideawayDirectory(mountPoint, hideaway); Files.delete(hideaway); //Fails if not empty } diff --git a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java new file mode 100644 index 000000000..715c47be1 --- /dev/null +++ b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java @@ -0,0 +1,208 @@ +package org.cryptomator.common.mount; + +import org.apache.commons.lang3.SystemUtils; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledOnOs; +import org.junit.jupiter.api.condition.EnabledOnOs; +import org.junit.jupiter.api.condition.OS; +import org.junit.jupiter.api.io.TempDir; + +import java.io.File; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.DirectoryNotEmptyException; +import java.nio.file.Files; +import java.nio.file.Path; + +import static java.nio.file.LinkOption.NOFOLLOW_LINKS; +import static org.cryptomator.common.mount.MountWithinParentUtil.cleanup; +import static org.cryptomator.common.mount.MountWithinParentUtil.getHideaway; +import static org.cryptomator.common.mount.MountWithinParentUtil.prepareParentNoMountPoint; +import static org.cryptomator.common.mount.MountWithinParentUtil.removeResidualHideaway; +import static org.cryptomator.common.mount.MountWithinParentUtil.removeResidualJunction; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class MountWithinParentUtilTest { + + @Test + void testPrepareNeitherExist(@TempDir Path parentDir) { + assertThrows(MountPointNotExistingException.class, () -> { + prepareParentNoMountPoint(parentDir.resolve("mount")); + }); + } + + @Test + void testPrepareOnlyHideawayFileExists(@TempDir Path parentDir) throws IOException { + var mount = parentDir.resolve("mount"); + var hideaway = getHideaway(mount); + Files.createFile(hideaway); + + assertThrows(HideawayNotDirectoryException.class, () -> { + prepareParentNoMountPoint(mount); + }); + } + + @Test + void testPrepareOnlyHideawayDirExists(@TempDir Path parentDir) throws IOException { + var mount = parentDir.resolve("mount"); + var hideaway = getHideaway(mount); + Files.createDirectory(hideaway); + assertFalse(isHidden(hideaway)); + + prepareParentNoMountPoint(mount); + + assertTrue(!SystemUtils.IS_OS_WINDOWS || isHidden(hideaway)); + } + + @Test + void testPrepareBothExistHideawayFile(@TempDir Path parentDir) throws IOException { + var mount = parentDir.resolve("mount"); + var hideaway = getHideaway(mount); + Files.createFile(hideaway); + Files.createDirectory(mount); + + assertThrows(HideawayNotDirectoryException.class, () -> { + prepareParentNoMountPoint(mount); + }); + } + + @Test + void testPrepareBothExistHideawayNotEmpty(@TempDir Path parentDir) throws IOException { + var mount = parentDir.resolve("mount"); + var hideaway = getHideaway(mount); + Files.createDirectory(hideaway); + Files.createFile(hideaway.resolve("dummy")); + Files.createDirectory(mount); + + var exc = assertThrows(UncheckedIOException.class, () -> { + prepareParentNoMountPoint(mount); + }); + assertInstanceOf(DirectoryNotEmptyException.class, exc.getCause()); + } + + @Test + void testPrepareBothExistMountNotDir(@TempDir Path parentDir) throws IOException { + var mount = parentDir.resolve("mount"); + var hideaway = getHideaway(mount); + Files.createDirectory(hideaway); + Files.createFile(mount); + + assertThrows(MountPointNotEmptyDirectoryException.class, () -> { + prepareParentNoMountPoint(mount); + }); + assertTrue(Files.notExists(hideaway, NOFOLLOW_LINKS)); + } + + @Test + void testPrepareBothExistMountNotEmpty(@TempDir Path parentDir) throws IOException { + var mount = parentDir.resolve("mount"); + var hideaway = getHideaway(mount); + Files.createDirectory(hideaway); + Files.createDirectory(mount); + Files.createFile(mount.resolve("dummy")); + + assertThrows(MountPointNotEmptyDirectoryException.class, () -> { + prepareParentNoMountPoint(mount); + }); + assertTrue(Files.notExists(hideaway, NOFOLLOW_LINKS)); + } + + @Test + void testPrepareBothExist(@TempDir Path parentDir) throws IOException { + var mount = parentDir.resolve("mount"); + var hideaway = getHideaway(mount); + Files.createDirectory(hideaway); + Files.createDirectory(mount); + + prepareParentNoMountPoint(mount); + assertTrue(Files.notExists(mount, NOFOLLOW_LINKS)); + assertTrue(isHidden(hideaway)); + } + + @Test + void testRemoveResidualJunction(@TempDir Path parentDir) throws IOException { + //Sadly can't easily create files with "Other" attribute + var regularFile = parentDir.resolve("regularFile"); + Files.createFile(regularFile); + + assertTrue(removeResidualJunction(regularFile)); + assertFalse(removeResidualJunction(parentDir.resolve("notExisting"))); + } + + @Test + void testRemoveResidualHideawayFile(@TempDir Path parentDir) throws IOException { + var hideaway = parentDir.resolve("hideaway"); + Files.createFile(hideaway); + + assertThrows(HideawayNotDirectoryException.class, () -> removeResidualHideaway(parentDir.resolve("mount"), hideaway)); + } + + @Test + void testRemoveResidualHideawayNotEmpty(@TempDir Path parentDir) throws IOException { + var hideaway = parentDir.resolve("hideaway"); + Files.createDirectory(hideaway); + Files.createFile(hideaway.resolve("dummy")); + + assertThrows(DirectoryNotEmptyException.class, () -> removeResidualHideaway(parentDir.resolve("mount"), hideaway)); + } + + @Test + void testCleanupNoHideaway(@TempDir Path parentDir) { + assertDoesNotThrow(() -> cleanup(parentDir.resolve("mount"))); + } + + @Test + void testCleanup(@TempDir Path parentDir) throws IOException { + var mount = parentDir.resolve("mount"); + var hideaway = getHideaway(mount); + Files.createDirectory(hideaway); + + cleanup(mount); + assertTrue(Files.exists(mount, NOFOLLOW_LINKS)); + assertTrue(Files.notExists(hideaway, NOFOLLOW_LINKS)); + assertFalse(isHidden(mount)); + } + + @Test + @EnabledOnOs(OS.WINDOWS) + void testGetHideawayRootDirWin() { + var mount = Path.of("C:\\mount"); + var hideaway = getHideaway(mount); + + assertEquals(mount.getParent().toAbsolutePath(), Path.of("C:\\").toAbsolutePath()); + assertEquals(mount.getParent(), hideaway.getParent()); + assertEquals(mount.getParent().resolve(".~$mount.tmp"), hideaway); + assertEquals(mount.getParent().toAbsolutePath() + ".~$mount.tmp", hideaway.toAbsolutePath().toString()); + } + + @Test + @DisabledOnOs(OS.WINDOWS) + void testGetHideawayRootDirUnix() { + var mount = Path.of("/mount"); + var hideaway = getHideaway(mount); + + assertEquals(mount.getParent().toAbsolutePath(), Path.of("/").toAbsolutePath()); + assertEquals(mount.getParent(), hideaway.getParent()); + assertEquals(mount.getParent().resolve(".~$mount.tmp"), hideaway); + assertEquals(mount.getParent().toAbsolutePath() + ".~$mount.tmp", hideaway.toAbsolutePath().toString()); + } + + @Test + void testGetHideaway(@TempDir Path parentDir) { + var mount = parentDir.resolve("mount"); + var hideaway = getHideaway(mount); + + assertEquals(mount.getParent(), hideaway.getParent()); + assertEquals(mount.getParent().resolve(".~$mount.tmp"), hideaway); + assertEquals(mount.getParent().toAbsolutePath() + File.separator + ".~$mount.tmp", hideaway.toAbsolutePath().toString()); + } + + private static boolean isHidden(Path path) throws IOException { + return (boolean) Files.getAttribute(path, "dos:hidden", NOFOLLOW_LINKS); + } +} \ No newline at end of file From 879e6dcab7182b809a06896deeb0814f7d57a064 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Wed, 19 Jul 2023 16:04:22 +0200 Subject: [PATCH 21/33] Updated error message See: https://github.com/cryptomator/cryptomator/pull/3001#discussion_r1268035540 --- src/main/resources/i18n/strings.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/i18n/strings.properties b/src/main/resources/i18n/strings.properties index d0d770329..0683c8c16 100644 --- a/src/main/resources/i18n/strings.properties +++ b/src/main/resources/i18n/strings.properties @@ -131,7 +131,7 @@ unlock.error.customPath.message=Unable to mount vault to custom path unlock.error.customPath.description.notSupported=If you wish to keep using the custom path, please go to the preferences and select a volume type that supports it. Otherwise, go to the vault options and choose a supported mount point. unlock.error.customPath.description.notExists=The custom mount path does not exist. Either create it in your local filesystem or change it in the vault options. unlock.error.customPath.description.inUse=Drive letter "%s" is already in use. -unlock.error.customPath.description.hideawayExists=The folder "%3$s", which is used to preserve folder properties set by you, has not been automatically cleaned up since your last mount to "%1$s". Please delete it manually and restore any properties to the original mount path if you set any. +unlock.error.customPath.description.hideawayExists=The temporary, hidden directory "%3$s" used for unlock could not be removed. Please check the directory and remove it manually. unlock.error.customPath.description.couldNotBeCleaned=Your vault could not be mounted to the path "%s". Please try again or choose a different path. unlock.error.customPath.description.notEmptyDir=The custom mount path "%s" is not an empty folder. Please choose an empty folder and try again. unlock.error.customPath.description.generic=You have selected a custom mount path for this vault, but using it failed with the message: %2$s From a29ebfd302dd4521a43c30fcba83c4ef615e4d25 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Wed, 19 Jul 2023 23:39:11 +0200 Subject: [PATCH 22/33] Updated error message --- src/main/resources/i18n/strings.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/i18n/strings.properties b/src/main/resources/i18n/strings.properties index dca30a2f9..8f06f963e 100644 --- a/src/main/resources/i18n/strings.properties +++ b/src/main/resources/i18n/strings.properties @@ -131,7 +131,7 @@ unlock.error.customPath.message=Unable to mount vault to custom path unlock.error.customPath.description.notSupported=If you wish to keep using the custom path, please go to the preferences and select a volume type that supports it. Otherwise, go to the vault options and choose a supported mount point. unlock.error.customPath.description.notExists=The custom mount path does not exist. Either create it in your local filesystem or change it in the vault options. unlock.error.customPath.description.inUse=Drive letter "%s" is already in use. -unlock.error.customPath.description.hideawayNotDir=The path "%3$s", which is used to preserve folder properties set by you for your mount path "%1$s", is not a folder. Please delete it manually and restore any properties to the original mount path if you set any. +unlock.error.customPath.description.hideawayNotDir=The temporary, hidden file "%3$s" used for unlock could not be removed. Please check the file and then delete it manually. unlock.error.customPath.description.couldNotBeCleaned=Your vault could not be mounted to the path "%s". Please try again or choose a different path. unlock.error.customPath.description.notEmptyDir=The custom mount path "%s" is not an empty folder. Please choose an empty folder and try again. unlock.error.customPath.description.generic=You have selected a custom mount path for this vault, but using it failed with the message: %2$s From 419a7ab245a9e940cfed8d11fa604009a373d89a Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Wed, 19 Jul 2023 23:53:58 +0200 Subject: [PATCH 23/33] Fixed faulty unit test --- .../org/cryptomator/common/mount/MountWithinParentUtilTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java index 715c47be1..240ac2dd7 100644 --- a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java +++ b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java @@ -121,7 +121,7 @@ class MountWithinParentUtilTest { prepareParentNoMountPoint(mount); assertTrue(Files.notExists(mount, NOFOLLOW_LINKS)); - assertTrue(isHidden(hideaway)); + assertTrue(!SystemUtils.IS_OS_WINDOWS || isHidden(hideaway)); } @Test From 821cc0940de3170e4583aeebf17242e6511cfccf Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Thu, 20 Jul 2023 16:27:29 +0200 Subject: [PATCH 24/33] Stopped wrapping IOEs as UncheckedIOEs See: https://github.com/cryptomator/cryptomator/pull/2996#discussion_r1269444937 https://github.com/cryptomator/cryptomator/pull/2996#discussion_r1269445672 https://github.com/cryptomator/cryptomator/pull/2996#discussion_r1269445894 --- .../common/mount/MountWithinParentUtil.java | 17 ++++------------- .../common/mount/MountWithinParentUtilTest.java | 4 +--- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index 467ffa9cf..4a9717baa 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -5,7 +5,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.IOException; -import java.io.UncheckedIOException; import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.LinkOption; @@ -22,7 +21,7 @@ public final class MountWithinParentUtil { private MountWithinParentUtil() {} - static void prepareParentNoMountPoint(Path mountPoint) throws IllegalMountPointException { + static void prepareParentNoMountPoint(Path mountPoint) throws IllegalMountPointException, IOException { Path hideaway = getHideaway(mountPoint); var mpExists = removeResidualJunction(mountPoint); //Handle junction as not existing var hideExists = Files.exists(hideaway, LinkOption.NOFOLLOW_LINKS); @@ -32,12 +31,8 @@ public final class MountWithinParentUtil { } else if (!mpExists) { //only hideaway exists checkIsHideawayDirectory(mountPoint, hideaway); LOG.info("Mountpoint {} seems to be not properly cleaned up. Will be fixed on unmount.", mountPoint); - try { - if (SystemUtils.IS_OS_WINDOWS) { - Files.setAttribute(hideaway, WIN_HIDDEN_ATTR, true, LinkOption.NOFOLLOW_LINKS); - } - } catch (IOException e) { - throw new UncheckedIOException(e); + if (SystemUtils.IS_OS_WINDOWS) { + Files.setAttribute(hideaway, WIN_HIDDEN_ATTR, true, LinkOption.NOFOLLOW_LINKS); } } else { //mountpoint exists... try { @@ -61,8 +56,6 @@ public final class MountWithinParentUtil { Thread.sleep(1000); attempts++; } - } catch (IOException e) { - throw new UncheckedIOException(e); } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new RuntimeException(e); @@ -71,7 +64,7 @@ public final class MountWithinParentUtil { } //visible for testing - static boolean removeResidualJunction(Path path) { + static boolean removeResidualJunction(Path path) throws IOException { try { if (Files.readAttributes(path, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS).isOther()) { LOG.info("Mountpoint \"{}\" is still a junction. Deleting it.", path); @@ -81,8 +74,6 @@ public final class MountWithinParentUtil { return true; } catch (NoSuchFileException e) { return false; - } catch (IOException e) { - throw new UncheckedIOException(e); } } diff --git a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java index 240ac2dd7..99dba15c1 100644 --- a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java +++ b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java @@ -9,7 +9,6 @@ import org.junit.jupiter.api.io.TempDir; import java.io.File; import java.io.IOException; -import java.io.UncheckedIOException; import java.nio.file.DirectoryNotEmptyException; import java.nio.file.Files; import java.nio.file.Path; @@ -79,10 +78,9 @@ class MountWithinParentUtilTest { Files.createFile(hideaway.resolve("dummy")); Files.createDirectory(mount); - var exc = assertThrows(UncheckedIOException.class, () -> { + assertThrows(DirectoryNotEmptyException.class, () -> { prepareParentNoMountPoint(mount); }); - assertInstanceOf(DirectoryNotEmptyException.class, exc.getCause()); } @Test From 19428508880e71f9e7d77d5a4d548acc8f55e5d7 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Thu, 20 Jul 2023 16:44:24 +0200 Subject: [PATCH 25/33] Replaced OS-Check with Assumptions See: https://github.com/cryptomator/cryptomator/pull/2996#discussion_r1269461956 --- .../cryptomator/common/mount/MountWithinParentUtilTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java index 99dba15c1..bf637006c 100644 --- a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java +++ b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java @@ -25,6 +25,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; class MountWithinParentUtilTest { @@ -55,7 +56,7 @@ class MountWithinParentUtilTest { prepareParentNoMountPoint(mount); - assertTrue(!SystemUtils.IS_OS_WINDOWS || isHidden(hideaway)); + assumeTrue(isHidden(hideaway)); } @Test @@ -119,7 +120,7 @@ class MountWithinParentUtilTest { prepareParentNoMountPoint(mount); assertTrue(Files.notExists(mount, NOFOLLOW_LINKS)); - assertTrue(!SystemUtils.IS_OS_WINDOWS || isHidden(hideaway)); + assumeTrue(isHidden(hideaway)); } @Test From 8f674047660371e746d4a83714b44a7cd7f6b714 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Fri, 21 Jul 2023 15:35:56 +0200 Subject: [PATCH 26/33] Fixed method violating API contract See: https://github.com/cryptomator/cryptomator/pull/2996#discussion_r1269458822 --- .../common/mount/MountWithinParentUtilTest.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java index bf637006c..49a6e4049 100644 --- a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java +++ b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java @@ -1,6 +1,5 @@ package org.cryptomator.common.mount; -import org.apache.commons.lang3.SystemUtils; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledOnOs; import org.junit.jupiter.api.condition.EnabledOnOs; @@ -12,6 +11,7 @@ import java.io.IOException; import java.nio.file.DirectoryNotEmptyException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Objects; import static java.nio.file.LinkOption.NOFOLLOW_LINKS; import static org.cryptomator.common.mount.MountWithinParentUtil.cleanup; @@ -22,7 +22,6 @@ import static org.cryptomator.common.mount.MountWithinParentUtil.removeResidualJ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -202,6 +201,10 @@ class MountWithinParentUtilTest { } private static boolean isHidden(Path path) throws IOException { - return (boolean) Files.getAttribute(path, "dos:hidden", NOFOLLOW_LINKS); + try { + return (boolean) Objects.requireNonNullElse(Files.getAttribute(path, "dos:hidden", NOFOLLOW_LINKS), false); + } catch (UnsupportedOperationException | IllegalMountPointException e) { + return false; + } } } \ No newline at end of file From 59d89faf3858c4cc89a7fc08d3ebf2427c5a629f Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Fri, 21 Jul 2023 16:48:15 +0200 Subject: [PATCH 27/33] Turned calls to "assumeTrue" into guard functions See: https://github.com/cryptomator/cryptomator/pull/2996#discussion_r1270734307 --- .../common/mount/MountWithinParentUtilTest.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java index 49a6e4049..71da04dbe 100644 --- a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java +++ b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java @@ -1,5 +1,6 @@ package org.cryptomator.common.mount; +import org.apache.commons.lang3.SystemUtils; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledOnOs; import org.junit.jupiter.api.condition.EnabledOnOs; @@ -55,7 +56,8 @@ class MountWithinParentUtilTest { prepareParentNoMountPoint(mount); - assumeTrue(isHidden(hideaway)); + assumeTrue(SystemUtils.IS_OS_WINDOWS); + assertTrue(isHidden(hideaway)); } @Test @@ -119,7 +121,9 @@ class MountWithinParentUtilTest { prepareParentNoMountPoint(mount); assertTrue(Files.notExists(mount, NOFOLLOW_LINKS)); - assumeTrue(isHidden(hideaway)); + + assumeTrue(SystemUtils.IS_OS_WINDOWS); + assertTrue(isHidden(hideaway)); } @Test From 9bb24320bf5619c5e48ffd7c00e96f82086c19d0 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Mon, 24 Jul 2023 18:53:19 +0200 Subject: [PATCH 28/33] Stopped user from mounting to vaults to the same path --- .../common/mount/MountWithinParentUtil.java | 23 ++++++++++--------- src/main/resources/i18n/strings.properties | 2 +- .../mount/MountWithinParentUtilTest.java | 9 ++++---- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index 4a9717baa..2c476e20d 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -8,7 +8,6 @@ import java.io.IOException; import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.LinkOption; -import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.attribute.BasicFileAttributes; @@ -23,7 +22,7 @@ public final class MountWithinParentUtil { static void prepareParentNoMountPoint(Path mountPoint) throws IllegalMountPointException, IOException { Path hideaway = getHideaway(mountPoint); - var mpExists = removeResidualJunction(mountPoint); //Handle junction as not existing + var mpExists = handleMountPointFolder(mountPoint); //Handle residual (= broken) junction as not existing var hideExists = Files.exists(hideaway, LinkOption.NOFOLLOW_LINKS); if (!mpExists && !hideExists) { //neither mountpoint nor hideaway exist @@ -64,17 +63,19 @@ public final class MountWithinParentUtil { } //visible for testing - static boolean removeResidualJunction(Path path) throws IOException { - try { - if (Files.readAttributes(path, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS).isOther()) { - LOG.info("Mountpoint \"{}\" is still a junction. Deleting it.", path); - Files.delete(path); //Throws if path is also a non-empty folder - return false; - } - return true; - } catch (NoSuchFileException e) { + static boolean handleMountPointFolder(Path path) throws IOException { + if (Files.notExists(path, LinkOption.NOFOLLOW_LINKS)) { return false; } + if (!Files.readAttributes(path, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS).isOther()) { + return true; + } + if (Files.exists(path /* FOLLOW_LINKS */)) { //Both junction and target exist + throw new MountPointInUseException(path); + } + LOG.info("Mountpoint \"{}\" is still a junction. Deleting it.", path); + Files.delete(path); //Throws if path is also a non-empty folder + return false; } //visible for testing diff --git a/src/main/resources/i18n/strings.properties b/src/main/resources/i18n/strings.properties index 8f06f963e..4a3e64ee5 100644 --- a/src/main/resources/i18n/strings.properties +++ b/src/main/resources/i18n/strings.properties @@ -130,7 +130,7 @@ unlock.success.revealBtn=Reveal Drive unlock.error.customPath.message=Unable to mount vault to custom path unlock.error.customPath.description.notSupported=If you wish to keep using the custom path, please go to the preferences and select a volume type that supports it. Otherwise, go to the vault options and choose a supported mount point. unlock.error.customPath.description.notExists=The custom mount path does not exist. Either create it in your local filesystem or change it in the vault options. -unlock.error.customPath.description.inUse=Drive letter "%s" is already in use. +unlock.error.customPath.description.inUse=The custom mount path "%s" is already in use. unlock.error.customPath.description.hideawayNotDir=The temporary, hidden file "%3$s" used for unlock could not be removed. Please check the file and then delete it manually. unlock.error.customPath.description.couldNotBeCleaned=Your vault could not be mounted to the path "%s". Please try again or choose a different path. unlock.error.customPath.description.notEmptyDir=The custom mount path "%s" is not an empty folder. Please choose an empty folder and try again. diff --git a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java index 71da04dbe..09648a98e 100644 --- a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java +++ b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java @@ -17,9 +17,9 @@ import java.util.Objects; import static java.nio.file.LinkOption.NOFOLLOW_LINKS; import static org.cryptomator.common.mount.MountWithinParentUtil.cleanup; import static org.cryptomator.common.mount.MountWithinParentUtil.getHideaway; +import static org.cryptomator.common.mount.MountWithinParentUtil.handleMountPointFolder; import static org.cryptomator.common.mount.MountWithinParentUtil.prepareParentNoMountPoint; import static org.cryptomator.common.mount.MountWithinParentUtil.removeResidualHideaway; -import static org.cryptomator.common.mount.MountWithinParentUtil.removeResidualJunction; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -127,13 +127,14 @@ class MountWithinParentUtilTest { } @Test - void testRemoveResidualJunction(@TempDir Path parentDir) throws IOException { + void testHandleMountPointFolder(@TempDir Path parentDir) throws IOException { //Sadly can't easily create files with "Other" attribute var regularFile = parentDir.resolve("regularFile"); Files.createFile(regularFile); - assertTrue(removeResidualJunction(regularFile)); - assertFalse(removeResidualJunction(parentDir.resolve("notExisting"))); + assertTrue(handleMountPointFolder(regularFile)); + assertTrue(Files.exists(regularFile)); + assertFalse(handleMountPointFolder(parentDir.resolve("notExisting"))); } @Test From ea8e850aa9226fa57cd2325c43aae96603d88390 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Mon, 24 Jul 2023 19:15:22 +0200 Subject: [PATCH 29/33] Moved check for dir/emptiness to "handleMountPointFolder" --- .../common/mount/MountWithinParentUtil.java | 5 ++--- .../common/mount/MountWithinParentUtilTest.java | 12 ++++++------ 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index 2c476e20d..f29186865 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -40,9 +40,6 @@ public final class MountWithinParentUtil { } //... (now) without hideaway - checkIsDirectory(mountPoint); - checkIsEmpty(mountPoint); - Files.move(mountPoint, hideaway); if (SystemUtils.IS_OS_WINDOWS) { Files.setAttribute(hideaway, WIN_HIDDEN_ATTR, true, LinkOption.NOFOLLOW_LINKS); @@ -68,6 +65,8 @@ public final class MountWithinParentUtil { return false; } if (!Files.readAttributes(path, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS).isOther()) { + checkIsDirectory(path); + checkIsEmpty(path); return true; } if (Files.exists(path /* FOLLOW_LINKS */)) { //Both junction and target exist diff --git a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java index 09648a98e..bfdda8fa0 100644 --- a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java +++ b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java @@ -89,27 +89,27 @@ class MountWithinParentUtilTest { void testPrepareBothExistMountNotDir(@TempDir Path parentDir) throws IOException { var mount = parentDir.resolve("mount"); var hideaway = getHideaway(mount); - Files.createDirectory(hideaway); + Files.createFile(hideaway); Files.createFile(mount); assertThrows(MountPointNotEmptyDirectoryException.class, () -> { - prepareParentNoMountPoint(mount); + prepareParentNoMountPoint(mount); //Must not throw something related to hideaway }); - assertTrue(Files.notExists(hideaway, NOFOLLOW_LINKS)); + assertTrue(Files.exists(hideaway, NOFOLLOW_LINKS)); } @Test void testPrepareBothExistMountNotEmpty(@TempDir Path parentDir) throws IOException { var mount = parentDir.resolve("mount"); var hideaway = getHideaway(mount); - Files.createDirectory(hideaway); + Files.createFile(hideaway); Files.createDirectory(mount); Files.createFile(mount.resolve("dummy")); assertThrows(MountPointNotEmptyDirectoryException.class, () -> { - prepareParentNoMountPoint(mount); + prepareParentNoMountPoint(mount); //Must not throw something related to hideaway }); - assertTrue(Files.notExists(hideaway, NOFOLLOW_LINKS)); + assertTrue(Files.exists(hideaway, NOFOLLOW_LINKS)); } @Test From 587cff9518bdf2659a817aa5a40b61852f2b6986 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Mon, 24 Jul 2023 19:22:47 +0200 Subject: [PATCH 30/33] Added more tests --- .../mount/MountWithinParentUtilTest.java | 37 +++++++++++++++++-- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java index bfdda8fa0..2a63d3af8 100644 --- a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java +++ b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java @@ -127,14 +127,43 @@ class MountWithinParentUtilTest { } @Test - void testHandleMountPointFolder(@TempDir Path parentDir) throws IOException { - //Sadly can't easily create files with "Other" attribute + void testHandleMountPointFolderDoesNotExist(@TempDir Path parentDir) throws IOException { + assertFalse(handleMountPointFolder(parentDir.resolve("notExisting"))); + } + + @Test + void testHandleMountPointFolderIsFile(@TempDir Path parentDir) throws IOException { var regularFile = parentDir.resolve("regularFile"); Files.createFile(regularFile); - assertTrue(handleMountPointFolder(regularFile)); + assertThrows(MountPointNotEmptyDirectoryException.class, () -> { + handleMountPointFolder(regularFile); + }); assertTrue(Files.exists(regularFile)); - assertFalse(handleMountPointFolder(parentDir.resolve("notExisting"))); + } + + @Test + void testHandleMountPointFolderIsNotEmpty(@TempDir Path parentDir) throws IOException { + var regularFolder = parentDir.resolve("regularFolder"); + var dummyFile = regularFolder.resolve("dummy"); + Files.createDirectory(regularFolder); + Files.createFile(dummyFile); + + assertThrows(MountPointNotEmptyDirectoryException.class, () -> { + handleMountPointFolder(regularFolder); + }); + assertTrue(Files.exists(regularFolder)); + assertTrue(Files.exists(dummyFile)); + } + + @Test + void testHandleMountPointFolder(@TempDir Path parentDir) throws IOException { + //Sadly can't easily create files with "Other" attribute + var regularFolder = parentDir.resolve("regularFolder"); + Files.createDirectory(regularFolder); + + assertTrue(handleMountPointFolder(regularFolder)); + assertTrue(Files.exists(regularFolder)); } @Test From 1c34402c87077252871cea2cbf4ec5f546adcb4d Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Tue, 25 Jul 2023 13:17:21 +0200 Subject: [PATCH 31/33] Applied minor corrections See: https://github.com/cryptomator/cryptomator/pull/2996#discussion_r1273277251 https://github.com/cryptomator/cryptomator/pull/2996#discussion_r1273280687 https://github.com/cryptomator/cryptomator/pull/2996#discussion_r1273287847 --- .../cryptomator/common/mount/MountWithinParentUtil.java | 8 ++++---- src/main/resources/i18n/strings.properties | 2 +- .../common/mount/MountWithinParentUtilTest.java | 2 -- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index f29186865..92023279c 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -65,8 +65,8 @@ public final class MountWithinParentUtil { return false; } if (!Files.readAttributes(path, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS).isOther()) { - checkIsDirectory(path); - checkIsEmpty(path); + checkIsMountPointDirectory(path); + checkIsMountPointEmpty(path); return true; } if (Files.exists(path /* FOLLOW_LINKS */)) { //Both junction and target exist @@ -119,7 +119,7 @@ public final class MountWithinParentUtil { } } - private static void checkIsDirectory(Path toCheck) throws IllegalMountPointException { + private static void checkIsMountPointDirectory(Path toCheck) throws IllegalMountPointException { if (!Files.isDirectory(toCheck, LinkOption.NOFOLLOW_LINKS)) { throw new MountPointNotEmptyDirectoryException(toCheck, "Mountpoint is not a directory: " + toCheck); } @@ -131,7 +131,7 @@ public final class MountWithinParentUtil { } } - private static void checkIsEmpty(Path toCheck) throws IllegalMountPointException, IOException { + private static void checkIsMountPointEmpty(Path toCheck) throws IllegalMountPointException, IOException { try (var dirStream = Files.list(toCheck)) { if (dirStream.findFirst().isPresent()) { throw new MountPointNotEmptyDirectoryException(toCheck, "Mountpoint directory is not empty: " + toCheck); diff --git a/src/main/resources/i18n/strings.properties b/src/main/resources/i18n/strings.properties index 4a3e64ee5..69a22855d 100644 --- a/src/main/resources/i18n/strings.properties +++ b/src/main/resources/i18n/strings.properties @@ -130,7 +130,7 @@ unlock.success.revealBtn=Reveal Drive unlock.error.customPath.message=Unable to mount vault to custom path unlock.error.customPath.description.notSupported=If you wish to keep using the custom path, please go to the preferences and select a volume type that supports it. Otherwise, go to the vault options and choose a supported mount point. unlock.error.customPath.description.notExists=The custom mount path does not exist. Either create it in your local filesystem or change it in the vault options. -unlock.error.customPath.description.inUse=The custom mount path "%s" is already in use. +unlock.error.customPath.description.inUse=The drive letter or custom mount path "%s" is already in use. unlock.error.customPath.description.hideawayNotDir=The temporary, hidden file "%3$s" used for unlock could not be removed. Please check the file and then delete it manually. unlock.error.customPath.description.couldNotBeCleaned=Your vault could not be mounted to the path "%s". Please try again or choose a different path. unlock.error.customPath.description.notEmptyDir=The custom mount path "%s" is not an empty folder. Please choose an empty folder and try again. diff --git a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java index 2a63d3af8..b9d0820a9 100644 --- a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java +++ b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java @@ -206,7 +206,6 @@ class MountWithinParentUtilTest { var mount = Path.of("C:\\mount"); var hideaway = getHideaway(mount); - assertEquals(mount.getParent().toAbsolutePath(), Path.of("C:\\").toAbsolutePath()); assertEquals(mount.getParent(), hideaway.getParent()); assertEquals(mount.getParent().resolve(".~$mount.tmp"), hideaway); assertEquals(mount.getParent().toAbsolutePath() + ".~$mount.tmp", hideaway.toAbsolutePath().toString()); @@ -218,7 +217,6 @@ class MountWithinParentUtilTest { var mount = Path.of("/mount"); var hideaway = getHideaway(mount); - assertEquals(mount.getParent().toAbsolutePath(), Path.of("/").toAbsolutePath()); assertEquals(mount.getParent(), hideaway.getParent()); assertEquals(mount.getParent().resolve(".~$mount.tmp"), hideaway); assertEquals(mount.getParent().toAbsolutePath() + ".~$mount.tmp", hideaway.toAbsolutePath().toString()); From ae392b40142d3666aa458c2f6afbf7fd7e85b716 Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Tue, 25 Jul 2023 13:34:39 +0200 Subject: [PATCH 32/33] Refactored "handleMountPointFolder" (now: "getMountPointState") See: https://github.com/cryptomator/cryptomator/pull/2996#discussion_r1273277804 --- .../common/mount/MountWithinParentUtil.java | 36 +++++++++++++------ .../mount/MountWithinParentUtilTest.java | 17 +++++---- 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index 92023279c..39d6a9479 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -22,18 +22,25 @@ public final class MountWithinParentUtil { static void prepareParentNoMountPoint(Path mountPoint) throws IllegalMountPointException, IOException { Path hideaway = getHideaway(mountPoint); - var mpExists = handleMountPointFolder(mountPoint); //Handle residual (= broken) junction as not existing + var mpState = getMountPointState(mountPoint); var hideExists = Files.exists(hideaway, LinkOption.NOFOLLOW_LINKS); - if (!mpExists && !hideExists) { //neither mountpoint nor hideaway exist + if (mpState == MountPointState.JUNCTION) { + LOG.info("Mountpoint \"{}\" is still a junction. Deleting it.", mountPoint); + Files.delete(mountPoint); //Throws if mountPoint is also a non-empty folder + mpState = MountPointState.NOT_EXISTING; + } + + if (mpState == MountPointState.NOT_EXISTING && !hideExists) { //neither mountpoint nor hideaway exist throw new MountPointNotExistingException(mountPoint); - } else if (!mpExists) { //only hideaway exists + } else if (mpState == MountPointState.NOT_EXISTING) { //only hideaway exists checkIsHideawayDirectory(mountPoint, hideaway); LOG.info("Mountpoint {} seems to be not properly cleaned up. Will be fixed on unmount.", mountPoint); if (SystemUtils.IS_OS_WINDOWS) { Files.setAttribute(hideaway, WIN_HIDDEN_ATTR, true, LinkOption.NOFOLLOW_LINKS); } - } else { //mountpoint exists... + } else { + assert mpState == MountPointState.EMPTY_DIR; try { if (hideExists) { //... with hideaway removeResidualHideaway(mountPoint, hideaway); @@ -60,21 +67,30 @@ public final class MountWithinParentUtil { } //visible for testing - static boolean handleMountPointFolder(Path path) throws IOException { + static MountPointState getMountPointState(Path path) throws IOException { if (Files.notExists(path, LinkOption.NOFOLLOW_LINKS)) { - return false; + return MountPointState.NOT_EXISTING; } if (!Files.readAttributes(path, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS).isOther()) { checkIsMountPointDirectory(path); checkIsMountPointEmpty(path); - return true; + return MountPointState.EMPTY_DIR; } if (Files.exists(path /* FOLLOW_LINKS */)) { //Both junction and target exist throw new MountPointInUseException(path); } - LOG.info("Mountpoint \"{}\" is still a junction. Deleting it.", path); - Files.delete(path); //Throws if path is also a non-empty folder - return false; + return MountPointState.JUNCTION; + } + + //visible for testing + enum MountPointState { + + NOT_EXISTING, + + EMPTY_DIR, + + JUNCTION; + } //visible for testing diff --git a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java index b9d0820a9..aad33410b 100644 --- a/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java +++ b/src/test/java/org/cryptomator/common/mount/MountWithinParentUtilTest.java @@ -15,14 +15,17 @@ import java.nio.file.Path; import java.util.Objects; import static java.nio.file.LinkOption.NOFOLLOW_LINKS; +import static org.cryptomator.common.mount.MountWithinParentUtil.MountPointState.EMPTY_DIR; +import static org.cryptomator.common.mount.MountWithinParentUtil.MountPointState.NOT_EXISTING; import static org.cryptomator.common.mount.MountWithinParentUtil.cleanup; import static org.cryptomator.common.mount.MountWithinParentUtil.getHideaway; -import static org.cryptomator.common.mount.MountWithinParentUtil.handleMountPointFolder; +import static org.cryptomator.common.mount.MountWithinParentUtil.getMountPointState; import static org.cryptomator.common.mount.MountWithinParentUtil.prepareParentNoMountPoint; import static org.cryptomator.common.mount.MountWithinParentUtil.removeResidualHideaway; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -128,7 +131,7 @@ class MountWithinParentUtilTest { @Test void testHandleMountPointFolderDoesNotExist(@TempDir Path parentDir) throws IOException { - assertFalse(handleMountPointFolder(parentDir.resolve("notExisting"))); + assertSame(getMountPointState(parentDir.resolve("notExisting")), NOT_EXISTING); } @Test @@ -137,9 +140,8 @@ class MountWithinParentUtilTest { Files.createFile(regularFile); assertThrows(MountPointNotEmptyDirectoryException.class, () -> { - handleMountPointFolder(regularFile); + getMountPointState(regularFile); }); - assertTrue(Files.exists(regularFile)); } @Test @@ -150,10 +152,8 @@ class MountWithinParentUtilTest { Files.createFile(dummyFile); assertThrows(MountPointNotEmptyDirectoryException.class, () -> { - handleMountPointFolder(regularFolder); + getMountPointState(regularFolder); }); - assertTrue(Files.exists(regularFolder)); - assertTrue(Files.exists(dummyFile)); } @Test @@ -162,8 +162,7 @@ class MountWithinParentUtilTest { var regularFolder = parentDir.resolve("regularFolder"); Files.createDirectory(regularFolder); - assertTrue(handleMountPointFolder(regularFolder)); - assertTrue(Files.exists(regularFolder)); + assertSame(getMountPointState(regularFolder), EMPTY_DIR); } @Test From b536bd3e0909d26553a2e754a084823b54b41d0c Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Tue, 25 Jul 2023 15:33:16 +0200 Subject: [PATCH 33/33] Applied suggestions from code review Added exception to method signature Renamed enum constant See: https://github.com/cryptomator/cryptomator/pull/2996#discussion_r1273497818 https://github.com/cryptomator/cryptomator/pull/2996#discussion_r1273499227 --- .../cryptomator/common/mount/MountWithinParentUtil.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java index 39d6a9479..b632923a8 100644 --- a/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java +++ b/src/main/java/org/cryptomator/common/mount/MountWithinParentUtil.java @@ -25,7 +25,7 @@ public final class MountWithinParentUtil { var mpState = getMountPointState(mountPoint); var hideExists = Files.exists(hideaway, LinkOption.NOFOLLOW_LINKS); - if (mpState == MountPointState.JUNCTION) { + if (mpState == MountPointState.BROKEN_JUNCTION) { LOG.info("Mountpoint \"{}\" is still a junction. Deleting it.", mountPoint); Files.delete(mountPoint); //Throws if mountPoint is also a non-empty folder mpState = MountPointState.NOT_EXISTING; @@ -67,7 +67,7 @@ public final class MountWithinParentUtil { } //visible for testing - static MountPointState getMountPointState(Path path) throws IOException { + static MountPointState getMountPointState(Path path) throws IOException, IllegalMountPointException { if (Files.notExists(path, LinkOption.NOFOLLOW_LINKS)) { return MountPointState.NOT_EXISTING; } @@ -79,7 +79,7 @@ public final class MountWithinParentUtil { if (Files.exists(path /* FOLLOW_LINKS */)) { //Both junction and target exist throw new MountPointInUseException(path); } - return MountPointState.JUNCTION; + return MountPointState.BROKEN_JUNCTION; } //visible for testing @@ -89,7 +89,7 @@ public final class MountWithinParentUtil { EMPTY_DIR, - JUNCTION; + BROKEN_JUNCTION; }