From 6eca8f2e0c84c574d1055856c2ef00cd4d04422e Mon Sep 17 00:00:00 2001 From: JaniruTEC Date: Tue, 17 Nov 2020 21:56:43 +0100 Subject: [PATCH 01/14] Added support for quote escaped values for mount arguments --- .../cryptomator/common/vaults/FuseVolume.java | 33 +++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java index 76a83983b..2c84b901e 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java @@ -1,6 +1,5 @@ package org.cryptomator.common.vaults; -import com.google.common.base.Splitter; import org.apache.commons.lang3.SystemUtils; import org.cryptomator.common.mountpoint.InvalidMountPointException; import org.cryptomator.common.mountpoint.MountPointChooser; @@ -18,12 +17,19 @@ import org.slf4j.LoggerFactory; import javax.inject.Inject; import javax.inject.Named; import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; import java.util.SortedSet; +import java.util.regex.MatchResult; +import java.util.regex.Pattern; +import java.util.stream.Collectors; public class FuseVolume extends AbstractVolume { private static final Logger LOG = LoggerFactory.getLogger(FuseVolume.class); + private static final Pattern pattern = Pattern.compile("[^\\s\"']+|\"([^\"]*)\"|'([^']*)'"); //Thanks to https://stackoverflow.com/a/366532 + private Mount mount; @Inject @@ -51,7 +57,30 @@ public class FuseVolume extends AbstractVolume { } private String[] splitFlags(String str) { - return Splitter.on(' ').splitToList(str).toArray(String[]::new); + List strings = new ArrayList<>(); + List results = pattern.matcher(str).results().collect(Collectors.toList()); + for (int i = 0; i < results.size(); i++) { + MatchResult current = results.get(i); + MatchResult next = i + 1 < results.size() ? results.get(i + 1) : null; + if (getSpecialString(next) != null) { + //"next" is a quoted argument + //--> "next" must be joined with "current" and is skipped in the regular iteration + strings.add(current.group() + getSpecialString(next)); + i++; + } else { + //"next" is a normal unquoted string + //--> Add "current" and advance + strings.add(current.group()); + } + } + return strings.toArray(new String[0]); + } + + private String getSpecialString(MatchResult result) { + if (result == null) { + return null; + } + return result.group(1) != null ? result.group(1) : result.group(2); } @Override From 947e0e2369c696771c9975ee982fde362982c408 Mon Sep 17 00:00:00 2001 From: JaniruTEC Date: Tue, 17 Nov 2020 22:00:41 +0100 Subject: [PATCH 02/14] Fixed #1408 --- .../main/java/org/cryptomator/common/vaults/VaultModule.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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..e9fe26957 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 @@ -101,7 +101,7 @@ public class VaultModule { if (readOnly.get()) { flags.append(" -ordonly"); } - flags.append(" -ovolname=").append(mountName.get()); + flags.append(" -ovolname=").append('"').append(mountName.get()).append('"'); flags.append(" -oatomic_o_trunc"); flags.append(" -oauto_xattr"); flags.append(" -oauto_cache"); @@ -158,7 +158,7 @@ public class VaultModule { flags.append(" -ouid=-1"); flags.append(" -ogid=-1"); } - flags.append(" -ovolname=").append(mountName.get()); + flags.append(" -ovolname=").append('"').append(mountName.get()).append('"'); //Dokany requires this option to be set, WinFSP doesn't seem to share this peculiarity, //but the option exists. Let's keep this here in case we need it. // flags.append(" -oThreadCount=").append(5); From c19c3754c67075073f0ea4e3c196305041e15ee1 Mon Sep 17 00:00:00 2001 From: JaniruTEC Date: Tue, 17 Nov 2020 22:27:41 +0100 Subject: [PATCH 03/14] Fixed high-spirited concatenation --- .../main/java/org/cryptomator/common/vaults/FuseVolume.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java index 2c84b901e..4d99bf494 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java @@ -62,13 +62,13 @@ public class FuseVolume extends AbstractVolume { for (int i = 0; i < results.size(); i++) { MatchResult current = results.get(i); MatchResult next = i + 1 < results.size() ? results.get(i + 1) : null; - if (getSpecialString(next) != null) { - //"next" is a quoted argument + if (getSpecialString(next) != null && current.group().endsWith("=")) { + //"next" is a quoted elements and "current" is missing it's argument //--> "next" must be joined with "current" and is skipped in the regular iteration strings.add(current.group() + getSpecialString(next)); i++; } else { - //"next" is a normal unquoted string + //"next" is a normal unquoted string/is not missing from "current" //--> Add "current" and advance strings.add(current.group()); } From d9c5d7641738f6573fb2e40a67b09d3689f1ddde Mon Sep 17 00:00:00 2001 From: JaniruTEC Date: Tue, 17 Nov 2020 22:41:41 +0100 Subject: [PATCH 04/14] Fixed irregular quoting --- .../cryptomator/common/vaults/FuseVolume.java | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java index 4d99bf494..82095d9be 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java @@ -60,27 +60,34 @@ public class FuseVolume extends AbstractVolume { List strings = new ArrayList<>(); List results = pattern.matcher(str).results().collect(Collectors.toList()); for (int i = 0; i < results.size(); i++) { - MatchResult current = results.get(i); + String current = group(results.get(i), false); MatchResult next = i + 1 < results.size() ? results.get(i + 1) : null; - if (getSpecialString(next) != null && current.group().endsWith("=")) { + + if (group(next, true) != null && current.endsWith("=")) { //"next" is a quoted elements and "current" is missing it's argument //--> "next" must be joined with "current" and is skipped in the regular iteration - strings.add(current.group() + getSpecialString(next)); + strings.add(current + group(next, true)); i++; } else { //"next" is a normal unquoted string/is not missing from "current" //--> Add "current" and advance - strings.add(current.group()); + strings.add(current); } } return strings.toArray(new String[0]); } - private String getSpecialString(MatchResult result) { + private String group(MatchResult result, boolean onlyMatchQuoted) { if (result == null) { return null; } - return result.group(1) != null ? result.group(1) : result.group(2); + if (result.group(1) != null) { + return result.group(1); + } + if (result.group(2) != null || onlyMatchQuoted) { + return result.group(2); + } + return result.group(); } @Override From 9083976989ace707d50cf7cdeadf210d54a0dd36 Mon Sep 17 00:00:00 2001 From: JaniruTEC Date: Wed, 18 Nov 2020 00:46:24 +0100 Subject: [PATCH 05/14] Fixed #1409 by addig an additonal check --- .../common/mountpoint/IrregularUnmountCleaner.java | 7 +++++++ 1 file changed, 7 insertions(+) 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 d91fd0041..8be740fc3 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 @@ -48,6 +48,7 @@ public class IrregularUnmountCleaner { private static void deleteEmptyDir(Path dir) throws IOException { assert Files.isDirectory(dir, LinkOption.NOFOLLOW_LINKS); try { + checkEmpty(dir); 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); @@ -61,4 +62,10 @@ public class IrregularUnmountCleaner { } } + private static void checkEmpty(Path dir) throws IOException { + if(Files.newDirectoryStream(dir).iterator().hasNext()) { + throw new DirectoryNotEmptyException(dir.toString()); + } + } + } From 86dec80726604ea01304d0bd2053adb71c07d8a0 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 18 Nov 2020 10:33:04 +0100 Subject: [PATCH 06/14] avoid weird iteration counter manipulation --- .../cryptomator/common/vaults/FuseVolume.java | 48 ++++++------------- 1 file changed, 15 insertions(+), 33 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java index 82095d9be..f8defc62e 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java @@ -1,5 +1,6 @@ package org.cryptomator.common.vaults; +import com.google.common.collect.Iterators; import org.apache.commons.lang3.SystemUtils; import org.cryptomator.common.mountpoint.InvalidMountPointException; import org.cryptomator.common.mountpoint.MountPointChooser; @@ -20,15 +21,12 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.List; import java.util.SortedSet; -import java.util.regex.MatchResult; import java.util.regex.Pattern; -import java.util.stream.Collectors; public class FuseVolume extends AbstractVolume { private static final Logger LOG = LoggerFactory.getLogger(FuseVolume.class); - - private static final Pattern pattern = Pattern.compile("[^\\s\"']+|\"([^\"]*)\"|'([^']*)'"); //Thanks to https://stackoverflow.com/a/366532 + private static final Pattern NON_WHITESPACE_OR_QUOTED = Pattern.compile("[^\\s\"']+|\"([^\"]*)\"|'([^']*)'"); // Thanks to https://stackoverflow.com/a/366532 private Mount mount; @@ -57,37 +55,21 @@ public class FuseVolume extends AbstractVolume { } private String[] splitFlags(String str) { - List strings = new ArrayList<>(); - List results = pattern.matcher(str).results().collect(Collectors.toList()); - for (int i = 0; i < results.size(); i++) { - String current = group(results.get(i), false); - MatchResult next = i + 1 < results.size() ? results.get(i + 1) : null; - - if (group(next, true) != null && current.endsWith("=")) { - //"next" is a quoted elements and "current" is missing it's argument - //--> "next" must be joined with "current" and is skipped in the regular iteration - strings.add(current + group(next, true)); - i++; - } else { - //"next" is a normal unquoted string/is not missing from "current" - //--> Add "current" and advance - strings.add(current); + List flags = new ArrayList<>(); + var matches = Iterators.peekingIterator(NON_WHITESPACE_OR_QUOTED.matcher(str).results().iterator()); + while (matches.hasNext()) { + String flag = matches.next().group(); + // check if flag is missing its argument: + if (flag.endsWith("=") && matches.hasNext() && matches.peek().group(1) != null) { // next is "double quoted" + // next is "double quoted" and flag is missing its argument + flag += matches.next().group(1); + } else if (flag.endsWith("=") && matches.hasNext() && matches.peek().group(2) != null) { + // next is 'single quoted' and flag is missing its argument + flag += matches.next().group(2); } + flags.add(flag); } - return strings.toArray(new String[0]); - } - - private String group(MatchResult result, boolean onlyMatchQuoted) { - if (result == null) { - return null; - } - if (result.group(1) != null) { - return result.group(1); - } - if (result.group(2) != null || onlyMatchQuoted) { - return result.group(2); - } - return result.group(); + return flags.toArray(String[]::new); } @Override From c6d1c2ca6b1ef9c3f5db02cfd72f98602de3a5bc Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 18 Nov 2020 13:05:56 +0100 Subject: [PATCH 07/14] added MacVolumeMountChooser and refactored "priority" of mount point choosers: now all priorities are set in MountPointChooserModule (as a map key) --- .../AvailableDriveLetterChooser.java | 9 +-- .../mountpoint/CustomDriveLetterChooser.java | 9 +-- .../mountpoint/CustomMountPointChooser.java | 8 +-- .../mountpoint/MacVolumeMountChooser.java | 64 +++++++++++++++++++ .../common/mountpoint/MountPointChooser.java | 33 +--------- .../mountpoint/MountPointChooserModule.java | 34 ++++++---- .../TemporaryMountPointChooser.java | 15 +---- .../common/vaults/AbstractVolume.java | 29 ++------- .../common/vaults/DokanyVolume.java | 2 +- .../cryptomator/common/vaults/FuseVolume.java | 2 +- 10 files changed, 101 insertions(+), 104 deletions(-) create mode 100644 main/commons/src/main/java/org/cryptomator/common/mountpoint/MacVolumeMountChooser.java diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/AvailableDriveLetterChooser.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/AvailableDriveLetterChooser.java index 7634601fa..d0fc04f3c 100644 --- a/main/commons/src/main/java/org/cryptomator/common/mountpoint/AvailableDriveLetterChooser.java +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/AvailableDriveLetterChooser.java @@ -8,9 +8,7 @@ import javax.inject.Inject; import java.nio.file.Path; import java.util.Optional; -public class AvailableDriveLetterChooser implements MountPointChooser { - - public static final int PRIORITY = 200; +class AvailableDriveLetterChooser implements MountPointChooser { private final WindowsDriveLetters windowsDriveLetters; @@ -28,9 +26,4 @@ public class AvailableDriveLetterChooser implements MountPointChooser { public Optional chooseMountPoint(Volume caller) { return this.windowsDriveLetters.getAvailableDriveLetterPath(); } - - @Override - public int getPriority() { - return PRIORITY; - } } diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/CustomDriveLetterChooser.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/CustomDriveLetterChooser.java index ac9e5b716..1a42aa5ad 100644 --- a/main/commons/src/main/java/org/cryptomator/common/mountpoint/CustomDriveLetterChooser.java +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/CustomDriveLetterChooser.java @@ -9,9 +9,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.Optional; -public class CustomDriveLetterChooser implements MountPointChooser { - - public static final int PRIORITY = 100; +class CustomDriveLetterChooser implements MountPointChooser { private final VaultSettings vaultSettings; @@ -29,9 +27,4 @@ public class CustomDriveLetterChooser implements MountPointChooser { public Optional chooseMountPoint(Volume caller) { return this.vaultSettings.getWinDriveLetter().map(letter -> letter.charAt(0) + ":\\").map(Paths::get); } - - @Override - public int getPriority() { - return PRIORITY; - } } diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/CustomMountPointChooser.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/CustomMountPointChooser.java index 6a86c654a..5f1a7fedd 100644 --- a/main/commons/src/main/java/org/cryptomator/common/mountpoint/CustomMountPointChooser.java +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/CustomMountPointChooser.java @@ -20,9 +20,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.Optional; -public class CustomMountPointChooser implements MountPointChooser { - - public static final int PRIORITY = 0; +class CustomMountPointChooser implements MountPointChooser { private static final Logger LOG = LoggerFactory.getLogger(CustomMountPointChooser.class); @@ -94,8 +92,4 @@ public class CustomMountPointChooser implements MountPointChooser { } } - @Override - public int getPriority() { - return PRIORITY; - } } diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/MacVolumeMountChooser.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/MacVolumeMountChooser.java new file mode 100644 index 000000000..d6ebb4da5 --- /dev/null +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/MacVolumeMountChooser.java @@ -0,0 +1,64 @@ +package org.cryptomator.common.mountpoint; + +import org.apache.commons.lang3.SystemUtils; +import org.cryptomator.common.settings.VaultSettings; +import org.cryptomator.common.vaults.Volume; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.inject.Inject; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Optional; + +class MacVolumeMountChooser implements MountPointChooser { + + private static final Logger LOG = LoggerFactory.getLogger(MacVolumeMountChooser.class); + private static final int MAX_MOUNTPOINT_CREATION_RETRIES = 10; + private static final Path VOLUME_PATH = Path.of("/Volumes"); + + private final VaultSettings vaultSettings; + + @Inject + public MacVolumeMountChooser(VaultSettings vaultSettings) { + this.vaultSettings = vaultSettings; + } + + @Override + public boolean isApplicable(Volume caller) { + return SystemUtils.IS_OS_MAC; + } + + @Override + public Optional chooseMountPoint(Volume caller) { + String basename = this.vaultSettings.mountName().get(); + // regular + Path mountPoint = VOLUME_PATH.resolve(basename); + if (Files.notExists(mountPoint)) { + return Optional.of(mountPoint); + } + // with id + mountPoint = VOLUME_PATH.resolve(basename + " (" + vaultSettings.getId() + ")"); + if (Files.notExists(mountPoint)) { + return Optional.of(mountPoint); + } + // with id and count + for (int i = 1; i < MAX_MOUNTPOINT_CREATION_RETRIES; i++) { + mountPoint = VOLUME_PATH.resolve(basename + "_(" + vaultSettings.getId() + ")_" + i); + if (Files.notExists(mountPoint)) { + return Optional.of(mountPoint); + } + } + LOG.error("Failed to find feasible mountpoint at /Volumes/{}_x. Giving up after {} attempts.", basename, MAX_MOUNTPOINT_CREATION_RETRIES); + return Optional.empty(); + } + + @Override + public boolean prepare(Volume caller, Path mountPoint) { + // https://github.com/osxfuse/osxfuse/issues/306#issuecomment-245114592: + // In order to allow non-admin users to mount FUSE volumes in `/Volumes`, + // starting with version 3.5.0, FUSE will create non-existent mount points automatically. + // Therefore we don't need to prepare anything. + return false; + } +} diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/MountPointChooser.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/MountPointChooser.java index 2c40bb74a..3a4d82cf3 100644 --- a/main/commons/src/main/java/org/cryptomator/common/mountpoint/MountPointChooser.java +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/MountPointChooser.java @@ -47,7 +47,7 @@ import java.util.SortedSet; * If the preparation succeeds {@link #cleanup(Volume, Path)} can be used after unmount to do any * remaining cleanup. */ -public interface MountPointChooser extends Comparable { +public interface MountPointChooser { /** * Called by the {@link Volume} to determine whether this MountPointChooser is @@ -135,35 +135,4 @@ public interface MountPointChooser extends Comparable { //NO-OP } - /** - * Called by the {@link MountPointChooserModule} to sort the available MPCs - * and determine their execution order. - * The priority must be defined by the developer to reflect a useful execution order. - * MPCs with lower priorities will be placed at lower indices in the resulting - * {@link SortedSet} and will be executed with higher probability.
- * A specific priority must not be assigned to more than one MPC at a time; - * the result of having two MPCs with equal priority is undefined. - * - * @return the priority of this MPC. - */ - int getPriority(); - - /** - * Called by the {@link Volume} to determine the execution order of the registered MPCs. - * Implementations usually may not override this method. This default implementation - * sorts the MPCs in ascending order of their {@link #getPriority() priority.}
- *
- * Original description: - *

{@inheritDoc} - * - * @implNote This default implementation sorts the MPCs in ascending order - * of their {@link #getPriority() priority.} - */ - @Override - default int compareTo(MountPointChooser other) { - Preconditions.checkNotNull(other, "Other must not be null!"); - - //Sort by priority (ascending order) - return Integer.compare(this.getPriority(), other.getPriority()); - } } diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/MountPointChooserModule.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/MountPointChooserModule.java index 09789dacb..cfb7b68ec 100644 --- a/main/commons/src/main/java/org/cryptomator/common/mountpoint/MountPointChooserModule.java +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/MountPointChooserModule.java @@ -1,15 +1,17 @@ package org.cryptomator.common.mountpoint; -import com.google.common.collect.ImmutableSortedSet; +import com.google.common.collect.Iterables; import dagger.Binds; import dagger.Module; import dagger.Provides; -import dagger.multibindings.IntoSet; +import dagger.multibindings.IntKey; +import dagger.multibindings.IntoMap; import org.cryptomator.common.vaults.PerVault; import javax.inject.Named; -import java.util.Set; -import java.util.SortedSet; +import java.util.Map; +import java.util.SortedMap; +import java.util.TreeMap; /** * Dagger-Module for {@link MountPointChooser MountPointChoosers.}
@@ -21,30 +23,40 @@ import java.util.SortedSet; public abstract class MountPointChooserModule { @Binds - @IntoSet + @IntoMap + @IntKey(0) @PerVault public abstract MountPointChooser bindCustomMountPointChooser(CustomMountPointChooser chooser); @Binds - @IntoSet + @IntoMap + @IntKey(100) @PerVault public abstract MountPointChooser bindCustomDriveLetterChooser(CustomDriveLetterChooser chooser); @Binds - @IntoSet + @IntoMap + @IntKey(101) + @PerVault + public abstract MountPointChooser bindMacVolumeMountChooser(MacVolumeMountChooser chooser); + + @Binds + @IntoMap + @IntKey(200) @PerVault public abstract MountPointChooser bindAvailableDriveLetterChooser(AvailableDriveLetterChooser chooser); @Binds - @IntoSet + @IntoMap + @IntKey(999) @PerVault public abstract MountPointChooser bindTemporaryMountPointChooser(TemporaryMountPointChooser chooser); @Provides @PerVault @Named("orderedMountPointChoosers") - public static SortedSet provideOrderedMountPointChoosers(Set choosers) { - //Sort by natural order. The natural order is defined by MountPointChooser#compareTo - return ImmutableSortedSet.copyOf(choosers); + public static Iterable provideOrderedMountPointChoosers(Map choosers) { + SortedMap sortedChoosers = new TreeMap<>(choosers); + return Iterables.unmodifiableIterable(sortedChoosers.values()); } } 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 17c960b1e..9b2d502d9 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 @@ -15,9 +15,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.Optional; -public class TemporaryMountPointChooser implements MountPointChooser { - - public static final int PRIORITY = 300; +class TemporaryMountPointChooser implements MountPointChooser { private static final Logger LOG = LoggerFactory.getLogger(TemporaryMountPointChooser.class); private static final int MAX_TMPMOUNTPOINT_CREATION_RETRIES = 10; @@ -70,13 +68,6 @@ public class TemporaryMountPointChooser implements MountPointChooser { @Override public boolean prepare(Volume caller, Path mountPoint) throws InvalidMountPointException { - // https://github.com/osxfuse/osxfuse/issues/306#issuecomment-245114592: - // In order to allow non-admin users to mount FUSE volumes in `/Volumes`, - // starting with version 3.5.0, FUSE will create non-existent mount points automatically. - if (SystemUtils.IS_OS_MAC && mountPoint.getParent().equals(Paths.get("/Volumes"))) { - return false; - } - try { switch (caller.getMountPointRequirement()) { case PARENT_NO_MOUNT_POINT -> { @@ -114,8 +105,4 @@ public class TemporaryMountPointChooser implements MountPointChooser { } } - @Override - public int getPriority() { - return PRIORITY; - } } diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/AbstractVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/AbstractVolume.java index 91c6e22e4..6a09683ca 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/AbstractVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/AbstractVolume.java @@ -1,50 +1,35 @@ package org.cryptomator.common.vaults; -import com.google.common.base.Joiner; -import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import org.cryptomator.common.mountpoint.InvalidMountPointException; import org.cryptomator.common.mountpoint.MountPointChooser; import java.nio.file.Path; import java.util.Optional; -import java.util.SortedSet; -import java.util.TreeSet; public abstract class AbstractVolume implements Volume { - private final SortedSet choosers; + private final Iterable choosers; protected Path mountPoint; - - //Cleanup private boolean cleanupRequired; private MountPointChooser usedChooser; - public AbstractVolume(SortedSet choosers) { + public AbstractVolume(Iterable choosers) { this.choosers = choosers; } protected Path determineMountPoint() throws InvalidMountPointException { - SortedSet checkedChoosers = new TreeSet<>(); //Natural order - for (MountPointChooser chooser : this.choosers) { - if (!chooser.isApplicable(this)) { - continue; - } - + for (var chooser : Iterables.filter(choosers, c -> c.isApplicable(this))) { Optional chosenPath = chooser.chooseMountPoint(this); - checkedChoosers.add(chooser); //Consider a chooser checked if it's #chooseMountPoint() method was called - if (chosenPath.isEmpty()) { - //Chooser was applicable, but couldn't find a feasible mountpoint + if (chosenPath.isEmpty()) { // chooser couldn't find a feasible mountpoint continue; } - this.cleanupRequired = chooser.prepare(this, chosenPath.get()); //Fail entirely if an Exception occurs + this.cleanupRequired = chooser.prepare(this, chosenPath.get()); this.usedChooser = chooser; return chosenPath.get(); } - //SortedSet#stream() should return a sorted stream (that's what it's docs and the docs of #spliterator() say, even if they are not 100% clear for me.) - //We want to keep that order, that's why we use ImmutableSet#toImmutableSet() to collect (even if it doesn't implement SortedSet, it's docs promise use encounter ordering.) - String checked = Joiner.on(", ").join(checkedChoosers.stream().map((mpc) -> mpc.getClass().getTypeName()).collect(ImmutableSet.toImmutableSet())); - throw new InvalidMountPointException(String.format("No feasible MountPoint found! Checked %s", checked.isBlank() ? "" : checked)); + throw new InvalidMountPointException("No feasible MountPoint found!"); } protected void cleanupMountPoint() { 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 e91c9f86d..89fd73203 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 @@ -28,7 +28,7 @@ public class DokanyVolume extends AbstractVolume { private Mount mount; @Inject - public DokanyVolume(VaultSettings vaultSettings, ExecutorService executorService, @Named("orderedMountPointChoosers") SortedSet choosers) { + public DokanyVolume(VaultSettings vaultSettings, ExecutorService executorService, @Named("orderedMountPointChoosers") Iterable choosers) { super(choosers); this.vaultSettings = vaultSettings; this.mountFactory = new MountFactory(executorService); diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java index 76a83983b..74db3bd54 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java @@ -27,7 +27,7 @@ public class FuseVolume extends AbstractVolume { private Mount mount; @Inject - public FuseVolume(@Named("orderedMountPointChoosers") SortedSet choosers) { + public FuseVolume(@Named("orderedMountPointChoosers") Iterable choosers) { super(choosers); } From d3c2b0509efe6fdfb12c9b19935b0008a2fa1b1e Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 18 Nov 2020 13:08:48 +0100 Subject: [PATCH 08/14] Lazily call IrregularUnmountCleaner from (and only from!) TemporaryMountPointChooser --- .../mountpoint/IrregularUnmountCleaner.java | 9 +------- .../TemporaryMountPointChooser.java | 22 +++++++++++++++++-- .../cryptomator/ui/launcher/UiLauncher.java | 7 ------ 3 files changed, 21 insertions(+), 17 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 8be740fc3..a01e385d6 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 @@ -10,7 +10,7 @@ import java.nio.file.LinkOption; import java.nio.file.Path; import java.nio.file.attribute.BasicFileAttributes; -public class IrregularUnmountCleaner { +class IrregularUnmountCleaner { public static Logger LOG = LoggerFactory.getLogger(IrregularUnmountCleaner.class); @@ -48,7 +48,6 @@ public class IrregularUnmountCleaner { private static void deleteEmptyDir(Path dir) throws IOException { assert Files.isDirectory(dir, LinkOption.NOFOLLOW_LINKS); try { - checkEmpty(dir); 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); @@ -62,10 +61,4 @@ public class IrregularUnmountCleaner { } } - private static void checkEmpty(Path dir) throws IOException { - if(Files.newDirectoryStream(dir).iterator().hasNext()) { - throw new DirectoryNotEmptyException(dir.toString()); - } - } - } 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 9b2d502d9..0db2e7892 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 @@ -11,9 +11,11 @@ import javax.inject.Inject; import java.io.File; import java.io.IOException; import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; class TemporaryMountPointChooser implements MountPointChooser { @@ -22,6 +24,7 @@ class TemporaryMountPointChooser implements MountPointChooser { private final VaultSettings vaultSettings; private final Environment environment; + private volatile boolean clearedDebris; @Inject public TemporaryMountPointChooser(VaultSettings vaultSettings, Environment environment) { @@ -40,9 +43,24 @@ class TemporaryMountPointChooser implements MountPointChooser { @Override public Optional chooseMountPoint(Volume caller) { + clearDebrisIfNeeded(); return this.environment.getMountPointsDir().map(this::choose); } + //clean leftovers of not-regularly unmounted vaults + //see https://github.com/cryptomator/cryptomator/issues/1013 and https://github.com/cryptomator/cryptomator/issues/1061 + private synchronized void clearDebrisIfNeeded() { + assert environment.getMountPointsDir().isPresent(); + if (clearedDebris) { + return; // already cleared + } + Path mountPointDir = environment.getMountPointsDir().get(); + if (Files.exists(mountPointDir, LinkOption.NOFOLLOW_LINKS)) { + IrregularUnmountCleaner.removeIrregularUnmountDebris(mountPointDir); + } + clearedDebris = true; + } + private Path choose(Path parent) { String basename = this.vaultSettings.mountName().get(); //regular @@ -51,13 +69,13 @@ class TemporaryMountPointChooser implements MountPointChooser { return mountPoint; } //with id - mountPoint = parent.resolve(basename + " (" +vaultSettings.getId() + ")"); + 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); + mountPoint = parent.resolve(basename + "_(" + vaultSettings.getId() + ")_" + i); if (Files.notExists(mountPoint)) { return mountPoint; } 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 350a0a1ca..7b3f1d68d 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,6 @@ 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; @@ -16,8 +15,6 @@ 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,10 +62,6 @@ 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()) { From c90e445a67ac61b3ee4ebce1ffcce221d59580b5 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 18 Nov 2020 13:10:01 +0100 Subject: [PATCH 09/14] Removed -Dcryptomator.mountPointsDir="/Volumes/" from macOS run profiles [ci skip] --- .idea/runConfigurations/Cryptomator_macOS.xml | 3 +-- .idea/runConfigurations/Cryptomator_macOS_Dev.xml | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.idea/runConfigurations/Cryptomator_macOS.xml b/.idea/runConfigurations/Cryptomator_macOS.xml index 235887653..9f372f570 100644 --- a/.idea/runConfigurations/Cryptomator_macOS.xml +++ b/.idea/runConfigurations/Cryptomator_macOS.xml @@ -5,8 +5,7 @@