Merge branch 'release/1.8.2'

Author: tobihagemann

.travis.yml

@@ -1,3 +1,4 @@
+dist: xenial
 language: java
 sudo: false
 jdk:

pom.xml

@@ -2,7 +2,7 @@
 	<modelVersion>4.0.0</modelVersion>
 	<groupId>org.cryptomator</groupId>
 	<artifactId>cryptofs</artifactId>
-	<version>1.8.1</version>
+	<version>1.8.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>

src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java

@@ -10,14 +10,12 @@
 import org.cryptomator.cryptofs.fh.FileHeaderLoader;
 import org.cryptomator.cryptofs.fh.OpenFileModifiedDate;
 import org.cryptomator.cryptofs.fh.OpenFileSize;
-import org.cryptomator.cryptolib.Cryptors;
 import org.cryptomator.cryptolib.api.Cryptor;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import javax.inject.Inject;
 import java.io.IOException;
-import java.io.UncheckedIOException;
 import java.nio.ByteBuffer;
 import java.nio.MappedByteBuffer;
 import java.nio.channels.FileChannel;
@@ -68,30 +66,12 @@ public CleartextFileChannel(FileChannel ciphertextFileChannel, FileHeaderLoader
 		this.exceptionsDuringWrite = exceptionsDuringWrite;
 		this.closeListener = closeListener;
 		this.stats = stats;
-		updateFileSize();
 		if (options.append()) {
 			position = fileSize.get();
 		}
 		headerWritten = !options.writable();
 	}
 
-	private void updateFileSize() {
-		try {
-			long ciphertextSize = ciphertextFileChannel.size();
-			if (ciphertextSize == 0l) {
-				fileSize.set(0l);
-			} else {
-				long cleartextSize = Cryptors.cleartextSize(ciphertextSize - cryptor.fileHeaderCryptor().headerSize(), cryptor);
-				fileSize.set(cleartextSize);
-			}
-		} catch (IllegalArgumentException e) {
-			LOG.warn("Invalid cipher text file size.", e);
-			fileSize.set(0l);
-		} catch (IOException e) {
-			throw new UncheckedIOException(e);
-		}
-	}
-
 	@Override
 	public long size() throws IOException {
 		assertOpen();
@@ -180,7 +160,8 @@ private long writeLockedInternal(ByteSource src, long position) throws IOExcepti
 			written += len;
 		}
 		long minSize = position + written;
-		fileSize.updateAndGet(size -> max(minSize, size));
+		long newSize = fileSize.updateAndGet(size -> max(minSize, size));
+		assert newSize >= minSize;
 		lastModified.set(Instant.now());
 		stats.addBytesWritten(written);
 		return written;
