diff --git a/src/main/java/org/cryptomator/common/mountpoint/CustomMountPointChooser.java b/src/main/java/org/cryptomator/common/mountpoint/CustomMountPointChooser.java index c55ede640..80c5b067b 100644 --- a/src/main/java/org/cryptomator/common/mountpoint/CustomMountPointChooser.java +++ b/src/main/java/org/cryptomator/common/mountpoint/CustomMountPointChooser.java @@ -4,6 +4,7 @@ import org.apache.commons.lang3.SystemUtils; import org.cryptomator.common.Environment; import org.cryptomator.common.settings.VaultSettings; import org.cryptomator.common.settings.VolumeImpl; +import org.cryptomator.common.vaults.MountPointRequirement; import org.cryptomator.common.vaults.Volume; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -11,10 +12,10 @@ import org.slf4j.LoggerFactory; import javax.inject.Inject; import java.io.IOException; import java.nio.file.DirectoryNotEmptyException; -import java.nio.file.DirectoryStream; 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.Paths; @@ -22,6 +23,9 @@ import java.util.Optional; class CustomMountPointChooser implements MountPointChooser { + private static final String HIDEAWAY_PREFIX = ".~$"; + private static final String HIDEAWAY_SUFFIX = ".tmp"; + private static final String WIN_HIDDEN = "dos:hidden"; private static final Logger LOG = LoggerFactory.getLogger(CustomMountPointChooser.class); private final VaultSettings vaultSettings; @@ -35,7 +39,6 @@ class CustomMountPointChooser implements MountPointChooser { @Override public boolean isApplicable(Volume caller) { - //Disable if useExperimentalFuse is required (Win + Fuse), but set to false return caller.getImplementationType() != VolumeImpl.FUSE || !SystemUtils.IS_OS_WINDOWS || environment.useExperimentalFuse(); } @@ -47,49 +50,102 @@ class CustomMountPointChooser implements MountPointChooser { @Override public boolean prepare(Volume caller, Path mountPoint) throws InvalidMountPointException { - switch (caller.getMountPointRequirement()) { - case PARENT_NO_MOUNT_POINT -> prepareParentNoMountPoint(mountPoint); - case EMPTY_MOUNT_POINT -> prepareEmptyMountPoint(mountPoint); - case NONE -> { - //Requirement "NONE" doesn't make any sense here. - //No need to prepare/verify a Mountpoint without requiring one... + return switch (caller.getMountPointRequirement()) { + case PARENT_NO_MOUNT_POINT -> { + prepareParentNoMountPoint(mountPoint); + LOG.debug("Successfully checked custom mount point: {}", mountPoint); + yield true; + } + case EMPTY_MOUNT_POINT -> { + prepareEmptyMountPoint(mountPoint); + LOG.debug("Successfully checked custom mount point: {}", mountPoint); + yield false; + } + case NONE, UNUSED_ROOT_DIR, PARENT_OPT_MOUNT_POINT -> { throw new InvalidMountPointException(new IllegalStateException("Illegal MountPointRequirement")); } - default -> { - //Currently the case for "UNUSED_ROOT_DIR, PARENT_OPT_MOUNT_POINT" - throw new InvalidMountPointException(new IllegalStateException("Not implemented")); - } - } - LOG.debug("Successfully checked custom mount point: {}", mountPoint); - return false; + }; } - private void prepareParentNoMountPoint(Path mountPoint) throws InvalidMountPointException { - //This the case on Windows when using FUSE - //See https://github.com/billziss-gh/winfsp/issues/320 - Path parent = mountPoint.getParent(); - if (!Files.isDirectory(parent)) { - throw new InvalidMountPointException(new NotDirectoryException(parent.toString())); - } - //We must use #notExists() here because notExists =/= !exists (see docs) - if (!Files.notExists(mountPoint, LinkOption.NOFOLLOW_LINKS)) { - //File exists OR can't be determined - throw new InvalidMountPointException(new FileAlreadyExistsException(mountPoint.toString())); + //This is case on Windows when using FUSE + //See https://github.com/billziss-gh/winfsp/issues/320 + void prepareParentNoMountPoint(Path mountPoint) throws InvalidMountPointException { + 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 InvalidMountPointException(new FileAlreadyExistsException(hideaway.toString())); + } else if (!mpExists && !hideExists) { //neither mountpoint nor hideaway exist + throw new InvalidMountPointException(new NoSuchFileException(mountPoint.toString())); + } else if (!mpExists) { //only hideaway exists + checkIsDirectory(hideaway); + LOG.info("Mountpoint {} for winfsp mount seems to be not properly cleaned up. Will be fixed on unmount.", mountPoint); + try { + if (SystemUtils.IS_OS_WINDOWS) { + Files.setAttribute(hideaway, WIN_HIDDEN, true, LinkOption.NOFOLLOW_LINKS); + } + } catch (IOException e) { + throw new InvalidMountPointException(e); + } + } else { //only mountpoint exists + try { + checkIsDirectory(mountPoint); + checkIsEmpty(mountPoint); + + Files.move(mountPoint, hideaway); + if (SystemUtils.IS_OS_WINDOWS) { + Files.setAttribute(hideaway, WIN_HIDDEN, true, LinkOption.NOFOLLOW_LINKS); + } + } catch (IOException e) { + throw new InvalidMountPointException(e); + } } } private void prepareEmptyMountPoint(Path mountPoint) throws InvalidMountPointException { //This is the case for Windows when using Dokany and for Linux and Mac - if (!Files.isDirectory(mountPoint)) { - throw new InvalidMountPointException(new NotDirectoryException(mountPoint.toString())); - } - try (DirectoryStream ds = Files.newDirectoryStream(mountPoint)) { - if (ds.iterator().hasNext()) { - throw new InvalidMountPointException(new DirectoryNotEmptyException(mountPoint.toString())); - } + checkIsDirectory(mountPoint); + try { + checkIsEmpty(mountPoint); } catch (IOException exception) { throw new InvalidMountPointException("IOException while checking folder content", exception); } } + @Override + public void cleanup(Volume caller, Path mountPoint) { + if (caller.getMountPointRequirement() == MountPointRequirement.PARENT_NO_MOUNT_POINT) { + Path hideaway = getHideaway(mountPoint); + try { + Files.move(hideaway, mountPoint); + if (SystemUtils.IS_OS_WINDOWS) { + Files.setAttribute(mountPoint, WIN_HIDDEN, false); + } + } catch (IOException e) { + LOG.error("Unable to clean up mountpoint {} for Winfsp mounting.", mountPoint, e); + } + } + } + + private void checkIsDirectory(Path toCheck) throws InvalidMountPointException { + if (!Files.isDirectory(toCheck, LinkOption.NOFOLLOW_LINKS)) { + throw new InvalidMountPointException(new NotDirectoryException(toCheck.toString())); + } + } + + private void checkIsEmpty(Path toCheck) throws InvalidMountPointException, IOException { + try (var dirStream = Files.list(toCheck)) { + if (dirStream.findFirst().isPresent()) { + throw new InvalidMountPointException(new DirectoryNotEmptyException(toCheck.toString())); + } + } + } + + //visible for testing + Path getHideaway(Path mountPoint) { + return mountPoint.resolveSibling(HIDEAWAY_PREFIX + mountPoint.getFileName().toString() + HIDEAWAY_SUFFIX); + } + } diff --git a/src/main/java/org/cryptomator/ui/vaultoptions/MountOptionsController.java b/src/main/java/org/cryptomator/ui/vaultoptions/MountOptionsController.java index 8739be791..3be38568d 100644 --- a/src/main/java/org/cryptomator/ui/vaultoptions/MountOptionsController.java +++ b/src/main/java/org/cryptomator/ui/vaultoptions/MountOptionsController.java @@ -11,9 +11,6 @@ import org.cryptomator.ui.common.FxController; import javax.inject.Inject; import javafx.beans.binding.Bindings; -import javafx.beans.binding.BooleanBinding; -import javafx.beans.property.BooleanProperty; -import javafx.beans.property.SimpleBooleanProperty; import javafx.beans.property.StringProperty; import javafx.beans.value.ObservableValue; import javafx.fxml.FXML; @@ -32,18 +29,15 @@ import java.nio.file.Path; import java.util.ResourceBundle; import java.util.Set; -/** - * TODO: if WebDav is selected on a windows system, custom mount directory is _not_ supported. This is currently not indicated/shown/etc in the ui - */ @VaultOptionsScoped public class MountOptionsController implements FxController { private final Stage window; private final Vault vault; - private final BooleanProperty osIsWindows = new SimpleBooleanProperty(SystemUtils.IS_OS_WINDOWS); - private final BooleanBinding webDavAndWindows; + private final VolumeImpl usedVolumeImpl; private final WindowsDriveLetters windowsDriveLetters; private final ResourceBundle resourceBundle; + public CheckBox readOnlyCheckbox; public CheckBox customMountFlagsCheckbox; public TextField mountFlags; @@ -53,20 +47,13 @@ public class MountOptionsController implements FxController { public RadioButton mountPointCustomDir; public ChoiceBox driveLetterSelection; - //FUSE + Windows -> Disable some (experimental) features for the user because they are unstable - //Use argument Dfuse.experimental="true" to override - private final BooleanBinding restrictToStableFuseOnWindows; - @Inject MountOptionsController(@VaultOptionsWindow Stage window, @VaultOptionsWindow Vault vault, Settings settings, WindowsDriveLetters windowsDriveLetters, ResourceBundle resourceBundle, Environment environment) { this.window = window; this.vault = vault; - this.webDavAndWindows = settings.preferredVolumeImpl().isEqualTo(VolumeImpl.WEBDAV).and(osIsWindows); + this.usedVolumeImpl = settings.preferredVolumeImpl().get(); this.windowsDriveLetters = windowsDriveLetters; this.resourceBundle = resourceBundle; - - BooleanBinding isFuseOnWindows = settings.preferredVolumeImpl().isEqualTo(VolumeImpl.FUSE).and(osIsWindows); - this.restrictToStableFuseOnWindows = isFuseOnWindows.and(new SimpleBooleanProperty(!environment.useExperimentalFuse())); //Is FUSE on Win and is NOT experimental fuse enabled } @FXML @@ -74,10 +61,11 @@ public class MountOptionsController implements FxController { // readonly: readOnlyCheckbox.selectedProperty().bindBidirectional(vault.getVaultSettings().usesReadOnlyMode()); - if (getRestrictToStableFuseOnWindows()) { + //TODO: support this feature on Windows + if (usedVolumeImpl == VolumeImpl.FUSE && isOsWindows()) { readOnlyCheckbox.setSelected(false); // to prevent invalid states + readOnlyCheckbox.setDisable(true); } - readOnlyCheckbox.disableProperty().bind(customMountFlagsCheckbox.selectedProperty().or(restrictToStableFuseOnWindows)); // custom mount flags: mountFlags.disableProperty().bind(customMountFlagsCheckbox.selectedProperty().not()); @@ -95,9 +83,7 @@ public class MountOptionsController implements FxController { driveLetterSelection.setConverter(new WinDriveLetterLabelConverter(windowsDriveLetters, resourceBundle)); driveLetterSelection.setValue(vault.getVaultSettings().winDriveLetter().get()); - if (vault.getVaultSettings().useCustomMountPath().get() - && vault.getVaultSettings().getCustomMountPath().isPresent() - && !getRestrictToStableFuseOnWindows() /* to prevent invalid states */) { + if (vault.getVaultSettings().useCustomMountPath().get() && vault.getVaultSettings().getCustomMountPath().isPresent()) { mountPoint.selectToggle(mountPointCustomDir); } else if (!Strings.isNullOrEmpty(vault.getVaultSettings().winDriveLetter().get())) { mountPoint.selectToggle(mountPointWinDriveLetter); @@ -188,32 +174,28 @@ public class MountOptionsController implements FxController { // Getter & Setter - public BooleanProperty osIsWindowsProperty() { - return osIsWindows; + public boolean isOsWindows() { + return SystemUtils.IS_OS_WINDOWS; } - public boolean getOsIsWindows() { - return osIsWindows.get(); + public boolean isCustomMountPointSupported() { + return !(usedVolumeImpl == VolumeImpl.WEBDAV && isOsWindows()); } - public BooleanBinding webDavAndWindowsProperty() { - return webDavAndWindows; - } - - public boolean isWebDavAndWindows() { - return webDavAndWindows.get(); + public boolean isReadOnlySupported() { + return !(usedVolumeImpl == VolumeImpl.FUSE && isOsWindows()); } public StringProperty customMountPathProperty() { return vault.getVaultSettings().customMountPath(); } + public boolean isCustomMountOptionsSupported() { + return usedVolumeImpl != VolumeImpl.WEBDAV; + } + public String getCustomMountPath() { return vault.getVaultSettings().customMountPath().get(); } - public Boolean getRestrictToStableFuseOnWindows() { - return restrictToStableFuseOnWindows.get(); - } - } diff --git a/src/main/resources/fxml/vault_options_mount.fxml b/src/main/resources/fxml/vault_options_mount.fxml index a96784020..48b1fc7cd 100644 --- a/src/main/resources/fxml/vault_options_mount.fxml +++ b/src/main/resources/fxml/vault_options_mount.fxml @@ -24,7 +24,7 @@ - + @@ -38,19 +38,19 @@ - + - - + + - + diff --git a/src/test/java/org/cryptomator/common/mountpoint/CustomMountPointChooserTest.java b/src/test/java/org/cryptomator/common/mountpoint/CustomMountPointChooserTest.java new file mode 100644 index 000000000..da2e0fde0 --- /dev/null +++ b/src/test/java/org/cryptomator/common/mountpoint/CustomMountPointChooserTest.java @@ -0,0 +1,187 @@ +package org.cryptomator.common.mountpoint; + +import org.cryptomator.common.Environment; +import org.cryptomator.common.settings.VaultSettings; +import org.cryptomator.common.vaults.MountPointRequirement; +import org.cryptomator.common.vaults.Volume; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.OS; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +public class CustomMountPointChooserTest { + + //--- Mocks --- + VaultSettings vaultSettings; + Environment environment; + Volume volume; + + CustomMountPointChooser customMpc; + + + @BeforeEach + public void init() { + this.volume = Mockito.mock(Volume.class); + this.vaultSettings = Mockito.mock(VaultSettings.class); + this.environment = Mockito.mock(Environment.class); + this.customMpc = new CustomMountPointChooser(vaultSettings, environment); + } + + @Nested + public class WinfspPreperations { + + @Test + @DisplayName("Hideaway name for PARENT_NO_MOUNTPOINT is not the same as mountpoint") + public void testGetHideaway() { + //prepare + Path mntPoint = Path.of("/foo/bar"); + //execute + var hideaway = customMpc.getHideaway(mntPoint); + //eval + Assertions.assertNotEquals(hideaway.getFileName(), mntPoint.getFileName()); + Assertions.assertEquals(hideaway.getParent(), mntPoint.getParent()); + Assertions.assertTrue(hideaway.getFileName().toString().contains(mntPoint.getFileName().toString())); + } + + @Test + @DisplayName("PARENT_NO_MOUNTPOINT preparations succeeds, if only mountpoint is present") + public void testPrepareParentNoMountpointOnlyMountpoint(@TempDir Path tmpDir) throws IOException { + //prepare + var mntPoint = tmpDir.resolve("mntPoint"); + Files.createDirectory(mntPoint); + + //execute + Assertions.assertDoesNotThrow(() -> customMpc.prepareParentNoMountPoint(mntPoint)); + + //evaluate + Assertions.assertTrue(Files.notExists(mntPoint)); + + Path hideaway = customMpc.getHideaway(mntPoint); + Assertions.assertTrue(Files.exists(hideaway)); + + if(OS.WINDOWS.isCurrentOs()) { + Assertions.assertTrue((Boolean) Files.getAttribute(hideaway, "dos:hidden")); + } + } + + @Test + @DisplayName("PARENT_NO_MOUNTPOINT preparations fail, if only non-empty mountpoint is present") + public void testPrepareParentNoMountpointOnlyNonEmptyMountpoint(@TempDir Path tmpDir) throws IOException { + //prepare + var mntPoint = tmpDir.resolve("mntPoint"); + Files.createDirectory(mntPoint); + Files.createFile(mntPoint.resolve("foo")); + + //execute + Assertions.assertThrows(InvalidMountPointException.class, () -> customMpc.prepareParentNoMountPoint(mntPoint)); + + //evaluate + Assertions.assertTrue(Files.exists(mntPoint.resolve("foo"))); + } + + @Test + @DisplayName("PARENT_NO_MOUNTPOINT preparation succeeds, if for any reason only hideaway dir is present") + public void testPrepareParentNoMountpointOnlyHideaway(@TempDir Path tmpDir) throws IOException { + //prepare + var mntPoint = tmpDir.resolve("mntPoint"); + var hideaway = customMpc.getHideaway(mntPoint); + Files.createDirectory(hideaway); //we explicitly do not set the file attributes here + + //execute + Assertions.assertDoesNotThrow(() -> customMpc.prepareParentNoMountPoint(mntPoint)); + + //evaluate + Assertions.assertTrue(Files.exists(hideaway)); + + if(OS.WINDOWS.isCurrentOs()) { + Assertions.assertTrue((Boolean) Files.getAttribute(hideaway, "dos:hidden")); + } + } + + @Test + @DisplayName("PARENT_NO_MOUNTPOINT preparation fails, if mountpoint and hideaway dirs are present") + public void testPrepareParentNoMountpointMountPointAndHideaway(@TempDir Path tmpDir) throws IOException { + //prepare + var mntPoint = tmpDir.resolve("mntPoint"); + var hideaway = customMpc.getHideaway(mntPoint); + Files.createDirectory(hideaway); //we explicitly do not set the file attributes here + Files.createDirectory(mntPoint); + + //execute + Assertions.assertThrows(InvalidMountPointException.class, () -> customMpc.prepareParentNoMountPoint(mntPoint)); + + //evaluate + Assertions.assertTrue(Files.exists(hideaway)); + Assertions.assertTrue(Files.exists(mntPoint)); + + if(OS.WINDOWS.isCurrentOs()) { + Assertions.assertFalse((Boolean) Files.getAttribute(hideaway, "dos:hidden")); + } + } + + @Test + @DisplayName("PARENT_NO_MOUNTPOINT preparation fails, if neither mountpoint nor hideaway dir is present") + public void testPrepareParentNoMountpointNothing(@TempDir Path tmpDir) { + //prepare + var mntPoint = tmpDir.resolve("mntPoint"); + var hideaway = customMpc.getHideaway(mntPoint); + + //execute + Assertions.assertThrows(InvalidMountPointException.class, () -> customMpc.prepareParentNoMountPoint(mntPoint)); + + //evaluate + Assertions.assertTrue(Files.notExists(hideaway)); + Assertions.assertTrue(Files.notExists(mntPoint)); + } + + @Test + @DisplayName("Normal Cleanup for PARENT_NO_MOUNTPOINT") + public void testCleanupSuccess(@TempDir Path tmpDir) throws IOException { + //prepare + var mntPoint = tmpDir.resolve("mntPoint"); + var hideaway = customMpc.getHideaway(mntPoint); + + Files.createDirectory(hideaway); + Mockito.when(volume.getMountPointRequirement()).thenReturn(MountPointRequirement.PARENT_NO_MOUNT_POINT); + + //execute + Assertions.assertDoesNotThrow(() -> customMpc.cleanup(volume, mntPoint)); + + //evaluate + Assertions.assertTrue(Files.exists(mntPoint)); + Assertions.assertTrue(Files.notExists(hideaway)); + + if(OS.WINDOWS.isCurrentOs()) { + Assertions.assertFalse((Boolean) Files.getAttribute(mntPoint, "dos:hidden")); + } + } + + @Test + @DisplayName("On IOException cleanup for PARENT_NO_MOUNTPOINT exits normally") + public void testCleanupIOFailure(@TempDir Path tmpDir) throws IOException { + //prepare + var mntPoint = tmpDir.resolve("mntPoint"); + var hideaway = customMpc.getHideaway(mntPoint); + + Files.createDirectory(hideaway); + Mockito.when(volume.getMountPointRequirement()).thenReturn(MountPointRequirement.PARENT_NO_MOUNT_POINT); + try (MockedStatic filesMock = Mockito.mockStatic(Files.class)) { + filesMock.when(() -> Files.move(Mockito.any(), Mockito.any(), Mockito.any())).thenThrow(new IOException("error")); + //execute + Assertions.assertDoesNotThrow(() -> customMpc.cleanup(volume, mntPoint)); + } + } + + } + + +}