Merge branch 'release/1.2.3'

pom.xml

@@ -2,7 +2,7 @@
 	<modelVersion>4.0.0</modelVersion>
 	<groupId>org.cryptomator</groupId>
 	<artifactId>cryptofs</artifactId>
-	<version>1.2.2</version>
+	<version>1.2.3</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>
@@ -16,7 +16,7 @@
 	<properties>
 		<java.version>1.8</java.version>
 		<cryptolib.version>1.1.1</cryptolib.version>
-		<dagger.version>2.10</dagger.version>
+		<dagger.version>2.11</dagger.version>
 		<guava.version>21.0</guava.version>
 		<commons.lang.version>3.5</commons.lang.version>
 		<slf4j.version>1.7.25</slf4j.version>
@@ -91,12 +91,6 @@
 			<artifactId>dagger</artifactId>
 			<version>${dagger.version}</version>
 		</dependency>
-		<dependency>
-			<groupId>com.google.dagger</groupId>
-			<artifactId>dagger-compiler</artifactId>
-			<version>${dagger.version}</version>
-			<scope>provided</scope>
-		</dependency>
 
 		<!-- Test -->
 		<dependency>
@@ -114,7 +108,7 @@
 		<dependency>
 			<groupId>org.mockito</groupId>
 			<artifactId>mockito-core</artifactId>
-			<version>2.7.21</version>
+			<version>2.8.9</version>
 			<scope>test</scope>
 		</dependency>
 		<dependency>
@@ -138,7 +132,7 @@
 		<dependency>
 			<groupId>org.bouncycastle</groupId>
 			<artifactId>bcprov-jdk15on</artifactId>
-			<version>1.56</version>
+			<version>1.57</version>
 			<!-- not actually needed, but otherwise unit tests fail in Eclipe, if the "cryptolib" project is opened in the same workspace -->
 			<scope>test</scope>
 		</dependency>
@@ -154,6 +148,13 @@
 					<source>${java.version}</source>
 					<target>${java.version}</target>
 					<showWarnings>true</showWarnings>
+					<annotationProcessorPaths>
+						<path>
+							<groupId>com.google.dagger</groupId>
+							<artifactId>dagger-compiler</artifactId>
+							<version>${dagger.version}</version>
+						</path>
+					</annotationProcessorPaths>
 				</configuration>
 			</plugin>
 		</plugins>
@@ -190,7 +191,7 @@
 				<dependency>
 					<groupId>com.codacy</groupId>
 					<artifactId>codacy-coverage-reporter</artifactId>
-					<version>1.0.13</version>
+					<version>2.0.0</version>
 					<classifier>assembly</classifier>
 					<exclusions>
 						<exclusion>
@@ -281,7 +282,7 @@
 					</plugin>
 					<plugin>
 						<artifactId>maven-dependency-plugin</artifactId>
-						<version>3.0.0</version>
+						<version>3.0.1</version>
 						<executions>
 							<execution>
 								<id>generate-dependency-list</id>

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

@@ -316,19 +316,19 @@ void createDirectory(CryptoPath cleartextDir, FileAttribute<?>... attrs) throws
 			throw new FileAlreadyExistsException(cleartextDir.toString());
 		}
 		Path ciphertextDirFile = cryptoPathMapper.getCiphertextFilePath(cleartextDir, CiphertextFileType.DIRECTORY);
-		boolean success = false;
+		Directory ciphertextDir = cryptoPathMapper.getCiphertextDir(cleartextDir);
+		// atomically check for FileAlreadyExists and create otherwise:
+		try (FileChannel channel = FileChannel.open(ciphertextDirFile, EnumSet.of(StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE), attrs)) {
+			channel.write(ByteBuffer.wrap(ciphertextDir.dirId.getBytes(UTF_8)));
+		}
+		// create dir if and only if the dirFile has been created right now (not if it has been created before):
 		try {
-			Directory ciphertextDir = cryptoPathMapper.getCiphertextDir(cleartextDir);
-			try (FileChannel channel = FileChannel.open(ciphertextDirFile, EnumSet.of(StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE), attrs)) {
-				channel.write(ByteBuffer.wrap(ciphertextDir.dirId.getBytes(UTF_8)));
-			}
 			Files.createDirectories(ciphertextDir.path);
-			success = true;
-		} finally {
-			if (!success) {
-				Files.delete(ciphertextDirFile);
-				dirIdProvider.delete(ciphertextDirFile);
-			}
+		} catch (IOException e) {
+			// make sure there is no orphan dir file:
+			Files.delete(ciphertextDirFile);
+			dirIdProvider.delete(ciphertextDirFile);
+			throw e;
 		}
 	}
 

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

@@ -1,9 +1,12 @@
 package org.cryptomator.cryptofs;
 
 import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.channels.FileChannel;
 import java.nio.charset.StandardCharsets;
-import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
 import java.util.UUID;
 
 import javax.inject.Inject;