@@ -221,6 +202,7 @@ public void force(boolean metaData) throws IOException {
 
 	private void forceInternal(boolean metaData) throws IOException {
 		if (isWritable()) {
+			writeHeaderIfNeeded();
 			chunkCache.invalidateAll(); // TODO performance: write chunks but keep them cached
 			exceptionsDuringWrite.throwIfPresent();
 			attrViewProvider.get().setTimes(FileTime.from(lastModified.get()), null, null);

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

@@ -8,9 +8,14 @@
  *******************************************************************************/
 package org.cryptomator.cryptofs.fh;
 
+import com.google.common.base.Preconditions;
 import org.cryptomator.cryptofs.EffectiveOpenOptions;
 import org.cryptomator.cryptofs.ch.ChannelComponent;
 import org.cryptomator.cryptofs.ch.CleartextFileChannel;
+import org.cryptomator.cryptolib.Cryptors;
+import org.cryptomator.cryptolib.api.Cryptor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import javax.inject.Inject;
 import java.io.Closeable;
@@ -27,26 +32,37 @@
 @OpenFileScoped
 public class OpenCryptoFile implements Closeable {
 
+	private static final Logger LOG = LoggerFactory.getLogger(OpenCryptoFile.class);
+
 	private final FileCloseListener listener;
 	private final AtomicReference<Instant> lastModified;
 	private final ChunkCache chunkCache;
+	private final Cryptor cryptor;
 	private final ChunkIO chunkIO;
 	private final AtomicReference<Path> currentFilePath;
 	private final AtomicLong fileSize;
 	private final OpenCryptoFileComponent component;
 	private final ConcurrentMap<CleartextFileChannel, FileChannel> openChannels = new ConcurrentHashMap<>();
 
 	@Inject
-	public OpenCryptoFile(FileCloseListener listener, ChunkCache chunkCache, ChunkIO chunkIO, @CurrentOpenFilePath AtomicReference<Path> currentFilePath, @OpenFileSize AtomicLong fileSize, @OpenFileModifiedDate AtomicReference<Instant> lastModified, OpenCryptoFileComponent component) {
+	public OpenCryptoFile(FileCloseListener listener, ChunkCache chunkCache, Cryptor cryptor, ChunkIO chunkIO, @CurrentOpenFilePath AtomicReference<Path> currentFilePath, @OpenFileSize AtomicLong fileSize, @OpenFileModifiedDate AtomicReference<Instant> lastModified, OpenCryptoFileComponent component) {
 		this.listener = listener;
 		this.chunkCache = chunkCache;
+		this.cryptor = cryptor;
 		this.chunkIO = chunkIO;
 		this.currentFilePath = currentFilePath;
 		this.fileSize = fileSize;
 		this.component = component;
 		this.lastModified = lastModified;
 	}
 
+	/**
+	 * Creates a new file channel with the given open options.
+	 *
+	 * @param options The options to use to open the file channel. For the most part these will be passed through to the ciphertext channel.
+	 * @return A new file channel. Ideally used in a try-with-resource statement. If the channel is not properly closed, this OpenCryptoFile will stay open indefinite.
+	 * @throws IOException
+	 */
 	public synchronized FileChannel newFileChannel(EffectiveOpenOptions options) throws IOException {
 		Path path = currentFilePath.get();
 
@@ -58,6 +74,7 @@ public synchronized FileChannel newFileChannel(EffectiveOpenOptions options) thr
 		CleartextFileChannel cleartextFileChannel = null;
 		try {
 			ciphertextFileChannel = path.getFileSystem().provider().newFileChannel(path, options.createOpenOptionsForEncryptedFile());
+			initFileSize(ciphertextFileChannel);
 			ChannelComponent channelComponent = component.newChannelComponent() //
 					.ciphertextChannel(ciphertextFileChannel) //
 					.openOptions(options) //
@@ -76,7 +93,36 @@ public synchronized FileChannel newFileChannel(EffectiveOpenOptions options) thr
 		return cleartextFileChannel;
 	}
 
+	/**
+	 * Called by {@link #newFileChannel(EffectiveOpenOptions)} to determine the fileSize.
+	 * <p>
+	 * Before the size is initialized (i.e. before a channel has been created), {@link #size()} must not be called.
+	 * <p>
+	 * Initialization happens at most once per open file. Subsequent invocations are no-ops.
+	 */
+	private void initFileSize(FileChannel ciphertextFileChannel) throws IOException {
+		if (fileSize.get() == -1l) {
+			LOG.trace("First channel for this openFile. Initializing file size...");
+			long cleartextSize = 0l;
+			try {
+				long ciphertextSize = ciphertextFileChannel.size();
+				if (ciphertextSize > 0l) {
+					cleartextSize = Cryptors.cleartextSize(ciphertextSize - cryptor.fileHeaderCryptor().headerSize(), cryptor);
+				}
+			} catch (IllegalArgumentException e) {
+				LOG.warn("Invalid cipher text file size. Assuming empty file.", e);
+				assert cleartextSize == 0l;
+			}
+			fileSize.compareAndSet(-1l, cleartextSize);
+		}
+	}
+
+	/**
+	 * @return The size of the opened file
+	 * @throws IllegalStateException If the OpenCryptoFile {@link OpenCryptoFiles#getOrCreate(Path) has been created} without {@link #newFileChannel(EffectiveOpenOptions) creating a file channel} next.
+	 */
 	public long size() {
+		Preconditions.checkState(fileSize.get() != -1l, "size must only be called after a FileChannel is created for this OpenCryptoFile");
 		return fileSize.get();
 	}
 

src/main/java/org/cryptomator/cryptofs/fh/OpenCryptoFileModule.java

@@ -56,14 +56,9 @@ public AtomicReference<Instant> provideLastModifiedDate(@OriginalOpenFilePath Pa
 	@Provides
 	@OpenFileScoped
 	@OpenFileSize
-	public AtomicLong provideFileSize(@OriginalOpenFilePath Path originalPath, Cryptor cryptor) {
-		long ciphertextSize = readBasicAttributes(originalPath).map(BasicFileAttributes::size).orElse(0l);
-		if (ciphertextSize == 0) {
-			return new AtomicLong();
-		} else {
-			int headerSize = cryptor.fileHeaderCryptor().headerSize();
-			return new AtomicLong(cleartextSize(ciphertextSize - headerSize, cryptor));
-		}
+	public AtomicLong provideFileSize() {
+		// will be initialized when first creating a FileChannel. See OpenCryptoFile#size()
+		return new AtomicLong(-1l);
 	}
 
 	private Optional<BasicFileAttributes> readBasicAttributes(Path path) {

src/main/java/org/cryptomator/cryptofs/fh/OpenCryptoFiles.java

@@ -40,11 +40,27 @@ public class OpenCryptoFiles implements Closeable {
 		this.component = component;
 	}
 
+	/**
+	 * Gets an OpenCryptoFile (if any is opened) without creating it.
+	 * <p>
+	 * Useful if you don't want to create any FileChannel but want to check whether this file is currently opened (e.g. to get its current {@link OpenCryptoFile#size()}).
+	 *
+	 * @param ciphertextPath Path of the file which might have been opened
+	 * @return The OpenCryptoFile if opened or an empty Optional otherwise.
+	 */
 	public Optional<OpenCryptoFile> get(Path ciphertextPath) {
 		Path normalizedPath = ciphertextPath.toAbsolutePath().normalize();
 		return Optional.ofNullable(openCryptoFiles.get(normalizedPath));
 	}
 
+	/**
+	 * Opens a file to {@link OpenCryptoFile#newFileChannel(EffectiveOpenOptions) retrieve a FileChannel}. If this file is already opened, a shared instance is returned.
+	 * Getting the file channel should be the next invocation, since the {@link OpenFileScoped lifecycle} of the OpenFile strictly depends on the lifecycle of the channel.
+	 *
+	 * @param ciphertextPath Path of the file to open
+	 * @return The opened file.
+	 * @see #get(Path)
+	 */
 	public OpenCryptoFile getOrCreate(Path ciphertextPath) {
 		Path normalizedPath = ciphertextPath.toAbsolutePath().normalize();
 		return openCryptoFiles.computeIfAbsent(normalizedPath, this::create); // computeIfAbsent is atomic, "create" is called at most once

src/main/java/org/cryptomator/cryptofs/fh/OpenFileScoped.java

@@ -1,12 +1,16 @@
 package org.cryptomator.cryptofs.fh;
 
-import static java.lang.annotation.RetentionPolicy.RUNTIME;
-
+import javax.inject.Scope;
 import java.lang.annotation.Documented;
 import java.lang.annotation.Retention;
 
-import javax.inject.Scope;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
 
+/**
+ * An OpenFile is {@link OpenCryptoFiles#getOrCreate(java.nio.file.Path) created} with the sole purpose of opening a FileChannel.
+ * <p>
+ * When the last active file channel is closed, the OpenFile is closed. I.e. it is strictly required for anyone creating an OpenFile to get, use and close a FileChannel.
+ */
 @Scope
 @Documented
 @Retention(RUNTIME)

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

@@ -58,6 +58,35 @@ public static void teardownClass() throws IOException {
 		inMemoryFs.close();
 	}
 
+	// tests https://github.com/cryptomator/cryptofs/issues/50
+	@Test
+	public void testReadWhileStillWriting() throws IOException {
+		Path file = filePath(nextFileId());
+
+		try (FileChannel ch1 = FileChannel.open(file, CREATE_NEW, WRITE)) {
+			// it actually matters that the channel writes more than one chunk size (32k)
+			ch1.write(repeat(1).times(35000).asByteBuffer(), 0);
+			try (FileChannel ch2 = FileChannel.open(file, READ)) {
+				ch1.write(repeat(2).times(5000).asByteBuffer(), 35000);
+			}
+		}
+
+		try (FileChannel ch1 = FileChannel.open(file, READ)) {
+			ByteBuffer buffer = ByteBuffer.allocate(40000);
+			int result = ch1.read(buffer);
+			Assertions.assertEquals(40000, result);
+			Assertions.assertEquals(EOF, ch1.read(ByteBuffer.allocate(0)));
+			buffer.flip();
+			for (int i = 0; i < 40000; i++) {
+				if (i < 35000) {
+					Assertions.assertEquals(1, buffer.get(i), format("byte(%d) = 1", i));
+				} else {
+					Assertions.assertEquals(2, buffer.get(i), format("byte(%d) = 2", i));
+				}
+			}
+		}
+	}
+
 	// tests https://github.com/cryptomator/cryptofs/issues/48
 	@Test
 	public void testTruncateExistingWhileStillOpen() throws IOException {
@@ -87,6 +116,8 @@ public void testFileSizeIsZeroAfterCreatingFileChannel() throws IOException {
 			Assertions.assertEquals(0, channel.size());
 			Assertions.assertEquals(0, Files.size(filePath(fileId)));
 		}
+
+		Assertions.assertEquals(0, Files.size(filePath(fileId)));
 	}
 
 	// tests https://github.com/cryptomator/cryptofs/issues/26
@@ -99,6 +130,8 @@ public void testFileSizeIsTenAfterWritingTenBytes() throws IOException {
 			Assertions.assertEquals(10, channel.size());
 			Assertions.assertEquals(10, Files.size(filePath(fileId)));
 		}
+
+		Assertions.assertEquals(10, Files.size(filePath(fileId)));
 	}
 
 	@Test
@@ -253,13 +286,13 @@ public void testAppend(int dataSize) throws IOException {
 			Assertions.assertEquals(dataSize, channel.size());
 			channel.write(repeat(1).times(dataSize).asByteBuffer());
 			channel.write(repeat(1).times(dataSize).asByteBuffer());
-			Assertions.assertEquals(3*dataSize, channel.size());
+			Assertions.assertEquals(3 * dataSize, channel.size());
 		}
 
 		try (FileChannel channel = readableChannel(fileId)) {
-			ByteBuffer buffer = ByteBuffer.allocate(3*dataSize);
+			ByteBuffer buffer = ByteBuffer.allocate(3 * dataSize);
 			int result = channel.read(buffer);
-			Assertions.assertEquals(3*dataSize, result);
+			Assertions.assertEquals(3 * dataSize, result);
 			Assertions.assertEquals(EOF, channel.read(ByteBuffer.allocate(0)));
 		}
 	}

src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java

@@ -75,7 +75,6 @@ public void setUp() throws IOException {
 		when(fileHeaderCryptor.headerSize()).thenReturn(50);
 		when(fileContentCryptor.cleartextChunkSize()).thenReturn(100);
 		when(fileContentCryptor.ciphertextChunkSize()).thenReturn(110);
-		when(ciphertextFileChannel.size()).thenReturn(160l); // initial cleartext size will be 100
 		when(attributeViewSupplier.get()).thenReturn(attributeView);
 		when(readWriteLock.readLock()).thenReturn(readLock);
 		when(readWriteLock.writeLock()).thenReturn(writeLock);
@@ -291,7 +290,7 @@ public void testReadFailsIfNotReadable() throws IOException {
 
 		@Test
 		public void testReadFromMultipleChunks() throws IOException {
-			when(ciphertextFileChannel.size()).thenReturn(5_500_000_160l); // initial cleartext size will be 5_000_000_100l
+			fileSize.set(5_000_000_100l); // initial cleartext size will be 5_000_000_100l
 			when(options.readable()).thenReturn(true);
 
 			inTest = new CleartextFileChannel(ciphertextFileChannel, headerLoader, readWriteLock, cryptor, chunkCache, options, fileSize, lastModified, attributeViewSupplier, exceptionsDuringWrite, closeListener, stats);