Merge pull request #1396 from cryptomator/feature/#1013-#1061-cleanupAndInformation

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);
+		}
+	}
+
+}

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;
 			}

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 */

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) {

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();
 	}

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);

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());
 	}
 

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<TrayIntegrationProvider> trayIntegration;
+	private final Environment env;
 
 	@Inject
-	public UiLauncher(Settings settings, ObservableList<Vault> vaults, TrayMenuComponent.Builder trayComponent, FxApplicationStarter fxApplicationStarter, AppLaunchEventHandler launchEventHandler, Optional<TrayIntegrationProvider> trayIntegration) {
+	public UiLauncher(Settings settings, ObservableList<Vault> vaults, TrayMenuComponent.Builder trayComponent, FxApplicationStarter fxApplicationStarter, AppLaunchEventHandler launchEventHandler, Optional<TrayIntegrationProvider> 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<Vault> vaultsToAutoUnlock = vaults.filtered(this::shouldAttemptAutoUnlock);
 		if (!vaultsToAutoUnlock.isEmpty()) {

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<WhenUnlocked> 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<WhenUnlocked> {
 
 		private final ResourceBundle resourceBundle;

main/ui/src/main/resources/fxml/unlock_invalid_mount_point.fxml

@@ -10,6 +10,7 @@
 <?import javafx.scene.layout.StackPane?>
 <?import javafx.scene.layout.VBox?>
 <?import javafx.scene.shape.Circle?>
+<?import javafx.scene.text.Text?>
 <VBox xmlns:fx="http://javafx.com/fxml"
 	  xmlns="http://javafx.com/javafx"
 	  fx:controller="org.cryptomator.ui.unlock.UnlockInvalidMountPointController"
@@ -23,10 +24,11 @@
 	<children>
 		<HBox spacing="12" VBox.vgrow="ALWAYS">
 			<StackPane alignment="CENTER" HBox.hgrow="NEVER">
-				<Circle styleClass="glyph-icon-primary" radius="24"/>
-				<FontAwesome5IconView styleClass="glyph-icon-white" glyph="EXCLAMATION" glyphSize="24"/>
+				<Circle styleClass="glyph-icon-red" radius="24"/>
+				<FontAwesome5IconView styleClass="glyph-icon-white" glyph="TIMES" glyphSize="24"/>
 			</StackPane>
 			<VBox spacing="6" HBox.hgrow="ALWAYS">
+				<Text text="%unlock.error.heading" styleClass="label-large"/>
 				<FormattedLabel visible="${controller.mustExist}" managed="${controller.mustExist}" format="%unlock.error.invalidMountPoint.notExisting" arg1="${controller.mountPoint}" wrapText="true"/>
 				<FormattedLabel visible="${!controller.mustExist}" managed="${!controller.mustExist}" format="%unlock.error.invalidMountPoint.existing" arg1="${controller.mountPoint}" wrapText="true"/>
 			</VBox>

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