@@ -13,19 +16,29 @@
 @PerFileSystem
 class DirectoryIdLoader extends CacheLoader<Path, String> {
 
+	private static final int MAX_DIR_ID_LENGTH = 1000;
+
 	@Inject
 	public DirectoryIdLoader() {
 	}
 
 	@Override
 	public String load(Path dirFilePath) throws IOException {
-		if (Files.exists(dirFilePath)) {
-			byte[] bytes = Files.readAllBytes(dirFilePath);
-			if (bytes.length == 0) {
+		try (FileChannel ch = FileChannel.open(dirFilePath, StandardOpenOption.READ)) {
+			long size = ch.size();
+			if (size == 0) {
 				throw new IOException("Invalid, empty directory file: " + dirFilePath);
+			} else if (size > MAX_DIR_ID_LENGTH) {
+				throw new IOException("Unexpectedly large directory file: " + dirFilePath);
+			} else {
+				assert size <= MAX_DIR_ID_LENGTH; // thus int
+				ByteBuffer buffer = ByteBuffer.allocate((int) size);
+				int read = ch.read(buffer);
+				assert read == size;
+				buffer.flip();
+				return StandardCharsets.UTF_8.decode(buffer).toString();
 			}
-			return new String(bytes, StandardCharsets.UTF_8);
-		} else {
+		} catch (NoSuchFileException e) {
 			return UUID.randomUUID().toString();
 		}
 	}

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

@@ -110,9 +110,9 @@ private void write(ByteSource source, long position) throws IOException {
 		int written = 0;
 		while (source.hasRemaining()) {
 			long currentPosition = position + written;
-			long chunkIndex = currentPosition / cleartextChunkSize;
-			int offsetInChunk = (int) currentPosition % cleartextChunkSize;
-			int len = (int) min(source.remaining(), cleartextChunkSize - offsetInChunk);
+			long chunkIndex = currentPosition / cleartextChunkSize; // floor by int-truncation
+			int offsetInChunk = (int) (currentPosition % cleartextChunkSize); // known to fit in int, because cleartextChunkSize is int
+			int len = (int) min(source.remaining(), cleartextChunkSize - offsetInChunk); // known to fit in int, because second argument is int
 			long minSize = currentPosition + len;
 			size.getAndUpdate(size -> max(minSize, size));
 			if (len == cleartextChunkSize) {

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

@@ -6,6 +6,8 @@
 import java.util.LinkedList;
 import java.util.function.Supplier;
 
+import com.google.common.base.Throwables;
+
 /**
  * <p>
  * Implements means to throw checked exceptions crossing method boundaries which do not declare them.
@@ -35,9 +37,7 @@ public <T> T from(Supplier<T> action) throws E {
 					allowedToBeThrownUnchecked.addFirst(type);
 					return action.get();
 				} catch (ExceptionThrownUnchecked e) {
-					if (type.isInstance(e.getCause())) {
-						throw type.cast(e.getCause());
-					}
+					Throwables.throwIfInstanceOf(e.getCause(), type);
 					throw e;
 				} finally {
 					allowedToBeThrownUnchecked.removeFirst();
@@ -56,9 +56,8 @@ public static <E extends Exception> RethrowUncheckedWithoutAction<E> rethrowUnch
 			public <T> T from(SupplierThrowingException<T, E> action) {
 				try {
 					return action.get();
-				} catch (RuntimeException e) {
-					throw e; // don't catch unchecked exceptions
 				} catch (Exception e) {
+					Throwables.throwIfUnchecked(e); // don't catch unchecked exceptions
 					throw new ExceptionThrownUnchecked(e);
 				}
 			}

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

@@ -714,8 +714,8 @@ public void copyDirectoryToAlreadyExistingDir() throws IOException {
 
 	public class CreateDirectory {
 
-		private FileSystemProvider provider = mock(FileSystemProvider.class);
-		private CryptoFileSystemImpl fileSystem = mock(CryptoFileSystemImpl.class);
+		private final FileSystemProvider provider = mock(FileSystemProvider.class);
+		private final CryptoFileSystemImpl fileSystem = mock(CryptoFileSystemImpl.class);
 
 		@Before
 		public void setup() {
@@ -796,46 +796,48 @@ public void createDirectoryCreatesDirectoryIfConditonsAreMet() throws IOExceptio
 		}
 
 		@Test
-		public void createDirectoryClearsDirIdAndDeletesDirFileIfWritingDirFileFails() throws IOException {
+		public void createDirectoryClearsDirIdAndDeletesDirFileIfCreatingDirFails() throws IOException {
 			CryptoPath path = mock(CryptoPath.class);
 			CryptoPath parent = mock(CryptoPath.class);
-			Path cyphertextParent = mock(Path.class);
-			Path cyphertextFile = mock(Path.class);
-			Path cyphertextDirFile = mock(Path.class);
-			Path cyphertextDirPath = mock(Path.class);
+			Path ciphertextParent = mock(Path.class, "ciphertextParent");
+			Path ciphertextFile = mock(Path.class, "ciphertextFile");
+			Path ciphertextDirFile = mock(Path.class, "ciphertextDirFile");
+			Path ciphertextDirPath = mock(Path.class, "ciphertextDir");
 			String dirId = "DirId1234ABC";
 			FileChannelMock channel = new FileChannelMock(100);
-			Directory cyphertextDir = new Directory(dirId, cyphertextDirPath);
+			Directory ciphertextDir = new Directory(dirId, ciphertextDirPath);
 			when(path.getParent()).thenReturn(parent);
-			when(cryptoPathMapper.getCiphertextDirPath(parent)).thenReturn(cyphertextParent);
-			when(cryptoPathMapper.getCiphertextFilePath(path, CiphertextFileType.FILE)).thenReturn(cyphertextFile);
-			when(cryptoPathMapper.getCiphertextFilePath(path, CiphertextFileType.DIRECTORY)).thenReturn(cyphertextDirFile);
-			when(cryptoPathMapper.getCiphertextDir(path)).thenReturn(cyphertextDir);
-			when(cyphertextParent.getFileSystem()).thenReturn(fileSystem);
-			when(cyphertextFile.getFileSystem()).thenReturn(fileSystem);
-			when(cyphertextDirFile.getFileSystem()).thenReturn(fileSystem);
-			when(cyphertextDirPath.getFileSystem()).thenReturn(fileSystem);
-			when(provider.newFileChannel(cyphertextDirFile, EnumSet.of(StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE))).thenReturn(channel);
-			IOException expectedException = new IOException();
-			channel.setFailOnNextOperation(expectedException);
-			doThrow(NoSuchFileException.class).when(provider).checkAccess(cyphertextFile);
+			when(cryptoPathMapper.getCiphertextDirPath(parent)).thenReturn(ciphertextParent);
+			when(cryptoPathMapper.getCiphertextFilePath(path, CiphertextFileType.FILE)).thenReturn(ciphertextFile);
+			when(cryptoPathMapper.getCiphertextFilePath(path, CiphertextFileType.DIRECTORY)).thenReturn(ciphertextDirFile);
+			when(cryptoPathMapper.getCiphertextDir(path)).thenReturn(ciphertextDir);
+			when(ciphertextParent.getFileSystem()).thenReturn(fileSystem);
+			when(ciphertextFile.getFileSystem()).thenReturn(fileSystem);
+			when(ciphertextDirFile.getFileSystem()).thenReturn(fileSystem);
+			when(ciphertextDirPath.getFileSystem()).thenReturn(fileSystem);
+			doThrow(new NoSuchFileException("ciphertextFile")).when(provider).checkAccess(ciphertextFile);
+			when(provider.newFileChannel(ciphertextDirFile, EnumSet.of(StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE))).thenReturn(channel);
 
-			thrown.expect(is(expectedException));
+			// make createDirectory with an FileSystemException during Files.createDirectories(ciphertextDirPath)
+			doThrow(new IOException()).when(provider).createDirectory(ciphertextDirPath);
+			when(ciphertextDirPath.toAbsolutePath()).thenReturn(ciphertextDirPath);
+			when(ciphertextDirPath.getParent()).thenReturn(null);
+			thrown.expect(IOException.class);
 
 			try {
 				inTest.createDirectory(path);
 			} finally {
-				verify(provider).delete(cyphertextDirFile);
-				verify(dirIdProvider).delete(cyphertextDirFile);
+				verify(provider).delete(ciphertextDirFile);
+				verify(dirIdProvider).delete(ciphertextDirFile);
 			}
 		}
 
 	}
 
 	public class IsHidden {
 
-		private FileSystemProvider provider = mock(FileSystemProvider.class);
-		private CryptoFileSystemImpl fileSystem = mock(CryptoFileSystemImpl.class);
+		private final FileSystemProvider provider = mock(FileSystemProvider.class);
+		private final CryptoFileSystemImpl fileSystem = mock(CryptoFileSystemImpl.class);
 
 		@Before
 		public void setup() {
@@ -886,8 +888,8 @@ public void isHiddenReturnsFalseIfDosFileAttributeViewIsAvailableAndIsHiddenIsFa
 
 	public class CheckAccess {
 
-		private FileSystemProvider provider = mock(FileSystemProvider.class);
-		private CryptoFileSystemImpl fileSystem = mock(CryptoFileSystemImpl.class);
+		private final FileSystemProvider provider = mock(FileSystemProvider.class);
+		private final CryptoFileSystemImpl fileSystem = mock(CryptoFileSystemImpl.class);
 
 		@Before
 		public void setup() {
@@ -1110,8 +1112,8 @@ public void testAccessModeContainsOnlyKnownValues() {
 
 	public class SetAttribute {
 
-		private FileSystemProvider provider = mock(FileSystemProvider.class);
-		private CryptoFileSystemImpl fileSystem = mock(CryptoFileSystemImpl.class);
+		private final FileSystemProvider provider = mock(FileSystemProvider.class);
+		private final CryptoFileSystemImpl fileSystem = mock(CryptoFileSystemImpl.class);
 
 		@Before
 		public void setup() {