From aa22f656e8c78e67c5726dc218a38657bc939cab Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Mon, 2 Nov 2020 17:34:34 +0100 Subject: [PATCH 01/18] Moving the code for unregular unmount leftovers from mountpoint chooser to an own class and execute it at each application start. --- .../mountpoint/UnregularUnmountCleaner.java | 60 +++++++++++++++++++ .../cryptomator/ui/launcher/UiLauncher.java | 10 +++- 2 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 main/commons/src/main/java/org/cryptomator/common/mountpoint/UnregularUnmountCleaner.java diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/UnregularUnmountCleaner.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/UnregularUnmountCleaner.java new file mode 100644 index 000000000..3229f909c --- /dev/null +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/UnregularUnmountCleaner.java @@ -0,0 +1,60 @@ +package org.cryptomator.common.mountpoint; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +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; +import java.util.ArrayList; +import java.util.Collection; + +public class UnregularUnmountCleaner { + + public static Logger LOG = LoggerFactory.getLogger(UnregularUnmountCleaner.class); + + public static void removeUnregularUnmountDebris(Path dirOfMountPoints) { + Collection exceptionCatcher = new ArrayList<>(); + try { + LOG.debug("Performing cleanup of mountpoint dir {}.", dirOfMountPoints); + Files.list(dirOfMountPoints).filter(p -> { + if (Files.isDirectory(p, LinkOption.NOFOLLOW_LINKS)) { + return true; + } else { + LOG.debug("Found non-directory element in mountpoint dir: {}", p); + return false; + } + }).forEach(dir -> { + try { + try { + Files.readAttributes(dir, BasicFileAttributes.class); //we follow the link and see if it exists + } catch (NoSuchFileException e) { + Files.delete(dir); //broken link file, we delete it + LOG.debug("Removed broken entry {} from mountpoint dir."); + return; + } + //check if dir is empty + if (Files.list(dir).findFirst().isEmpty()) { + Files.delete(dir); + LOG.debug("Removed empty dir {} from mountpoint dir."); + } else { + LOG.debug("Found non-empty directory in mountpoint dir: {}", dir); + } + } catch (IOException e) { + exceptionCatcher.add(e); + } + }); + + exceptionCatcher.forEach(exception -> { + LOG.debug("Unable to clean element from mountpoint dir due to", exception); + }); + } catch (IOException e) { + LOG.debug("Unable to perform cleanup of mountpoint dir {}.", dirOfMountPoints, e); + } + + } + +} diff --git a/main/ui/src/main/java/org/cryptomator/ui/launcher/UiLauncher.java b/main/ui/src/main/java/org/cryptomator/ui/launcher/UiLauncher.java index ad98fe226..9b9e7ee05 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/launcher/UiLauncher.java +++ b/main/ui/src/main/java/org/cryptomator/ui/launcher/UiLauncher.java @@ -1,5 +1,7 @@ package org.cryptomator.ui.launcher; +import org.cryptomator.common.Environment; +import org.cryptomator.common.mountpoint.UnregularUnmountCleaner; import org.cryptomator.common.settings.Settings; import org.cryptomator.common.vaults.Vault; import org.cryptomator.jni.JniException; @@ -30,15 +32,17 @@ public class UiLauncher { private final FxApplicationStarter fxApplicationStarter; private final AppLaunchEventHandler launchEventHandler; private final Optional macFunctions; + private final Environment env; @Inject - public UiLauncher(Settings settings, ObservableList vaults, TrayMenuComponent.Builder trayComponent, FxApplicationStarter fxApplicationStarter, AppLaunchEventHandler launchEventHandler, Optional macFunctions) { + public UiLauncher(Settings settings, ObservableList vaults, TrayMenuComponent.Builder trayComponent, FxApplicationStarter fxApplicationStarter, AppLaunchEventHandler launchEventHandler, Optional macFunctions, Environment env) { this.settings = settings; this.vaults = vaults; this.trayComponent = trayComponent; this.fxApplicationStarter = fxApplicationStarter; this.launchEventHandler = launchEventHandler; this.macFunctions = macFunctions; + this.env = env; } public void launch() { @@ -61,6 +65,10 @@ public class UiLauncher { // register app reopen listener Desktop.getDesktop().addAppEventListener((AppReopenedListener) e -> showMainWindowAsync(hasTrayIcon)); + //clean leftovers of not-regularly unmounted vaults + //see https://github.com/cryptomator/cryptomator/issues/1013 and https://github.com/cryptomator/cryptomator/issues/1061 + env.getMountPointsDir().ifPresent(UnregularUnmountCleaner::removeUnregularUnmountDebris); + // auto unlock Collection vaultsToAutoUnlock = vaults.filtered(this::shouldAttemptAutoUnlock); if (!vaultsToAutoUnlock.isEmpty()) { From 268bae88792c771c6bb99ef75deb2e9bf45c5581 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 3 Nov 2020 12:13:22 +0100 Subject: [PATCH 02/18] Change Invalid mount point error window: * Add heading * change icon to times_with_circle * reformat text --- .../org/cryptomator/ui/controls/FontAwesome5Icon.java | 1 + .../main/resources/fxml/unlock_invalid_mount_point.fxml | 5 +++-- main/ui/src/main/resources/i18n/strings.properties | 8 +++++--- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/main/ui/src/main/java/org/cryptomator/ui/controls/FontAwesome5Icon.java b/main/ui/src/main/java/org/cryptomator/ui/controls/FontAwesome5Icon.java index f4d0d058a..0a199fb48 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/controls/FontAwesome5Icon.java +++ b/main/ui/src/main/java/org/cryptomator/ui/controls/FontAwesome5Icon.java @@ -39,6 +39,7 @@ public enum FontAwesome5Icon { SPINNER("\uF110"), // SYNC("\uF021"), // TIMES("\uF00D"), // + TIMES_CIRCLE("\uF057"), // TRASH("\uF1F8"), // UNLINK("\uf127"), // WRENCH("\uF0AD"), // diff --git a/main/ui/src/main/resources/fxml/unlock_invalid_mount_point.fxml b/main/ui/src/main/resources/fxml/unlock_invalid_mount_point.fxml index 707bef240..9d56a32e7 100644 --- a/main/ui/src/main/resources/fxml/unlock_invalid_mount_point.fxml +++ b/main/ui/src/main/resources/fxml/unlock_invalid_mount_point.fxml @@ -10,6 +10,7 @@ + - - + + diff --git a/main/ui/src/main/resources/i18n/strings.properties b/main/ui/src/main/resources/i18n/strings.properties index 3805399aa..0767db7e5 100644 --- a/main/ui/src/main/resources/i18n/strings.properties +++ b/main/ui/src/main/resources/i18n/strings.properties @@ -102,9 +102,11 @@ unlock.unlockBtn=Unlock unlock.success.message=Unlocked "%s" successfully! Your vault is now accessible. unlock.success.rememberChoice=Remember choice, don't show this again unlock.success.revealBtn=Reveal Vault -## Invalid Mount Point -unlock.error.invalidMountPoint.notExisting=Mount point is not an empty directory or doesn't exist: %s -unlock.error.invalidMountPoint.existing=Mount point/folder already exists or parent folder is missing: %s +## Failure +unlock.error.heading=Unable to unlock vault +### Invalid Mount Point +unlock.error.invalidMountPoint.notExisting=Mount point "%s" is not a directory, not empty or does not exist. +unlock.error.invalidMountPoint.existing=Mount point "%s" already exists or parent folder is missing. # Migration migration.title=Upgrade Vault From 1554437884b0019a5b147c45215a568d9f18f121 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 3 Nov 2020 12:44:09 +0100 Subject: [PATCH 03/18] Use regular display name and only normalize if webdav is used: * rename StringBinding mountName to normalizedDisplayName * replace in all other occurences mountName with DisplayName --- .../common/mountpoint/TemporaryMountPointChooser.java | 2 +- .../org/cryptomator/common/settings/VaultSettings.java | 8 ++++---- .../java/org/cryptomator/common/vaults/DokanyVolume.java | 2 +- .../java/org/cryptomator/common/vaults/VaultModule.java | 7 ++++--- .../java/org/cryptomator/common/vaults/WebDavVolume.java | 2 +- .../org/cryptomator/common/vaults/VaultModuleTest.java | 2 +- 6 files changed, 12 insertions(+), 11 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java index 60991b97e..4c96c0599 100644 --- a/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java @@ -46,7 +46,7 @@ public class TemporaryMountPointChooser implements MountPointChooser { } private Path choose(Path parent) { - String basename = this.vaultSettings.mountName().get(); + String basename = this.vaultSettings.displayName().get(); for (int i = 0; i < MAX_TMPMOUNTPOINT_CREATION_RETRIES; i++) { Path mountPoint = parent.resolve(basename + "_" + i); if (Files.notExists(mountPoint)) { diff --git a/main/commons/src/main/java/org/cryptomator/common/settings/VaultSettings.java b/main/commons/src/main/java/org/cryptomator/common/settings/VaultSettings.java index c99da6325..7bcb2d4fc 100644 --- a/main/commons/src/main/java/org/cryptomator/common/settings/VaultSettings.java +++ b/main/commons/src/main/java/org/cryptomator/common/settings/VaultSettings.java @@ -53,11 +53,11 @@ public class VaultSettings { private final IntegerProperty filenameLengthLimit = new SimpleIntegerProperty(DEFAULT_FILENAME_LENGTH_LIMIT); private final ObjectProperty actionAfterUnlock = new SimpleObjectProperty<>(DEFAULT_ACTION_AFTER_UNLOCK); - private final StringBinding mountName; + private final StringBinding normalizedDisplayName; public VaultSettings(String id) { this.id = Objects.requireNonNull(id); - this.mountName = Bindings.createStringBinding(this::normalizeDisplayName, displayName); + this.normalizedDisplayName = Bindings.createStringBinding(this::normalizeDisplayName, displayName); } Observable[] observables() { @@ -108,8 +108,8 @@ public class VaultSettings { return displayName; } - public StringBinding mountName() { - return mountName; + public StringBinding normalizedDisplayName() { + return normalizedDisplayName; } public StringProperty winDriveLetter() { diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java index 644604b4b..20d840bcc 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java @@ -44,7 +44,7 @@ public class DokanyVolume extends AbstractVolume { this.mountPoint = determineMountPoint(); String mountName = vaultSettings.displayName().get(); try { - this.mount = mountFactory.mount(fs.getPath("/"), mountPoint, vaultSettings.mountName().get(), FS_TYPE_NAME, mountFlags.strip()); + this.mount = mountFactory.mount(fs.getPath("/"), mountPoint, vaultSettings.displayName().get(), FS_TYPE_NAME, mountFlags.strip()); } catch (MountFailedException e) { if (vaultSettings.getCustomMountPath().isPresent()) { LOG.warn("Failed to mount vault into {}. Is this directory currently accessed by another process (e.g. Windows Explorer)?", mountPoint); diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java index 4844143a3..4fa04ca49 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java @@ -23,6 +23,7 @@ import javafx.beans.property.BooleanProperty; import javafx.beans.property.ObjectProperty; import javafx.beans.property.ReadOnlyBooleanProperty; import javafx.beans.property.SimpleObjectProperty; +import javafx.beans.property.StringProperty; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -75,7 +76,7 @@ public class VaultModule { @DefaultMountFlags public StringBinding provideDefaultMountFlags(Settings settings, VaultSettings vaultSettings) { ObjectProperty preferredVolumeImpl = settings.preferredVolumeImpl(); - StringBinding mountName = vaultSettings.mountName(); + StringProperty mountName = vaultSettings.displayName(); BooleanProperty readOnly = vaultSettings.usesReadOnlyMode(); return Bindings.createStringBinding(() -> { @@ -95,7 +96,7 @@ public class VaultModule { } // see: https://github.com/osxfuse/osxfuse/wiki/Mount-options - private String getMacFuseDefaultMountFlags(StringBinding mountName, ReadOnlyBooleanProperty readOnly) { + private String getMacFuseDefaultMountFlags(StringProperty mountName, ReadOnlyBooleanProperty readOnly) { assert SystemUtils.IS_OS_MAC_OSX; StringBuilder flags = new StringBuilder(); if (readOnly.get()) { @@ -147,7 +148,7 @@ public class VaultModule { // see https://github.com/billziss-gh/winfsp/blob/5d0b10d0b643652c00ebb4704dc2bb28e7244973/src/dll/fuse/fuse_main.c#L53-L62 for syntax guide // see https://github.com/billziss-gh/winfsp/blob/5d0b10d0b643652c00ebb4704dc2bb28e7244973/src/dll/fuse/fuse.c#L295-L319 for options (-o <...>) // see https://github.com/billziss-gh/winfsp/wiki/Frequently-Asked-Questions/5ba00e4be4f5e938eaae6ef1500b331de12dee77 (FUSE 4.) on why the given defaults were choosen - private String getWindowsFuseDefaultMountFlags(StringBinding mountName, ReadOnlyBooleanProperty readOnly) { + private String getWindowsFuseDefaultMountFlags(StringProperty mountName, ReadOnlyBooleanProperty readOnly) { assert SystemUtils.IS_OS_WINDOWS; StringBuilder flags = new StringBuilder(); diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java index bf42e353f..aa40ef57b 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java @@ -45,7 +45,7 @@ public class WebDavVolume implements Volume { if (!server.isRunning()) { server.start(); } - servlet = server.createWebDavServlet(fs.getPath("/"), vaultSettings.getId() + "/" + vaultSettings.mountName().get()); + servlet = server.createWebDavServlet(fs.getPath("/"), vaultSettings.getId() + "/" + vaultSettings.normalizedDisplayName().get()); servlet.start(); mount(); } diff --git a/main/commons/src/test/java/org/cryptomator/common/vaults/VaultModuleTest.java b/main/commons/src/test/java/org/cryptomator/common/vaults/VaultModuleTest.java index 907e88623..81c821eb8 100644 --- a/main/commons/src/test/java/org/cryptomator/common/vaults/VaultModuleTest.java +++ b/main/commons/src/test/java/org/cryptomator/common/vaults/VaultModuleTest.java @@ -28,7 +28,7 @@ public class VaultModuleTest { @BeforeEach public void setup(@TempDir Path tmpDir) { - Mockito.when(vaultSettings.mountName()).thenReturn(Bindings.createStringBinding(() -> "TEST")); + Mockito.when(vaultSettings.normalizedDisplayName()).thenReturn(Bindings.createStringBinding(() -> "TEST")); Mockito.when(vaultSettings.usesReadOnlyMode()).thenReturn(new SimpleBooleanProperty(true)); System.setProperty("user.home", tmpDir.toString()); } From 3eb44b06aff5c31cbade57a988accb258607310b Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 3 Nov 2020 12:50:52 +0100 Subject: [PATCH 04/18] Fixes #1061: * temp mount point is first tried without any addition * then with id Suffix in brackets * then with underscores, id suffix and count --- .../mountpoint/TemporaryMountPointChooser.java | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java index 4c96c0599..518ba520b 100644 --- a/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java @@ -47,8 +47,19 @@ public class TemporaryMountPointChooser implements MountPointChooser { private Path choose(Path parent) { String basename = this.vaultSettings.displayName().get(); - for (int i = 0; i < MAX_TMPMOUNTPOINT_CREATION_RETRIES; i++) { - Path mountPoint = parent.resolve(basename + "_" + i); + //regular + Path mountPoint = parent.resolve(basename); + if (Files.notExists(mountPoint)) { + return mountPoint; + } + //with id + mountPoint = parent.resolve(basename + " (" +vaultSettings.getId() + ")"); + if (Files.notExists(mountPoint)) { + return mountPoint; + } + //with id and count + for (int i = 1; i < MAX_TMPMOUNTPOINT_CREATION_RETRIES; i++) { + mountPoint = parent.resolve(basename + "_(" +vaultSettings.getId() + ")_"+i); if (Files.notExists(mountPoint)) { return mountPoint; } From 8853054ed4feb1ce37f1576e924400859bff49b9 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 3 Nov 2020 14:53:51 +0100 Subject: [PATCH 05/18] fixing test --- .../java/org/cryptomator/common/vaults/VaultModuleTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/main/commons/src/test/java/org/cryptomator/common/vaults/VaultModuleTest.java b/main/commons/src/test/java/org/cryptomator/common/vaults/VaultModuleTest.java index 81c821eb8..4329db479 100644 --- a/main/commons/src/test/java/org/cryptomator/common/vaults/VaultModuleTest.java +++ b/main/commons/src/test/java/org/cryptomator/common/vaults/VaultModuleTest.java @@ -17,6 +17,7 @@ import javafx.beans.binding.Bindings; import javafx.beans.binding.StringBinding; import javafx.beans.property.SimpleBooleanProperty; import javafx.beans.property.SimpleObjectProperty; +import javafx.beans.property.SimpleStringProperty; import java.nio.file.Path; public class VaultModuleTest { @@ -30,6 +31,7 @@ public class VaultModuleTest { public void setup(@TempDir Path tmpDir) { Mockito.when(vaultSettings.normalizedDisplayName()).thenReturn(Bindings.createStringBinding(() -> "TEST")); Mockito.when(vaultSettings.usesReadOnlyMode()).thenReturn(new SimpleBooleanProperty(true)); + Mockito.when(vaultSettings.displayName()).thenReturn(new SimpleStringProperty("Vault")); System.setProperty("user.home", tmpDir.toString()); } From c01dd225c9b542d32099695d26674cd8f98fa848 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Thu, 5 Nov 2020 11:47:18 +0100 Subject: [PATCH 06/18] Refactor normalizedMountname to only exclude certain, filesystem reserved characters or Unicode control sequences --- .../common/settings/VaultSettings.java | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/settings/VaultSettings.java b/main/commons/src/main/java/org/cryptomator/common/settings/VaultSettings.java index 7bcb2d4fc..4f471616f 100644 --- a/main/commons/src/main/java/org/cryptomator/common/settings/VaultSettings.java +++ b/main/commons/src/main/java/org/cryptomator/common/settings/VaultSettings.java @@ -7,7 +7,6 @@ package org.cryptomator.common.settings; import com.google.common.base.Strings; import com.google.common.io.BaseEncoding; -import org.apache.commons.lang3.StringUtils; import javafx.beans.Observable; import javafx.beans.binding.Bindings; @@ -24,6 +23,8 @@ import java.nio.file.Path; import java.util.Objects; import java.util.Optional; import java.util.Random; +import java.util.Set; +import java.util.stream.Collectors; /** * The settings specific to a single vault. @@ -76,20 +77,18 @@ public class VaultSettings { //visible for testing String normalizeDisplayName() { - String normalizedMountName = StringUtils.stripAccents(displayName.get()); StringBuilder builder = new StringBuilder(); - for (char c : normalizedMountName.toCharArray()) { - if (Character.isWhitespace(c)) { - if (builder.length() == 0 || builder.charAt(builder.length() - 1) != '_') { - builder.append('_'); + Set notAllowedCharacters = "<>:\"/\\|?*".chars().boxed().collect(Collectors.toUnmodifiableSet()); + if (displayName.isEmpty().get() || displayName.getValueSafe().equals(".") || displayName.getValueSafe().equals("..")) { + builder.append("_"); + } else { + displayName.get().codePoints().forEach(codePoint -> { + if (Character.isDefined(codePoint) && !Character.isIdentifierIgnorable(codePoint) && !notAllowedCharacters.contains(codePoint)) { + builder.appendCodePoint(codePoint); + } else { + builder.append("_"); } - } else if (c < 127 && Character.isLetterOrDigit(c)) { - builder.append(c); - } else { - if (builder.length() == 0 || builder.charAt(builder.length() - 1) != '_') { - builder.append('_'); - } - } + }); } return builder.toString(); } From f64144d1daa4d520788a836b42b27ad574a55a46 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Thu, 5 Nov 2020 11:52:38 +0100 Subject: [PATCH 07/18] Revert 1554437884b0019a5b147c45215a568d9f18f121 --- .../common/mountpoint/TemporaryMountPointChooser.java | 2 +- .../org/cryptomator/common/settings/VaultSettings.java | 8 ++++---- .../java/org/cryptomator/common/vaults/DokanyVolume.java | 2 +- .../java/org/cryptomator/common/vaults/VaultModule.java | 7 +++---- .../java/org/cryptomator/common/vaults/WebDavVolume.java | 2 +- .../org/cryptomator/common/vaults/VaultModuleTest.java | 2 +- 6 files changed, 11 insertions(+), 12 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java index 518ba520b..17c960b1e 100644 --- a/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java @@ -46,7 +46,7 @@ public class TemporaryMountPointChooser implements MountPointChooser { } private Path choose(Path parent) { - String basename = this.vaultSettings.displayName().get(); + String basename = this.vaultSettings.mountName().get(); //regular Path mountPoint = parent.resolve(basename); if (Files.notExists(mountPoint)) { diff --git a/main/commons/src/main/java/org/cryptomator/common/settings/VaultSettings.java b/main/commons/src/main/java/org/cryptomator/common/settings/VaultSettings.java index 4f471616f..9ab999f74 100644 --- a/main/commons/src/main/java/org/cryptomator/common/settings/VaultSettings.java +++ b/main/commons/src/main/java/org/cryptomator/common/settings/VaultSettings.java @@ -54,11 +54,11 @@ public class VaultSettings { private final IntegerProperty filenameLengthLimit = new SimpleIntegerProperty(DEFAULT_FILENAME_LENGTH_LIMIT); private final ObjectProperty actionAfterUnlock = new SimpleObjectProperty<>(DEFAULT_ACTION_AFTER_UNLOCK); - private final StringBinding normalizedDisplayName; + private final StringBinding mountName; public VaultSettings(String id) { this.id = Objects.requireNonNull(id); - this.normalizedDisplayName = Bindings.createStringBinding(this::normalizeDisplayName, displayName); + this.mountName = Bindings.createStringBinding(this::normalizeDisplayName, displayName); } Observable[] observables() { @@ -107,8 +107,8 @@ public class VaultSettings { return displayName; } - public StringBinding normalizedDisplayName() { - return normalizedDisplayName; + public StringBinding mountName() { + return mountName; } public StringProperty winDriveLetter() { diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java index 20d840bcc..644604b4b 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java @@ -44,7 +44,7 @@ public class DokanyVolume extends AbstractVolume { this.mountPoint = determineMountPoint(); String mountName = vaultSettings.displayName().get(); try { - this.mount = mountFactory.mount(fs.getPath("/"), mountPoint, vaultSettings.displayName().get(), FS_TYPE_NAME, mountFlags.strip()); + this.mount = mountFactory.mount(fs.getPath("/"), mountPoint, vaultSettings.mountName().get(), FS_TYPE_NAME, mountFlags.strip()); } catch (MountFailedException e) { if (vaultSettings.getCustomMountPath().isPresent()) { LOG.warn("Failed to mount vault into {}. Is this directory currently accessed by another process (e.g. Windows Explorer)?", mountPoint); diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java index 4fa04ca49..4844143a3 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java @@ -23,7 +23,6 @@ import javafx.beans.property.BooleanProperty; import javafx.beans.property.ObjectProperty; import javafx.beans.property.ReadOnlyBooleanProperty; import javafx.beans.property.SimpleObjectProperty; -import javafx.beans.property.StringProperty; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -76,7 +75,7 @@ public class VaultModule { @DefaultMountFlags public StringBinding provideDefaultMountFlags(Settings settings, VaultSettings vaultSettings) { ObjectProperty preferredVolumeImpl = settings.preferredVolumeImpl(); - StringProperty mountName = vaultSettings.displayName(); + StringBinding mountName = vaultSettings.mountName(); BooleanProperty readOnly = vaultSettings.usesReadOnlyMode(); return Bindings.createStringBinding(() -> { @@ -96,7 +95,7 @@ public class VaultModule { } // see: https://github.com/osxfuse/osxfuse/wiki/Mount-options - private String getMacFuseDefaultMountFlags(StringProperty mountName, ReadOnlyBooleanProperty readOnly) { + private String getMacFuseDefaultMountFlags(StringBinding mountName, ReadOnlyBooleanProperty readOnly) { assert SystemUtils.IS_OS_MAC_OSX; StringBuilder flags = new StringBuilder(); if (readOnly.get()) { @@ -148,7 +147,7 @@ public class VaultModule { // see https://github.com/billziss-gh/winfsp/blob/5d0b10d0b643652c00ebb4704dc2bb28e7244973/src/dll/fuse/fuse_main.c#L53-L62 for syntax guide // see https://github.com/billziss-gh/winfsp/blob/5d0b10d0b643652c00ebb4704dc2bb28e7244973/src/dll/fuse/fuse.c#L295-L319 for options (-o <...>) // see https://github.com/billziss-gh/winfsp/wiki/Frequently-Asked-Questions/5ba00e4be4f5e938eaae6ef1500b331de12dee77 (FUSE 4.) on why the given defaults were choosen - private String getWindowsFuseDefaultMountFlags(StringProperty mountName, ReadOnlyBooleanProperty readOnly) { + private String getWindowsFuseDefaultMountFlags(StringBinding mountName, ReadOnlyBooleanProperty readOnly) { assert SystemUtils.IS_OS_WINDOWS; StringBuilder flags = new StringBuilder(); diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java index aa40ef57b..bf42e353f 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java @@ -45,7 +45,7 @@ public class WebDavVolume implements Volume { if (!server.isRunning()) { server.start(); } - servlet = server.createWebDavServlet(fs.getPath("/"), vaultSettings.getId() + "/" + vaultSettings.normalizedDisplayName().get()); + servlet = server.createWebDavServlet(fs.getPath("/"), vaultSettings.getId() + "/" + vaultSettings.mountName().get()); servlet.start(); mount(); } diff --git a/main/commons/src/test/java/org/cryptomator/common/vaults/VaultModuleTest.java b/main/commons/src/test/java/org/cryptomator/common/vaults/VaultModuleTest.java index 4329db479..9b25ebb08 100644 --- a/main/commons/src/test/java/org/cryptomator/common/vaults/VaultModuleTest.java +++ b/main/commons/src/test/java/org/cryptomator/common/vaults/VaultModuleTest.java @@ -29,7 +29,7 @@ public class VaultModuleTest { @BeforeEach public void setup(@TempDir Path tmpDir) { - Mockito.when(vaultSettings.normalizedDisplayName()).thenReturn(Bindings.createStringBinding(() -> "TEST")); + Mockito.when(vaultSettings.mountName()).thenReturn(Bindings.createStringBinding(() -> "TEST")); Mockito.when(vaultSettings.usesReadOnlyMode()).thenReturn(new SimpleBooleanProperty(true)); Mockito.when(vaultSettings.displayName()).thenReturn(new SimpleStringProperty("Vault")); System.setProperty("user.home", tmpDir.toString()); From a22fbea467dce68e3f969b73a49b1d61fd38d256 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Thu, 5 Nov 2020 11:55:25 +0100 Subject: [PATCH 08/18] The display/vault name is now constrained: * should not be empty * is stripped of leading & trailing whitespaces * is truncated to 50 characters --- .../GeneralVaultOptionsController.java | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/main/ui/src/main/java/org/cryptomator/ui/vaultoptions/GeneralVaultOptionsController.java b/main/ui/src/main/java/org/cryptomator/ui/vaultoptions/GeneralVaultOptionsController.java index 4425139f3..cf356c0c8 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/vaultoptions/GeneralVaultOptionsController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/vaultoptions/GeneralVaultOptionsController.java @@ -1,20 +1,26 @@ package org.cryptomator.ui.vaultoptions; +import com.google.common.base.Strings; import org.cryptomator.common.settings.WhenUnlocked; import org.cryptomator.common.vaults.Vault; import org.cryptomator.ui.common.FxController; import javax.inject.Inject; +import javafx.beans.Observable; import javafx.fxml.FXML; import javafx.scene.control.CheckBox; import javafx.scene.control.ChoiceBox; import javafx.scene.control.TextField; +import javafx.stage.Stage; import javafx.util.StringConverter; import java.util.ResourceBundle; @VaultOptionsScoped public class GeneralVaultOptionsController implements FxController { + private static final int VAULTNAME_TRUNCATE_THRESHOLD = 50; + + private final Stage window; private final Vault vault; private final ResourceBundle resourceBundle; @@ -23,20 +29,37 @@ public class GeneralVaultOptionsController implements FxController { public ChoiceBox actionAfterUnlockChoiceBox; @Inject - GeneralVaultOptionsController(@VaultOptionsWindow Vault vault, ResourceBundle resourceBundle) { + GeneralVaultOptionsController(@VaultOptionsWindow Stage window, @VaultOptionsWindow Vault vault, ResourceBundle resourceBundle) { + this.window = window; this.vault = vault; this.resourceBundle = resourceBundle; } @FXML public void initialize() { - vaultName.textProperty().bindBidirectional(vault.getVaultSettings().displayName()); + vaultName.textProperty().set(vault.getVaultSettings().displayName().get()); + vaultName.focusedProperty().addListener(this::checkTrimAndTuncateVaultName); unlockOnStartupCheckbox.selectedProperty().bindBidirectional(vault.getVaultSettings().unlockAfterStartup()); actionAfterUnlockChoiceBox.getItems().addAll(WhenUnlocked.values()); actionAfterUnlockChoiceBox.valueProperty().bindBidirectional(vault.getVaultSettings().actionAfterUnlock()); actionAfterUnlockChoiceBox.setConverter(new WhenUnlockedConverter(resourceBundle)); } + private void checkTrimAndTuncateVaultName(Observable observable, Boolean oldValue, Boolean newValue) { + if (!vaultName.isFocused()) { //only check if TextField loses focus + String currentContent = vaultName.getText().trim(); + if (!Strings.isNullOrEmpty(currentContent)) { + if (currentContent.length() > 50) { + currentContent = currentContent.substring(0, VAULTNAME_TRUNCATE_THRESHOLD); + } + vaultName.textProperty().set(currentContent); + vault.getVaultSettings().displayName().setValue(currentContent); + } else { + vaultName.textProperty().set(vault.getVaultSettings().displayName().get()); + } + } + } + private static class WhenUnlockedConverter extends StringConverter { private final ResourceBundle resourceBundle; From 510ea8a6f6ca0728b22f30887a4e727178326d1e Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Thu, 5 Nov 2020 12:44:51 +0100 Subject: [PATCH 09/18] refined normalize Method, fxied Tests for it: * all unicode spaces are now replaced with \u0020 * if the end string only contains whitspaces, "_" will be returned --- .../common/settings/VaultSettings.java | 27 ++++++++++++------- .../common/settings/VaultSettingsTest.java | 2 +- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/settings/VaultSettings.java b/main/commons/src/main/java/org/cryptomator/common/settings/VaultSettings.java index 9ab999f74..0619119d7 100644 --- a/main/commons/src/main/java/org/cryptomator/common/settings/VaultSettings.java +++ b/main/commons/src/main/java/org/cryptomator/common/settings/VaultSettings.java @@ -79,18 +79,25 @@ public class VaultSettings { String normalizeDisplayName() { StringBuilder builder = new StringBuilder(); Set notAllowedCharacters = "<>:\"/\\|?*".chars().boxed().collect(Collectors.toUnmodifiableSet()); + if (displayName.isEmpty().get() || displayName.getValueSafe().equals(".") || displayName.getValueSafe().equals("..")) { - builder.append("_"); - } else { - displayName.get().codePoints().forEach(codePoint -> { - if (Character.isDefined(codePoint) && !Character.isIdentifierIgnorable(codePoint) && !notAllowedCharacters.contains(codePoint)) { - builder.appendCodePoint(codePoint); - } else { - builder.append("_"); - } - }); + return builder.append("_").toString(); + } + + displayName.get().codePoints().forEach(codePoint -> { + if (Character.isDefined(codePoint) && !Character.isIdentifierIgnorable(codePoint) && !notAllowedCharacters.contains(codePoint)) { + builder.appendCodePoint(Character.isWhitespace(codePoint) || Character.isSpaceChar(codePoint) ? 0x020 : codePoint); + } else { + builder.append("_"); + } + }); + + String result = builder.toString(); + if (!result.isBlank()) { + return result; + } else { + return "_"; } - return builder.toString(); } /* Getter/Setter */ diff --git a/main/commons/src/test/java/org/cryptomator/common/settings/VaultSettingsTest.java b/main/commons/src/test/java/org/cryptomator/common/settings/VaultSettingsTest.java index 8ec6dc681..198e2937c 100644 --- a/main/commons/src/test/java/org/cryptomator/common/settings/VaultSettingsTest.java +++ b/main/commons/src/test/java/org/cryptomator/common/settings/VaultSettingsTest.java @@ -16,7 +16,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; public class VaultSettingsTest { @ParameterizedTest - @CsvSource({"a a,a_a", "ä,a", "Ĉ,C", ":,_", "汉语,_"}) + @CsvSource({"a\u000Fa,a_a", ": \\,_ _", "汉语,汉语", "..,_", "a\ta,a\u0020a", "\t\n\r,_"}) public void testNormalize(String test, String expected) { VaultSettings settings = new VaultSettings("id"); settings.displayName().setValue(test); From 813c01aaedaaa182629867449737724fbd21b999 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Thu, 5 Nov 2020 13:23:30 +0100 Subject: [PATCH 10/18] Use mountName instead of display name --- .../main/java/org/cryptomator/common/vaults/DokanyVolume.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java index 644604b4b..e91c9f86d 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java @@ -42,7 +42,7 @@ public class DokanyVolume extends AbstractVolume { @Override public void mount(CryptoFileSystem fs, String mountFlags) throws InvalidMountPointException, VolumeException { this.mountPoint = determineMountPoint(); - String mountName = vaultSettings.displayName().get(); + String mountName = vaultSettings.mountName().get(); try { this.mount = mountFactory.mount(fs.getPath("/"), mountPoint, vaultSettings.mountName().get(), FS_TYPE_NAME, mountFlags.strip()); } catch (MountFailedException e) { From 78df4e24b3d9a29446128651a00672a44448aeee Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 6 Nov 2020 09:51:51 +0100 Subject: [PATCH 11/18] keep iconography consistent by reusing already existing icons --- .../java/org/cryptomator/ui/controls/FontAwesome5Icon.java | 1 - .../ui/src/main/resources/fxml/unlock_invalid_mount_point.fxml | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/main/ui/src/main/java/org/cryptomator/ui/controls/FontAwesome5Icon.java b/main/ui/src/main/java/org/cryptomator/ui/controls/FontAwesome5Icon.java index 0a199fb48..f4d0d058a 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/controls/FontAwesome5Icon.java +++ b/main/ui/src/main/java/org/cryptomator/ui/controls/FontAwesome5Icon.java @@ -39,7 +39,6 @@ public enum FontAwesome5Icon { SPINNER("\uF110"), // SYNC("\uF021"), // TIMES("\uF00D"), // - TIMES_CIRCLE("\uF057"), // TRASH("\uF1F8"), // UNLINK("\uf127"), // WRENCH("\uF0AD"), // diff --git a/main/ui/src/main/resources/fxml/unlock_invalid_mount_point.fxml b/main/ui/src/main/resources/fxml/unlock_invalid_mount_point.fxml index 9d56a32e7..42ef788eb 100644 --- a/main/ui/src/main/resources/fxml/unlock_invalid_mount_point.fxml +++ b/main/ui/src/main/resources/fxml/unlock_invalid_mount_point.fxml @@ -24,7 +24,8 @@ - + + From c838da9df4c266a6220fa94ce323bc47e1e3496a Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 6 Nov 2020 09:52:31 +0100 Subject: [PATCH 12/18] reducing complexity for "vault name max length" --- .../GeneralVaultOptionsController.java | 24 +++++++------------ 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/main/ui/src/main/java/org/cryptomator/ui/vaultoptions/GeneralVaultOptionsController.java b/main/ui/src/main/java/org/cryptomator/ui/vaultoptions/GeneralVaultOptionsController.java index cf356c0c8..562660f9b 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/vaultoptions/GeneralVaultOptionsController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/vaultoptions/GeneralVaultOptionsController.java @@ -1,16 +1,15 @@ package org.cryptomator.ui.vaultoptions; -import com.google.common.base.Strings; import org.cryptomator.common.settings.WhenUnlocked; import org.cryptomator.common.vaults.Vault; import org.cryptomator.ui.common.FxController; import javax.inject.Inject; -import javafx.beans.Observable; import javafx.fxml.FXML; import javafx.scene.control.CheckBox; import javafx.scene.control.ChoiceBox; import javafx.scene.control.TextField; +import javafx.scene.control.TextFormatter; import javafx.stage.Stage; import javafx.util.StringConverter; import java.util.ResourceBundle; @@ -37,26 +36,19 @@ public class GeneralVaultOptionsController implements FxController { @FXML public void initialize() { - vaultName.textProperty().set(vault.getVaultSettings().displayName().get()); - vaultName.focusedProperty().addListener(this::checkTrimAndTuncateVaultName); + vaultName.textProperty().bindBidirectional(vault.getVaultSettings().displayName()); + vaultName.setTextFormatter(new TextFormatter<>(this::checkVaultNameLength)); unlockOnStartupCheckbox.selectedProperty().bindBidirectional(vault.getVaultSettings().unlockAfterStartup()); actionAfterUnlockChoiceBox.getItems().addAll(WhenUnlocked.values()); actionAfterUnlockChoiceBox.valueProperty().bindBidirectional(vault.getVaultSettings().actionAfterUnlock()); actionAfterUnlockChoiceBox.setConverter(new WhenUnlockedConverter(resourceBundle)); } - private void checkTrimAndTuncateVaultName(Observable observable, Boolean oldValue, Boolean newValue) { - if (!vaultName.isFocused()) { //only check if TextField loses focus - String currentContent = vaultName.getText().trim(); - if (!Strings.isNullOrEmpty(currentContent)) { - if (currentContent.length() > 50) { - currentContent = currentContent.substring(0, VAULTNAME_TRUNCATE_THRESHOLD); - } - vaultName.textProperty().set(currentContent); - vault.getVaultSettings().displayName().setValue(currentContent); - } else { - vaultName.textProperty().set(vault.getVaultSettings().displayName().get()); - } + private TextFormatter.Change checkVaultNameLength(TextFormatter.Change change) { + if (change.isContentChange() && change.getControlNewText().length() > VAULTNAME_TRUNCATE_THRESHOLD) { + return null; // reject any change that would lead to a text exceeding threshold + } else { + return change; } } From f4103fc9177dec13b3abc37f9aeaeaf9b6b3f094 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 6 Nov 2020 09:54:25 +0100 Subject: [PATCH 13/18] reducing complexity of normalizedDisplayName --- .../common/settings/VaultSettings.java | 28 ++++++------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/settings/VaultSettings.java b/main/commons/src/main/java/org/cryptomator/common/settings/VaultSettings.java index 0619119d7..7de22753a 100644 --- a/main/commons/src/main/java/org/cryptomator/common/settings/VaultSettings.java +++ b/main/commons/src/main/java/org/cryptomator/common/settings/VaultSettings.java @@ -5,6 +5,7 @@ *******************************************************************************/ package org.cryptomator.common.settings; +import com.google.common.base.CharMatcher; import com.google.common.base.Strings; import com.google.common.io.BaseEncoding; @@ -77,27 +78,16 @@ public class VaultSettings { //visible for testing String normalizeDisplayName() { - StringBuilder builder = new StringBuilder(); - Set notAllowedCharacters = "<>:\"/\\|?*".chars().boxed().collect(Collectors.toUnmodifiableSet()); - - if (displayName.isEmpty().get() || displayName.getValueSafe().equals(".") || displayName.getValueSafe().equals("..")) { - return builder.append("_").toString(); - } - - displayName.get().codePoints().forEach(codePoint -> { - if (Character.isDefined(codePoint) && !Character.isIdentifierIgnorable(codePoint) && !notAllowedCharacters.contains(codePoint)) { - builder.appendCodePoint(Character.isWhitespace(codePoint) || Character.isSpaceChar(codePoint) ? 0x020 : codePoint); - } else { - builder.append("_"); - } - }); - - String result = builder.toString(); - if (!result.isBlank()) { - return result; - } else { + var original = displayName.getValueSafe(); + if (original.isBlank() || ".".equals(original) || "..".equals(original)) { return "_"; } + + // replace whitespaces (tabs, linebreaks, ...) by simple space (0x20) + var withoutFancyWhitespaces = CharMatcher.whitespace().collapseFrom(original, ' '); + + // replace control chars as well as chars that aren't allowed in file names on standard file systems by underscore + return CharMatcher.anyOf("<>:\"/\\|?*").or(CharMatcher.javaIsoControl()).collapseFrom(withoutFancyWhitespaces, '_'); } /* Getter/Setter */ From 6848f1a38e6d6f4e004a63efb5c9ddedb884ccef Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 6 Nov 2020 11:07:22 +0100 Subject: [PATCH 14/18] Renamed IrregularUnmountCleaner and relayouted code --- .../mountpoint/IrregularUnmountCleaner.java | 64 +++++++++++++++++++ .../mountpoint/UnregularUnmountCleaner.java | 60 ----------------- .../cryptomator/ui/launcher/UiLauncher.java | 4 +- 3 files changed, 66 insertions(+), 62 deletions(-) create mode 100644 main/commons/src/main/java/org/cryptomator/common/mountpoint/IrregularUnmountCleaner.java delete mode 100644 main/commons/src/main/java/org/cryptomator/common/mountpoint/UnregularUnmountCleaner.java diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/IrregularUnmountCleaner.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/IrregularUnmountCleaner.java new file mode 100644 index 000000000..3104ae20b --- /dev/null +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/IrregularUnmountCleaner.java @@ -0,0 +1,64 @@ +package org.cryptomator.common.mountpoint; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.nio.file.DirectoryNotEmptyException; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributes; + +public class IrregularUnmountCleaner { + + public static Logger LOG = LoggerFactory.getLogger(IrregularUnmountCleaner.class); + + public static void removeUnregularUnmountDebris(Path dirContainingMountPoints) { + IOException cleanupFailed = new IOException("Cleanup failed"); + + try { + LOG.debug("Performing cleanup of mountpoint dir {}.", dirContainingMountPoints); + for (Path p : Files.newDirectoryStream(dirContainingMountPoints)) { + try { + var attr = Files.readAttributes(p, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS); + if (attr.isOther() && attr.isDirectory()) { // yes, this is possible with windows junction points -.- + Files.delete(p); + } else if (attr.isDirectory()) { + deleteEmptyDir(p); + } else if (attr.isSymbolicLink()) { + deleteDeadLink(p); + } else { + LOG.debug("Found non-directory element in mountpoint dir: {}", p); + } + } catch (IOException e) { + cleanupFailed.addSuppressed(e); + } + } + + if (cleanupFailed.getSuppressed().length > 0) { + throw cleanupFailed; + } + } catch (IOException e) { + LOG.warn("Unable to perform cleanup of mountpoint dir {}.", dirContainingMountPoints, e); + } + + } + + private static void deleteEmptyDir(Path dir) throws IOException { + assert Files.isDirectory(dir, LinkOption.NOFOLLOW_LINKS); + try { + Files.delete(dir); // attempt to delete dir non-recursively (will fail, if there are contents) + } catch (DirectoryNotEmptyException e) { + LOG.info("Found non-empty directory in mountpoint dir: {}", dir); + } + } + + private static void deleteDeadLink(Path symlink) throws IOException { + assert Files.isSymbolicLink(symlink); + if (Files.notExists(symlink)) { // following link: target does not exist + Files.delete(symlink); + } + } + +} diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/UnregularUnmountCleaner.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/UnregularUnmountCleaner.java deleted file mode 100644 index 3229f909c..000000000 --- a/main/commons/src/main/java/org/cryptomator/common/mountpoint/UnregularUnmountCleaner.java +++ /dev/null @@ -1,60 +0,0 @@ -package org.cryptomator.common.mountpoint; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import java.io.IOException; -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; -import java.util.ArrayList; -import java.util.Collection; - -public class UnregularUnmountCleaner { - - public static Logger LOG = LoggerFactory.getLogger(UnregularUnmountCleaner.class); - - public static void removeUnregularUnmountDebris(Path dirOfMountPoints) { - Collection exceptionCatcher = new ArrayList<>(); - try { - LOG.debug("Performing cleanup of mountpoint dir {}.", dirOfMountPoints); - Files.list(dirOfMountPoints).filter(p -> { - if (Files.isDirectory(p, LinkOption.NOFOLLOW_LINKS)) { - return true; - } else { - LOG.debug("Found non-directory element in mountpoint dir: {}", p); - return false; - } - }).forEach(dir -> { - try { - try { - Files.readAttributes(dir, BasicFileAttributes.class); //we follow the link and see if it exists - } catch (NoSuchFileException e) { - Files.delete(dir); //broken link file, we delete it - LOG.debug("Removed broken entry {} from mountpoint dir."); - return; - } - //check if dir is empty - if (Files.list(dir).findFirst().isEmpty()) { - Files.delete(dir); - LOG.debug("Removed empty dir {} from mountpoint dir."); - } else { - LOG.debug("Found non-empty directory in mountpoint dir: {}", dir); - } - } catch (IOException e) { - exceptionCatcher.add(e); - } - }); - - exceptionCatcher.forEach(exception -> { - LOG.debug("Unable to clean element from mountpoint dir due to", exception); - }); - } catch (IOException e) { - LOG.debug("Unable to perform cleanup of mountpoint dir {}.", dirOfMountPoints, e); - } - - } - -} diff --git a/main/ui/src/main/java/org/cryptomator/ui/launcher/UiLauncher.java b/main/ui/src/main/java/org/cryptomator/ui/launcher/UiLauncher.java index 9b9e7ee05..f1de98ea7 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/launcher/UiLauncher.java +++ b/main/ui/src/main/java/org/cryptomator/ui/launcher/UiLauncher.java @@ -1,7 +1,7 @@ package org.cryptomator.ui.launcher; import org.cryptomator.common.Environment; -import org.cryptomator.common.mountpoint.UnregularUnmountCleaner; +import org.cryptomator.common.mountpoint.IrregularUnmountCleaner; import org.cryptomator.common.settings.Settings; import org.cryptomator.common.vaults.Vault; import org.cryptomator.jni.JniException; @@ -67,7 +67,7 @@ public class UiLauncher { //clean leftovers of not-regularly unmounted vaults //see https://github.com/cryptomator/cryptomator/issues/1013 and https://github.com/cryptomator/cryptomator/issues/1061 - env.getMountPointsDir().ifPresent(UnregularUnmountCleaner::removeUnregularUnmountDebris); + env.getMountPointsDir().ifPresent(IrregularUnmountCleaner::removeUnregularUnmountDebris); // auto unlock Collection vaultsToAutoUnlock = vaults.filtered(this::shouldAttemptAutoUnlock); From 9076fe5b4652fbcbf2f591bb8639f70f5965db91 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 6 Nov 2020 11:07:45 +0100 Subject: [PATCH 15/18] Reintroduced trimming of vault display name --- .../vaultoptions/GeneralVaultOptionsController.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/main/ui/src/main/java/org/cryptomator/ui/vaultoptions/GeneralVaultOptionsController.java b/main/ui/src/main/java/org/cryptomator/ui/vaultoptions/GeneralVaultOptionsController.java index 562660f9b..abf233c2b 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/vaultoptions/GeneralVaultOptionsController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/vaultoptions/GeneralVaultOptionsController.java @@ -5,6 +5,7 @@ import org.cryptomator.common.vaults.Vault; import org.cryptomator.ui.common.FxController; import javax.inject.Inject; +import javafx.beans.Observable; import javafx.fxml.FXML; import javafx.scene.control.CheckBox; import javafx.scene.control.ChoiceBox; @@ -36,7 +37,8 @@ public class GeneralVaultOptionsController implements FxController { @FXML public void initialize() { - vaultName.textProperty().bindBidirectional(vault.getVaultSettings().displayName()); + vaultName.textProperty().set(vault.getVaultSettings().displayName().get()); + vaultName.focusedProperty().addListener(this::checkTrimAndTuncateVaultName); vaultName.setTextFormatter(new TextFormatter<>(this::checkVaultNameLength)); unlockOnStartupCheckbox.selectedProperty().bindBidirectional(vault.getVaultSettings().unlockAfterStartup()); actionAfterUnlockChoiceBox.getItems().addAll(WhenUnlocked.values()); @@ -44,6 +46,13 @@ public class GeneralVaultOptionsController implements FxController { actionAfterUnlockChoiceBox.setConverter(new WhenUnlockedConverter(resourceBundle)); } + private void checkTrimAndTuncateVaultName(Observable observable, Boolean wasFocussed, Boolean isFocussed) { + if (!isFocussed) { + var trimmed = vaultName.getText().trim(); + vault.getVaultSettings().displayName().set(trimmed); + } + } + private TextFormatter.Change checkVaultNameLength(TextFormatter.Change change) { if (change.isContentChange() && change.getControlNewText().length() > VAULTNAME_TRUNCATE_THRESHOLD) { return null; // reject any change that would lead to a text exceeding threshold From afb9d4f01029f8de8155c9a7dd4ae2c55c93cab4 Mon Sep 17 00:00:00 2001 From: JaniruTEC Date: Fri, 6 Nov 2020 18:43:23 +0100 Subject: [PATCH 16/18] Fixed exception on startup if mount folder doesn't exist IrregularUnmountCleaner should not be called if there is no folder that could contain mountpoints. --- .../src/main/java/org/cryptomator/ui/launcher/UiLauncher.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/main/ui/src/main/java/org/cryptomator/ui/launcher/UiLauncher.java b/main/ui/src/main/java/org/cryptomator/ui/launcher/UiLauncher.java index 6b2abb557..0fc8ab086 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/launcher/UiLauncher.java +++ b/main/ui/src/main/java/org/cryptomator/ui/launcher/UiLauncher.java @@ -16,6 +16,8 @@ import javafx.collections.ObservableList; import java.awt.Desktop; import java.awt.SystemTray; import java.awt.desktop.AppReopenedListener; +import java.nio.file.Files; +import java.nio.file.LinkOption; import java.util.Collection; import java.util.Optional; @@ -65,7 +67,7 @@ public class UiLauncher { //clean leftovers of not-regularly unmounted vaults //see https://github.com/cryptomator/cryptomator/issues/1013 and https://github.com/cryptomator/cryptomator/issues/1061 - env.getMountPointsDir().ifPresent(IrregularUnmountCleaner::removeUnregularUnmountDebris); + env.getMountPointsDir().filter(path -> Files.exists(path, LinkOption.NOFOLLOW_LINKS)).ifPresent(IrregularUnmountCleaner::removeUnregularUnmountDebris); // auto unlock Collection vaultsToAutoUnlock = vaults.filtered(this::shouldAttemptAutoUnlock); From 75f66e40bf245d1c5b4567734fefab95da124e17 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 10 Nov 2020 12:05:30 +0100 Subject: [PATCH 17/18] Add additional mountName crunching to be urlconform when WebDAV is used --- .../java/org/cryptomator/common/vaults/WebDavVolume.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java index bf42e353f..fe9246085 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java @@ -1,6 +1,7 @@ package org.cryptomator.common.vaults; +import com.google.common.base.CharMatcher; import org.cryptomator.common.settings.Settings; import org.cryptomator.common.settings.VaultSettings; import org.cryptomator.common.settings.VolumeImpl; @@ -45,7 +46,9 @@ public class WebDavVolume implements Volume { if (!server.isRunning()) { server.start(); } - servlet = server.createWebDavServlet(fs.getPath("/"), vaultSettings.getId() + "/" + vaultSettings.mountName().get()); + CharMatcher acceptable = CharMatcher.inRange('0', '9').or(CharMatcher.inRange('A', 'Z')).or(CharMatcher.inRange('a', 'z')); + String urlConformMountName = acceptable.negate().collapseFrom(vaultSettings.mountName().get(), '_'); + servlet = server.createWebDavServlet(fs.getPath("/"), vaultSettings.getId() + "/" + urlConformMountName); servlet.start(); mount(); } From 1947623be861ef263a033091494e4c21a8c88e79 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 10 Nov 2020 12:24:49 +0100 Subject: [PATCH 18/18] final cleanup: * renamed methods * apply code style --- .../common/mountpoint/IrregularUnmountCleaner.java | 2 +- .../java/org/cryptomator/ui/launcher/UiLauncher.java | 10 +++++----- .../ui/vaultoptions/GeneralVaultOptionsController.java | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/IrregularUnmountCleaner.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/IrregularUnmountCleaner.java index 3104ae20b..d91fd0041 100644 --- a/main/commons/src/main/java/org/cryptomator/common/mountpoint/IrregularUnmountCleaner.java +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/IrregularUnmountCleaner.java @@ -14,7 +14,7 @@ public class IrregularUnmountCleaner { public static Logger LOG = LoggerFactory.getLogger(IrregularUnmountCleaner.class); - public static void removeUnregularUnmountDebris(Path dirContainingMountPoints) { + public static void removeIrregularUnmountDebris(Path dirContainingMountPoints) { IOException cleanupFailed = new IOException("Cleanup failed"); try { diff --git a/main/ui/src/main/java/org/cryptomator/ui/launcher/UiLauncher.java b/main/ui/src/main/java/org/cryptomator/ui/launcher/UiLauncher.java index 0fc8ab086..350a0a1ca 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/launcher/UiLauncher.java +++ b/main/ui/src/main/java/org/cryptomator/ui/launcher/UiLauncher.java @@ -31,8 +31,8 @@ public class UiLauncher { private final TrayMenuComponent.Builder trayComponent; private final FxApplicationStarter fxApplicationStarter; private final AppLaunchEventHandler launchEventHandler; - private final Optional trayIntegration; - private final Environment env; + private final Optional trayIntegration; + private final Environment env; @Inject public UiLauncher(Settings settings, ObservableList vaults, TrayMenuComponent.Builder trayComponent, FxApplicationStarter fxApplicationStarter, AppLaunchEventHandler launchEventHandler, Optional trayIntegration, Environment env) { @@ -41,8 +41,8 @@ public class UiLauncher { this.trayComponent = trayComponent; this.fxApplicationStarter = fxApplicationStarter; this.launchEventHandler = launchEventHandler; - this.trayIntegration = trayIntegration; - this.env = env; + this.trayIntegration = trayIntegration; + this.env = env; } public void launch() { @@ -67,7 +67,7 @@ public class UiLauncher { //clean leftovers of not-regularly unmounted vaults //see https://github.com/cryptomator/cryptomator/issues/1013 and https://github.com/cryptomator/cryptomator/issues/1061 - env.getMountPointsDir().filter(path -> Files.exists(path, LinkOption.NOFOLLOW_LINKS)).ifPresent(IrregularUnmountCleaner::removeUnregularUnmountDebris); + env.getMountPointsDir().filter(path -> Files.exists(path, LinkOption.NOFOLLOW_LINKS)).ifPresent(IrregularUnmountCleaner::removeIrregularUnmountDebris); // auto unlock Collection vaultsToAutoUnlock = vaults.filtered(this::shouldAttemptAutoUnlock); diff --git a/main/ui/src/main/java/org/cryptomator/ui/vaultoptions/GeneralVaultOptionsController.java b/main/ui/src/main/java/org/cryptomator/ui/vaultoptions/GeneralVaultOptionsController.java index abf233c2b..b760ebf9a 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/vaultoptions/GeneralVaultOptionsController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/vaultoptions/GeneralVaultOptionsController.java @@ -38,7 +38,7 @@ public class GeneralVaultOptionsController implements FxController { @FXML public void initialize() { vaultName.textProperty().set(vault.getVaultSettings().displayName().get()); - vaultName.focusedProperty().addListener(this::checkTrimAndTuncateVaultName); + vaultName.focusedProperty().addListener(this::trimVaultNameOnFocusLoss); vaultName.setTextFormatter(new TextFormatter<>(this::checkVaultNameLength)); unlockOnStartupCheckbox.selectedProperty().bindBidirectional(vault.getVaultSettings().unlockAfterStartup()); actionAfterUnlockChoiceBox.getItems().addAll(WhenUnlocked.values()); @@ -46,7 +46,7 @@ public class GeneralVaultOptionsController implements FxController { actionAfterUnlockChoiceBox.setConverter(new WhenUnlockedConverter(resourceBundle)); } - private void checkTrimAndTuncateVaultName(Observable observable, Boolean wasFocussed, Boolean isFocussed) { + private void trimVaultNameOnFocusLoss(Observable observable, Boolean wasFocussed, Boolean isFocussed) { if (!isFocussed) { var trimmed = vaultName.getText().trim(); vault.getVaultSettings().displayName().set(trimmed);