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..d91fd0041 --- /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 removeIrregularUnmountDebris(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/TemporaryMountPointChooser.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java index 60991b97e..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 @@ -47,8 +47,19 @@ public class TemporaryMountPointChooser implements MountPointChooser { private Path choose(Path parent) { String basename = this.vaultSettings.mountName().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; } 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..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,9 +5,9 @@ *******************************************************************************/ package org.cryptomator.common.settings; +import com.google.common.base.CharMatcher; 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 +24,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,22 +78,16 @@ 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('_'); - } - } else if (c < 127 && Character.isLetterOrDigit(c)) { - builder.append(c); - } else { - if (builder.length() == 0 || builder.charAt(builder.length() - 1) != '_') { - builder.append('_'); - } - } + var original = displayName.getValueSafe(); + if (original.isBlank() || ".".equals(original) || "..".equals(original)) { + return "_"; } - return builder.toString(); + + // 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 */ 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) { 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(); } 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); 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..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 @@ -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.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()); } 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 ecef51dbf..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 @@ -1,5 +1,7 @@ package org.cryptomator.ui.launcher; +import org.cryptomator.common.Environment; +import org.cryptomator.common.mountpoint.IrregularUnmountCleaner; import org.cryptomator.common.settings.Settings; import org.cryptomator.common.vaults.Vault; import org.cryptomator.integrations.tray.TrayIntegrationProvider; @@ -14,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; @@ -28,15 +32,17 @@ public class UiLauncher { private final FxApplicationStarter fxApplicationStarter; private final AppLaunchEventHandler launchEventHandler; private final Optional trayIntegration; + private final Environment env; @Inject - public UiLauncher(Settings settings, ObservableList vaults, TrayMenuComponent.Builder trayComponent, FxApplicationStarter fxApplicationStarter, AppLaunchEventHandler launchEventHandler, Optional trayIntegration) { + public UiLauncher(Settings settings, ObservableList vaults, TrayMenuComponent.Builder trayComponent, FxApplicationStarter fxApplicationStarter, AppLaunchEventHandler launchEventHandler, Optional trayIntegration, Environment env) { this.settings = settings; this.vaults = vaults; this.trayComponent = trayComponent; this.fxApplicationStarter = fxApplicationStarter; this.launchEventHandler = launchEventHandler; this.trayIntegration = trayIntegration; + this.env = env; } public void launch() { @@ -59,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().filter(path -> Files.exists(path, LinkOption.NOFOLLOW_LINKS)).ifPresent(IrregularUnmountCleaner::removeIrregularUnmountDebris); + // auto unlock Collection vaultsToAutoUnlock = vaults.filtered(this::shouldAttemptAutoUnlock); if (!vaultsToAutoUnlock.isEmpty()) { 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..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 @@ -5,16 +5,22 @@ 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; @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,38 @@ 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::trimVaultNameOnFocusLoss); + 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 trimVaultNameOnFocusLoss(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 + } else { + return change; + } + } + private static class WhenUnlockedConverter extends StringConverter { private final ResourceBundle resourceBundle; 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..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 @@ -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 18f3f33d9..b76bba812 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