Merge branch 'release/1.9.2'

cryptomator · Feb 17, 2020 · 000dc40 · 000dc40
2 parents 54149b5 + a251ad2
commit 000dc40
Show file tree

Hide file tree

Showing 10 changed files with 233 additions and 19 deletions.
diff --git a/pom.xml b/pom.xml
@@ -2,7 +2,7 @@
 	<modelVersion>4.0.0</modelVersion>
 	<groupId>org.cryptomator</groupId>
 	<artifactId>cryptofs</artifactId>
-	<version>1.9.1</version>
+	<version>1.9.2</version>
 	<name>Cryptomator Crypto Filesystem</name>
 	<description>This library provides the Java filesystem provider used by Cryptomator.</description>
 	<url>https://github.com/cryptomator/cryptofs</url>
@@ -15,11 +15,11 @@
 
 	<properties>
 		<cryptolib.version>1.3.0</cryptolib.version>
-		<dagger.version>2.25.4</dagger.version>
+		<dagger.version>2.26</dagger.version>
 		<guava.version>28.2-jre</guava.version>
 		<slf4j.version>1.7.30</slf4j.version>
 
-		<junit.jupiter.version>5.5.2</junit.jupiter.version>
+		<junit.jupiter.version>5.6.0</junit.jupiter.version>
 		<mockito.version>3.2.4</mockito.version>
 		<hamcrest.version>2.2</hamcrest.version>
 		<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
@@ -239,6 +239,31 @@
 							</execution>
 						</executions>
 						<configuration>
+							<tags>
+								<!-- workaround for "unknown tag: implNote", see https://blog.codefx.org/java/new-javadoc-tags/#Maven  -->
+								<tag>
+									<name>apiNote</name>
+									<placement>a</placement>
+									<head>API Note:</head>
+								</tag>
+								<tag>
+									<name>implSpec</name>
+									<placement>a</placement>
+									<head>Implementation Requirements:</head>
+								</tag>
+								<tag>
+									<name>implNote</name>
+									<placement>a</placement>
+									<head>Implementation Note:</head>
+								</tag>
+								<tag><name>param</name></tag>
+								<tag><name>return</name></tag>
+								<tag><name>throws</name></tag>
+								<tag><name>since</name></tag>
+								<tag><name>version</name></tag>
+								<tag><name>serialData</name></tag>
+								<tag><name>see</name></tag>
+							</tags>
 							<additionalDependencies>
 								<additionalDependency>
 									<groupId>javax.annotation</groupId>

diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProvider.java b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProvider.java
@@ -10,6 +10,7 @@
 
 import org.cryptomator.cryptofs.ch.AsyncDelegatingFileChannel;
 import org.cryptomator.cryptofs.common.Constants;
+import org.cryptomator.cryptofs.common.FileSystemCapabilityChecker;
 import org.cryptomator.cryptofs.common.MasterkeyBackupFileHasher;
 import org.cryptomator.cryptofs.migration.Migrators;
 import org.cryptomator.cryptolib.Cryptors;
