Merge pull request #268 from cryptomator/feature/refactor-filesystem-checker

Feature: Refactor filesystem checker

Author: infeo

src/main/java/org/cryptomator/cryptofs/CryptoFileSystems.java

@@ -34,13 +34,11 @@ class CryptoFileSystems {
 
 	private final ConcurrentMap<Path, CryptoFileSystemImpl> fileSystems = new ConcurrentHashMap<>();
 	private final CryptoFileSystemComponent.Factory cryptoFileSystemComponentFactory;
-	private final FileSystemCapabilityChecker capabilityChecker;
 	private final SecureRandom csprng;
 
 	@Inject
-	public CryptoFileSystems(CryptoFileSystemComponent.Factory cryptoFileSystemComponentFactory, FileSystemCapabilityChecker capabilityChecker, SecureRandom csprng) {
+	public CryptoFileSystems(CryptoFileSystemComponent.Factory cryptoFileSystemComponentFactory, SecureRandom csprng) {
 		this.cryptoFileSystemComponentFactory = cryptoFileSystemComponentFactory;
-		this.capabilityChecker = capabilityChecker;
 		this.csprng = csprng;
 	}
 
@@ -53,13 +51,12 @@ public CryptoFileSystemImpl create(CryptoFileSystemProvider provider, Path pathT
 		try (Masterkey key = properties.keyLoader().loadKey(keyId)) {
 			var config = configLoader.verify(key.getEncoded(), Constants.VAULT_VERSION);
 			backupVaultConfigFile(normalizedPathToVault, properties);
-			var adjustedProperties = adjustForCapabilities(pathToVault, properties);
 			var cryptor = CryptorProvider.forScheme(config.getCipherCombo()).provide(key.copy(), csprng);
 			try {
 				checkVaultRootExistence(pathToVault, cryptor);
 				return fileSystems.compute(normalizedPathToVault, (path, fs) -> {
 					if (fs == null) {
-						return cryptoFileSystemComponentFactory.create(cryptor, config, provider, normalizedPathToVault, adjustedProperties).cryptoFileSystem();
+						return cryptoFileSystemComponentFactory.create(cryptor, config, provider, normalizedPathToVault, properties).cryptoFileSystem();
 					} else {
 						throw new FileSystemAlreadyExistsException();
 					}
@@ -123,23 +120,6 @@ private void backupVaultConfigFile(Path pathToVault, CryptoFileSystemProperties
 		BackupHelper.attemptBackup(vaultConfigFile);
 	}
 
-	private CryptoFileSystemProperties adjustForCapabilities(Path pathToVault, CryptoFileSystemProperties originalProperties) throws FileSystemCapabilityChecker.MissingCapabilityException {
-		if (!originalProperties.readonly()) {
-			try {
-				capabilityChecker.assertWriteAccess(pathToVault);
-				return originalProperties;
-			} catch (FileSystemCapabilityChecker.MissingCapabilityException e) {
-				capabilityChecker.assertReadAccess(pathToVault);
-				LOG.warn("No write access to vault. Fallback to read-only access.");
-				Set<CryptoFileSystemProperties.FileSystemFlags> flags = EnumSet.copyOf(originalProperties.flags());
-				flags.add(CryptoFileSystemProperties.FileSystemFlags.READONLY);
-				return CryptoFileSystemProperties.cryptoFileSystemPropertiesFrom(originalProperties).withFlags(flags).build();
-			}
-		} else {
-			return originalProperties;
-		}
-	}
-
 	public void remove(CryptoFileSystemImpl cryptoFileSystem) {
 		fileSystems.values().remove(cryptoFileSystem);
 	}

src/main/java/org/cryptomator/cryptofs/common/FileSystemCapabilityChecker.java

@@ -7,8 +7,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.inject.Inject;
-import javax.inject.Singleton;
 import java.io.IOException;
 import java.nio.file.DirectoryIteratorException;
 import java.nio.file.DirectoryStream;
@@ -17,8 +15,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 
-@Singleton
-public class FileSystemCapabilityChecker {
+public final class FileSystemCapabilityChecker {
 
 	private static final Logger LOG = LoggerFactory.getLogger(FileSystemCapabilityChecker.class);
 
@@ -38,8 +35,8 @@ public enum Capability {
 		WRITE_ACCESS,
 	}
 
-	@Inject
-	public FileSystemCapabilityChecker() {
+	private FileSystemCapabilityChecker() {
+
 	}
 
 	/**
@@ -50,7 +47,7 @@ public FileSystemCapabilityChecker() {
 	 * @implNote Only short-running tests with constant time are performed
 	 * @since 1.9.2
 	 */
-	public void assertAllCapabilities(Path pathToVault) throws MissingCapabilityException {
+	public static void assertAllCapabilities(Path pathToVault) throws MissingCapabilityException {
 		assertReadAccess(pathToVault);
 		assertWriteAccess(pathToVault);
 	}
@@ -62,7 +59,7 @@ public void assertAllCapabilities(Path pathToVault) throws MissingCapabilityExce
 	 * @throws MissingCapabilityException if the check fails
 	 * @since 1.9.3
 	 */
-	public void assertReadAccess(Path pathToVault) throws MissingCapabilityException {
+	public static void assertReadAccess(Path pathToVault) throws MissingCapabilityException {
 		try (DirectoryStream<Path> ds = Files.newDirectoryStream(pathToVault)) {
 			assert ds != null;
 		} catch (IOException e) {
@@ -77,7 +74,7 @@ public void assertReadAccess(Path pathToVault) throws MissingCapabilityException
 	 * @throws MissingCapabilityException if the check fails
 	 * @since 1.9.3
 	 */
-	public void assertWriteAccess(Path pathToVault) throws MissingCapabilityException {
+	public static void assertWriteAccess(Path pathToVault) throws MissingCapabilityException {
 		Path checkDir = pathToVault.resolve("c");
 		try {
 			Files.createDirectories(checkDir);
@@ -90,7 +87,7 @@ public void assertWriteAccess(Path pathToVault) throws MissingCapabilityExceptio
 		}
 	}
 
-	public int determineSupportedCleartextFileNameLength(Path pathToVault) throws IOException {
+	public static int determineSupportedCleartextFileNameLength(Path pathToVault) throws IOException {
 		int maxCiphertextLen = determineSupportedCiphertextFileNameLength(pathToVault);
 		assert maxCiphertextLen >= Constants.MIN_CIPHER_NAME_LENGTH;
 		// math explained in https://github.com/cryptomator/cryptofs/issues/60#issuecomment-523238303;
@@ -105,22 +102,22 @@ public int determineSupportedCleartextFileNameLength(Path pathToVault) throws IO
 	 * @return Number of chars a .c9r file is allowed to have
 	 * @throws IOException If unable to perform this check
 	 */
-	public int determineSupportedCiphertextFileNameLength(Path pathToVault) throws IOException {
+	public static int determineSupportedCiphertextFileNameLength(Path pathToVault) throws IOException {
 		int subPathLength = Constants.MAX_ADDITIONAL_PATH_LENGTH - 2; // subtract "c/"
 		return determineSupportedCiphertextFileNameLength(pathToVault.resolve("c"), subPathLength, Constants.MIN_CIPHER_NAME_LENGTH, Constants.MAX_CIPHER_NAME_LENGTH);
 	}
 
 	/**
 	 * Determines the number of chars a filename is allowed to have inside of subdirectories of <code>dir</code> by running an experiment.
 	 *
-	 * @param dir               Path to a directory where to conduct the experiment (e.g. <code>/path/to/vault/c</code>)
-	 * @param subPathLength     Defines the combined number of chars of the subdirectories inside <code>dir</code>, including slashes but excluding the leading slash. Must be a minimum of 6
+	 * @param dir Path to a directory where to conduct the experiment (e.g. <code>/path/to/vault/c</code>)
+	 * @param subPathLength Defines the combined number of chars of the subdirectories inside <code>dir</code>, including slashes but excluding the leading slash. Must be a minimum of 6
 	 * @param minFileNameLength The minimum filename length to check
 	 * @param maxFileNameLength The maximum filename length to check
 	 * @return The supported filename length inside a subdirectory of <code>dir</code> with <code>subPathLength</code> chars
 	 * @throws IOException If unable to perform this check
 	 */
-	public int determineSupportedCiphertextFileNameLength(Path dir, int subPathLength, int minFileNameLength, int maxFileNameLength) throws IOException {
+	public static int determineSupportedCiphertextFileNameLength(Path dir, int subPathLength, int minFileNameLength, int maxFileNameLength) throws IOException {
 		Preconditions.checkArgument(subPathLength >= 6, "subPathLength must be larger than charcount(a/nnn/)");
 		Preconditions.checkArgument(minFileNameLength > 0);
 		Preconditions.checkArgument(maxFileNameLength <= 999);
@@ -141,7 +138,7 @@ public int determineSupportedCiphertextFileNameLength(Path dir, int subPathLengt
 		}
 	}
 
-	private int determineSupportedCiphertextFileNameLength(Path p, int lowerBoundIncl, int upperBoundExcl) {
+	private static int determineSupportedCiphertextFileNameLength(Path p, int lowerBoundIncl, int upperBoundExcl) {
 		assert lowerBoundIncl < upperBoundExcl;
 		int mid = (lowerBoundIncl + upperBoundExcl) / 2;
 		assert mid < upperBoundExcl;
@@ -156,7 +153,7 @@ private int determineSupportedCiphertextFileNameLength(Path p, int lowerBoundInc
 		}
 	}
 
-	private boolean canHandleFileNameLength(Path parent, int nameLength) {
+	private static boolean canHandleFileNameLength(Path parent, int nameLength) {
 		Path checkDir = parent.resolve(String.format("%03d", nameLength));
 		Path checkFile = checkDir.resolve(Strings.repeat("a", nameLength));
 		try {
@@ -175,7 +172,7 @@ private boolean canHandleFileNameLength(Path parent, int nameLength) {
 		}
 	}
 
-	private boolean canListDir(Path dir) {
+	private static boolean canListDir(Path dir) {
 		try (DirectoryStream<Path> ds = Files.newDirectoryStream(dir)) {
 			ds.iterator().hasNext(); // throws DirectoryIteratorException on Windows if child path too long
 			return true;
@@ -184,15 +181,15 @@ private boolean canListDir(Path dir) {
 		}
 	}
 
-	private void deleteSilently(Path path) {
+	private static void deleteSilently(Path path) {
 		try {
 			Files.delete(path);
 		} catch (IOException e) {
 			LOG.trace("Failed to delete " + path, e);
 		}
 	}
 
-	private void deleteRecursivelySilently(Path dir) {
+	private static void deleteRecursivelySilently(Path dir) {
 		try {
 			if (Files.exists(dir)) {
 				MoreFiles.deleteRecursively(dir, RecursiveDeleteOption.ALLOW_INSECURE);

src/main/java/org/cryptomator/cryptofs/migration/MigrationModule.java

@@ -25,11 +25,6 @@
 @Module
 class MigrationModule {
 
-	@Provides
-	FileSystemCapabilityChecker provideFileSystemCapabilityChecker() {
-		return new FileSystemCapabilityChecker();
-	}
-
 	@Provides
 	@IntoMap
 	@MigratorKey(Migration.FIVE_TO_SIX)

src/main/java/org/cryptomator/cryptofs/migration/Migrators.java

@@ -46,12 +46,10 @@ public class Migrators {
 	private static final MigrationComponent COMPONENT = DaggerMigrationComponent.builder().csprng(strongSecureRandom()).build();
 
 	private final Map<Migration, Migrator> migrators;
-	private final FileSystemCapabilityChecker fsCapabilityChecker;
 
 	@Inject
-	Migrators(Map<Migration, Migrator> migrators, FileSystemCapabilityChecker fsCapabilityChecker) {
+	Migrators(Map<Migration, Migrator> migrators) {
 		this.migrators = migrators;
-		this.fsCapabilityChecker = fsCapabilityChecker;
 	}
 
 	private static SecureRandom strongSecureRandom() {
@@ -69,9 +67,9 @@ public static Migrators get() {
 	/**
 	 * Inspects the vault and checks if it is supported by this library.
 	 *
-	 * @param pathToVault         Path to the vault's root
+	 * @param pathToVault Path to the vault's root
 	 * @param vaultConfigFilename Name of the vault config file located in the vault
-	 * @param masterkeyFilename   Name of the masterkey file optionally located in the vault
+	 * @param masterkeyFilename Name of the masterkey file optionally located in the vault
 	 * @return <code>true</code> if the vault at the given path is of an older format than supported by this library
 	 * @throws IOException if an I/O error occurs parsing the masterkey file
 	 */
@@ -83,19 +81,19 @@ public boolean needsMigration(Path pathToVault, String vaultConfigFilename, Stri
 	/**
 	 * Performs the actual migration. This task may take a while and this method will block.
 	 *
-	 * @param pathToVault          Path to the vault's root
-	 * @param vaultConfigFilename  Name of the vault config file located inside <code>pathToVault</code>
-	 * @param masterkeyFilename    Name of the masterkey file located inside <code>pathToVault</code>
-	 * @param passphrase           The passphrase needed to unlock the vault
-	 * @param progressListener     Listener that will get notified of progress updates
+	 * @param pathToVault Path to the vault's root
+	 * @param vaultConfigFilename Name of the vault config file located inside <code>pathToVault</code>
+	 * @param masterkeyFilename Name of the masterkey file located inside <code>pathToVault</code>
+	 * @param passphrase The passphrase needed to unlock the vault
+	 * @param progressListener Listener that will get notified of progress updates
 	 * @param continuationListener Listener that will get asked if there are events that require feedback
-	 * @throws NoApplicableMigratorException                          If the vault can not be migrated, because no migrator could be found
-	 * @throws InvalidPassphraseException                             If the passphrase could not be used to unlock the vault
+	 * @throws NoApplicableMigratorException If the vault can not be migrated, because no migrator could be found
+	 * @throws InvalidPassphraseException If the passphrase could not be used to unlock the vault
 	 * @throws FileSystemCapabilityChecker.MissingCapabilityException If the underlying filesystem lacks features required to store a vault
-	 * @throws IOException                                            if an I/O error occurs migrating the vault
+	 * @throws IOException if an I/O error occurs migrating the vault
 	 */
 	public void migrate(Path pathToVault, String vaultConfigFilename, String masterkeyFilename, CharSequence passphrase, MigrationProgressListener progressListener, MigrationContinuationListener continuationListener) throws NoApplicableMigratorException, CryptoException, IOException {
-		fsCapabilityChecker.assertAllCapabilities(pathToVault);
+		FileSystemCapabilityChecker.assertAllCapabilities(pathToVault);
 		int vaultVersion = determineVaultVersion(pathToVault, vaultConfigFilename, masterkeyFilename);
 		try {
 			Migrator migrator = findApplicableMigrator(vaultVersion).orElseThrow(NoApplicableMigratorException::new);

src/main/java/org/cryptomator/cryptofs/migration/v7/Version7Migrator.java

@@ -66,7 +66,7 @@ public void migrate(Path vaultRoot, String vaultConfigFilename, String masterkey
 			LOG.info("Backed up masterkey from {} to {}.", masterkeyFile.getFileName(), masterkeyBackupFile.getFileName());
 
 			// check file system capabilities:
-			int filenameLengthLimit = new FileSystemCapabilityChecker().determineSupportedCiphertextFileNameLength(vaultRoot.resolve("c"), 46, 28, 220);
+			int filenameLengthLimit = FileSystemCapabilityChecker.determineSupportedCiphertextFileNameLength(vaultRoot.resolve("c"), 46, 28, 220);
 			int pathLengthLimit = filenameLengthLimit + 48; // TODO
 			PreMigrationVisitor preMigrationVisitor;
 			if (filenameLengthLimit >= 220) {

src/test/java/org/cryptomator/cryptofs/CryptoFileSystemsTest.java

@@ -41,7 +41,6 @@ public class CryptoFileSystemsTest {
 	private final Path dataDirPath = mock(Path.class, "normalizedVaultPath/d");
 	private final Path preContenRootPath = mock(Path.class, "normalizedVaultPath/d/AB");
 	private final Path contenRootPath = mock(Path.class, "normalizedVaultPath/d/AB/CDEFGHIJKLMNOP");
-	private final FileSystemCapabilityChecker capabilityChecker = mock(FileSystemCapabilityChecker.class);
 	private final CryptoFileSystemProvider provider = mock(CryptoFileSystemProvider.class);
 	private final CryptoFileSystemProperties properties = mock(CryptoFileSystemProperties.class);
 	private final CryptoFileSystemComponent cryptoFileSystemComponent = mock(CryptoFileSystemComponent.class);
@@ -65,7 +64,7 @@ public class CryptoFileSystemsTest {
 	private MockedStatic<CryptorProvider> cryptorProviderClass;
 	private MockedStatic<BackupHelper> backupHelperClass;
 
-	private final CryptoFileSystems inTest = new CryptoFileSystems(cryptoFileSystemComponentFactory, capabilityChecker, csprng);
+	private final CryptoFileSystems inTest = new CryptoFileSystems(cryptoFileSystemComponentFactory, csprng);
 
 	@BeforeEach
 	public void setup() throws IOException, MasterkeyLoadingFailedException {