@@ -124,6 +125,7 @@ private static SecureRandom strongSecureRandom() {
 	 * @param properties Parameters used during initialization of the file system
 	 * @return a new file system
 	 * @throws FileSystemNeedsMigrationException if the vault format needs to get updated and <code>properties</code> did not contain a flag for implicit migration.
+	 * @throws FileSystemCapabilityChecker.MissingCapabilityException If the underlying filesystem lacks features required to store a vault
 	 * @throws IOException if an I/O error occurs creating the file system
 	 */
 	public static CryptoFileSystem newFileSystem(Path pathToVault, CryptoFileSystemProperties properties) throws FileSystemNeedsMigrationException, IOException {
@@ -138,6 +140,7 @@ public static CryptoFileSystem newFileSystem(Path pathToVault, CryptoFileSystemP
 	 * @param masterkeyFilename Name of the masterkey file
 	 * @param passphrase Passphrase that should be used to unlock the vault
 	 * @throws NotDirectoryException If the given path is not an existing directory.
+	 * @throws FileSystemCapabilityChecker.MissingCapabilityException If the underlying filesystem lacks features required to store a vault
 	 * @throws IOException If the vault structure could not be initialized due to I/O errors
 	 * @since 1.3.0
 	 */
@@ -153,13 +156,15 @@ public static void initialize(Path pathToVault, String masterkeyFilename, CharSe
 	 * @param pepper Application-specific pepper used during key derivation
 	 * @param passphrase Passphrase that should be used to unlock the vault
 	 * @throws NotDirectoryException If the given path is not an existing directory.
+	 * @throws FileSystemCapabilityChecker.MissingCapabilityException If the underlying filesystem lacks features required to store a vault
 	 * @throws IOException If the vault structure could not be initialized due to I/O errors
 	 * @since 1.3.2
 	 */
 	public static void initialize(Path pathToVault, String masterkeyFilename, byte[] pepper, CharSequence passphrase) throws NotDirectoryException, IOException {
 		if (!Files.isDirectory(pathToVault)) {
 			throw new NotDirectoryException(pathToVault.toString());
 		}
+		new FileSystemCapabilityChecker().checkCapabilities(pathToVault);
 		try (Cryptor cryptor = CRYPTOR_PROVIDER.createNew()) {
 			// save masterkey file:
 			Path masterKeyPath = pathToVault.resolve(masterkeyFilename);
@@ -288,6 +293,8 @@ public String getScheme() {
 	public CryptoFileSystem newFileSystem(URI uri, Map<String, ?> rawProperties) throws IOException {
 		CryptoFileSystemUri parsedUri = CryptoFileSystemUri.parse(uri);
 		CryptoFileSystemProperties properties = CryptoFileSystemProperties.wrap(rawProperties);
+
+		new FileSystemCapabilityChecker().checkCapabilities(parsedUri.pathToVault());
 
 		// TODO remove implicit initialization in 2.0.0
 		initializeFileSystemIfRequired(parsedUri, properties);

diff --git a/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java b/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java
@@ -211,9 +211,7 @@ private void forceInternal(boolean metaData) throws IOException {
 		flush();
 		ciphertextFileChannel.force(metaData);
 		if (metaData) {
-			FileTime lastModifiedTime = isWritable() ? FileTime.from(lastModified.get()) : null;
-			FileTime lastAccessTime = FileTime.from(Instant.now());
-			attrViewProvider.get().setTimes(lastModifiedTime, lastAccessTime, null);
+			persistLastModified();
 		}
 	}
 
@@ -229,6 +227,17 @@ private void flush() throws IOException {
 		}
 	}
 
+	/**
+	 * Corrects the last modified and access date due to possible cache invalidation (i.e. write operation!)
+	 *
+	 * @throws IOException
+	 */
+	private void persistLastModified() throws IOException {
+		FileTime lastModifiedTime = isWritable() ? FileTime.from(lastModified.get()) : null;
+		FileTime lastAccessTime = FileTime.from(Instant.now());
+		attrViewProvider.get().setTimes(lastModifiedTime, lastAccessTime, null);
+	}
+
 	@Override
 	public MappedByteBuffer map(MapMode mode, long position, long size) {
 		throw new UnsupportedOperationException();
@@ -294,6 +303,7 @@ long beginOfChunk(long cleartextPos) {
 	protected void implCloseChannel() throws IOException {
 		try {
 			flush();
+			persistLastModified();
 		} finally {
 			super.implCloseChannel();
 			closeListener.closed(this);

diff --git a/src/main/java/org/cryptomator/cryptofs/common/FileSystemCapabilityChecker.java b/src/main/java/org/cryptomator/cryptofs/common/FileSystemCapabilityChecker.java
@@ -0,0 +1,88 @@
+package org.cryptomator.cryptofs.common;
+
+import com.google.common.base.Joiner;
+import com.google.common.base.Strings;
+import com.google.common.io.MoreFiles;
+import com.google.common.io.RecursiveDeleteOption;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.FileSystemException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+public class FileSystemCapabilityChecker {
+
+	private static final Logger LOG = LoggerFactory.getLogger(FileSystemCapabilityChecker.class);
+
+	public enum Capability {
+		/**
+		 * File system supports filenames with ≥ 230 chars.
+		 */
+		LONG_FILENAMES,
+
+		/**
+		 * File system supports paths with ≥ 400 chars.
+		 */
+		LONG_PATHS,
+	}
+
+	/**
+	 * Checks whether the underlying filesystem has all required capabilities.
+	 *
+	 * @param pathToVault Path to a vault's storage location
+	 * @throws MissingCapabilityException if any check fails
+	 * @implNote Only short-running tests with constant time are performed
+	 * @since 1.9.2
+	 */
+	public void checkCapabilities(Path pathToVault) throws MissingCapabilityException {
+		Path checkDir = pathToVault.resolve("c");
+		try {
+			checkLongFilenames(checkDir);
+			checkLongFilePaths(checkDir);
+		} finally {
+			try {
+				MoreFiles.deleteRecursively(checkDir, RecursiveDeleteOption.ALLOW_INSECURE);
+			} catch (IOException e) {
+				LOG.warn("Failed to clean up " + checkDir, e);
+			}
+		}
+	}
+
+	private void checkLongFilenames(Path checkDir) throws MissingCapabilityException {
+		String longFileName = Strings.repeat("a", 226) + ".c9r";
+		Path p = checkDir.resolve(longFileName);
+		try {
+			Files.createDirectories(p);
+		} catch (IOException e) {
+			throw new MissingCapabilityException(p, Capability.LONG_FILENAMES);
+		}
+	}
+
+	private void checkLongFilePaths(Path checkDir) throws MissingCapabilityException {
+		String longFileName = Strings.repeat("a", 96) + ".c9r";
+		String longPath = Joiner.on('/').join(longFileName, longFileName, longFileName, longFileName);
+		Path p = checkDir.resolve(longPath);
+		try {
+			Files.createDirectories(p);
+		} catch (IOException e) {
+			throw new MissingCapabilityException(p, Capability.LONG_PATHS);
+		}
+	}
+
+	public static class MissingCapabilityException extends FileSystemException {
+
+		private final Capability missingCapability;
+
+		public MissingCapabilityException(Path path, Capability missingCapability) {
+			super(path.toString(), null, "Filesystem doesn't support " + missingCapability);
+			this.missingCapability = missingCapability;
+		}
+
+		public Capability getMissingCapability() {
+			return missingCapability;
+		}
+	}
+
+}
diff --git a/src/main/java/org/cryptomator/cryptofs/migration/MigrationModule.java b/src/main/java/org/cryptomator/cryptofs/migration/MigrationModule.java
@@ -13,6 +13,7 @@
 import dagger.Module;
 import dagger.Provides;
 import dagger.multibindings.IntoMap;
+import org.cryptomator.cryptofs.common.FileSystemCapabilityChecker;
 import org.cryptomator.cryptofs.migration.api.Migrator;
 import org.cryptomator.cryptofs.migration.v6.Version6Migrator;
 import org.cryptomator.cryptofs.migration.v7.Version7Migrator;
@@ -34,6 +35,11 @@ class MigrationModule {
 	CryptorProvider provideVersion1CryptorProvider() {
 		return version1Cryptor;
 	}
+
+	@Provides
+	FileSystemCapabilityChecker provideFileSystemCapabilityChecker() {
+		return new FileSystemCapabilityChecker();
+	}
 
 	@Provides
 	@IntoMap

diff --git a/src/main/java/org/cryptomator/cryptofs/migration/Migrators.java b/src/main/java/org/cryptomator/cryptofs/migration/Migrators.java
@@ -16,6 +16,7 @@
 import javax.inject.Inject;
 
 import org.cryptomator.cryptofs.common.Constants;
+import org.cryptomator.cryptofs.common.FileSystemCapabilityChecker;
 import org.cryptomator.cryptofs.migration.api.MigrationProgressListener;
 import org.cryptomator.cryptofs.migration.api.Migrator;
 import org.cryptomator.cryptofs.migration.api.NoApplicableMigratorException;
@@ -46,10 +47,12 @@ public class Migrators {
 			.build();
 
 	private final Map<Migration, Migrator> migrators;
+	private final FileSystemCapabilityChecker fsCapabilityChecker;
 
 	@Inject
-	Migrators(Map<Migration, Migrator> migrators) {
+	Migrators(Map<Migration, Migrator> migrators, FileSystemCapabilityChecker fsCapabilityChecker) {
 		this.migrators = migrators;
+		this.fsCapabilityChecker = fsCapabilityChecker;
 	}
 
 	private static SecureRandom strongSecureRandom() {
@@ -87,9 +90,12 @@ public boolean needsMigration(Path pathToVault, String masterkeyFilename) throws
 	 * @param passphrase The passphrase needed 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
 	 */
 	public void migrate(Path pathToVault, String masterkeyFilename, CharSequence passphrase, MigrationProgressListener progressListener) throws NoApplicableMigratorException, InvalidPassphraseException, IOException {
+		fsCapabilityChecker.checkCapabilities(pathToVault);
+
 		Path masterKeyPath = pathToVault.resolve(masterkeyFilename);
 		byte[] keyFileContents = Files.readAllBytes(masterKeyPath);
 		KeyFile keyFile = KeyFile.parse(keyFileContents);

diff --git a/src/test/java/org/cryptomator/cryptofs/CryptoFileChannelWriteReadIntegrationTest.java b/src/test/java/org/cryptomator/cryptofs/CryptoFileChannelWriteReadIntegrationTest.java
@@ -63,9 +63,9 @@ public void setupClass(@TempDir Path tmpDir) throws IOException {
 			fileSystem = new CryptoFileSystemProvider().newFileSystem(create(tmpDir), cryptoFileSystemProperties().withPassphrase("asd").build());
 		}
 
-		// tests https://github.com/cryptomator/cryptofs/issues/56
+		// tests https://github.com/cryptomator/cryptofs/issues/69
 		@Test
-		public void testForceDoesntBumpModifiedDate() throws IOException {
+		public void testCloseDoesNotBumpModifiedDate() throws IOException {
 			Path file = fileSystem.getPath("/file.txt");
 
 			Instant t0, t1;
@@ -76,7 +76,43 @@ public void testForceDoesntBumpModifiedDate() throws IOException {
 			}
 
 			t1 = Files.getLastModifiedTime(file).toInstant().truncatedTo(ChronoUnit.SECONDS);
-			Assertions.assertTrue(t1.equals(t0));
+			Assertions.assertEquals(t0, t1);
+		}
+
+		@Test
+		public void testLastModifiedIsPreservedOverSeveralOperations() throws IOException, InterruptedException {
+			Path file = fileSystem.getPath("/file2.txt");
+
+			Instant t0, t1, t2, t3, t4, t5;
+			t0 = Instant.ofEpochSecond(123456789).truncatedTo(ChronoUnit.SECONDS);
+			ByteBuffer data = ByteBuffer.wrap("CryptoFS".getBytes());
+
+			try (FileChannel ch = FileChannel.open(file, CREATE_NEW, WRITE)) {
+				t1 = Files.getLastModifiedTime(file).toInstant().truncatedTo(ChronoUnit.MILLIS);
+				Thread.currentThread().sleep(50);
+
+				ch.write(data);
+				ch.force(true);
+				Thread.currentThread().sleep(50);
+				t2 = Files.getLastModifiedTime(file).toInstant().truncatedTo(ChronoUnit.MILLIS);
+
+				Files.setLastModifiedTime(file, FileTime.from(t0));
+				ch.force(true);
+				Thread.currentThread().sleep(50);
+				t3 = Files.getLastModifiedTime(file).toInstant().truncatedTo(ChronoUnit.MILLIS);
+
+				ch.write(data);
+				ch.force(true);
+				Thread.currentThread().sleep(1000);
+				t4 = Files.getLastModifiedTime(file).toInstant().truncatedTo(ChronoUnit.SECONDS);
+
+			}
+
+			t5 = Files.getLastModifiedTime(file).toInstant().truncatedTo(ChronoUnit.SECONDS);
+			Assertions.assertNotEquals(t1, t2);
+			Assertions.assertEquals(t0, t3);
+			Assertions.assertNotEquals(t4, t3);
+			Assertions.assertEquals(t4, t5);
 		}
 
 	}

diff --git a/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java b/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java
@@ -222,6 +222,26 @@ public void testCloseTriggersCloseListener() throws IOException {
 
 			verify(closeListener).closed(inTest);
 		}
+
+		@Test
+		public void testCloseUpdatesLastModifiedTimeIfWriteable() throws IOException {
+			when(options.writable()).thenReturn(true);
+			lastModified.set(Instant.ofEpochMilli(123456789000l));
+			FileTime fileTime = FileTime.from(lastModified.get());
+
+			inTest.implCloseChannel();
+
+			verify(attributeView).setTimes(Mockito.eq(fileTime), Mockito.any(), Mockito.isNull());
+		}
+
+		@Test
+		public void testCloseDoesNotUpdateLastModifiedTimeIfReadOnly() throws IOException {
+			when(options.writable()).thenReturn(false);
+
+			inTest.implCloseChannel();
+
+			verify(attributeView).setTimes(Mockito.isNull(), Mockito.any(), Mockito.isNull());
+		}
 	}
 
 	@Test
@@ -267,7 +287,7 @@ public void testTryLockReturnsNullIfDelegateReturnsNull() throws IOException {
 		@Test
 		@DisplayName("successful tryLock()")
 		public void testTryLockReturnsCryptoFileLockWrappingDelegate() throws IOException {
-			when(ciphertextFileChannel.tryLock(380l, 4670l+110l-380l, true)).thenReturn(delegate);
+			when(ciphertextFileChannel.tryLock(380l, 4670l + 110l - 380l, true)).thenReturn(delegate);
 
 			FileLock result = inTest.tryLock(372l, 3828l, true);
 
@@ -283,7 +303,7 @@ public void testTryLockReturnsCryptoFileLockWrappingDelegate() throws IOExceptio
 		@Test
 		@DisplayName("successful lock()")
 		public void testLockReturnsCryptoFileLockWrappingDelegate() throws IOException {
-			when(ciphertextFileChannel.lock(380l, 4670l+110l-380l, true)).thenReturn(delegate);
+			when(ciphertextFileChannel.lock(380l, 4670l + 110l - 380l, true)).thenReturn(delegate);
 
 			FileLock result = inTest.lock(372l, 3828l, true);
 
@@ -472,7 +492,7 @@ public void testDontRewriteHeader() throws IOException {
 			inTest = new CleartextFileChannel(ciphertextFileChannel, header, false, readWriteLock, cryptor, chunkCache, options, fileSize, lastModified, attributeViewSupplier, exceptionsDuringWrite, closeListener, stats);
 
 			inTest.force(true);
-			
+
 			Mockito.verify(ciphertextFileChannel, Mockito.never()).write(Mockito.any(), Mockito.eq(0l));
 		}