From a959b99376ea650a15aa83ad93dbe24ad743dc89 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Thu, 16 Dec 2021 13:22:48 +0100 Subject: [PATCH 01/28] [ci skip] bump to SNAPSHOT version --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 498d7825..42fbb42b 100644 --- a/pom.xml +++ b/pom.xml @@ -2,7 +2,7 @@ 4.0.0 org.cryptomator cryptofs - 2.3.1 + 2.4.0-SNAPSHOT Cryptomator Crypto Filesystem This library provides the Java filesystem provider used by Cryptomator. https://github.com/cryptomator/cryptofs From fe23ff483c26484c4e6a456b7d6797596f689b02 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Thu, 16 Dec 2021 14:14:46 +0100 Subject: [PATCH 02/28] clean up --- .../cryptofs/health/shortened/NotDecodableLongName.java | 6 ++++++ .../cryptofs/health/shortened/ShortenedNamesCheck.java | 2 +- .../cryptofs/health/shortened/TrailingBytesInNameFile.java | 5 +++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/cryptomator/cryptofs/health/shortened/NotDecodableLongName.java b/src/main/java/org/cryptomator/cryptofs/health/shortened/NotDecodableLongName.java index f3c5eb92..f8f1012e 100644 --- a/src/main/java/org/cryptomator/cryptofs/health/shortened/NotDecodableLongName.java +++ b/src/main/java/org/cryptomator/cryptofs/health/shortened/NotDecodableLongName.java @@ -1,5 +1,6 @@ package org.cryptomator.cryptofs.health.shortened; +import org.cryptomator.cryptofs.common.Constants; import org.cryptomator.cryptofs.health.api.CommonDetailKeys; import org.cryptomator.cryptofs.health.api.DiagnosticResult; @@ -40,4 +41,9 @@ public Map details() { return Map.of(CommonDetailKeys.ENCRYPTED_PATH, nameFile.toString(), // "Stored String", longName); } + + @Override + public String toString() { + return String.format("String \"%s\" stored in %s is not a valid Cryptomator filename.", longName, nameFile); + } } diff --git a/src/main/java/org/cryptomator/cryptofs/health/shortened/ShortenedNamesCheck.java b/src/main/java/org/cryptomator/cryptofs/health/shortened/ShortenedNamesCheck.java index 8deec732..bc252716 100644 --- a/src/main/java/org/cryptomator/cryptofs/health/shortened/ShortenedNamesCheck.java +++ b/src/main/java/org/cryptomator/cryptofs/health/shortened/ShortenedNamesCheck.java @@ -132,7 +132,7 @@ SyntaxResult checkSyntax(String toAnalyse) { return SyntaxResult.INVALID; } - if (toAnalyse.substring(posObligatoryC9rString).length() > Constants.CRYPTOMATOR_FILE_SUFFIX.length()) { + if (toAnalyse.length() > posObligatoryC9rString + Constants.CRYPTOMATOR_FILE_SUFFIX.length()) { return SyntaxResult.TRAILING_BYTES; } diff --git a/src/main/java/org/cryptomator/cryptofs/health/shortened/TrailingBytesInNameFile.java b/src/main/java/org/cryptomator/cryptofs/health/shortened/TrailingBytesInNameFile.java index 74e74bbd..3cc7bb17 100644 --- a/src/main/java/org/cryptomator/cryptofs/health/shortened/TrailingBytesInNameFile.java +++ b/src/main/java/org/cryptomator/cryptofs/health/shortened/TrailingBytesInNameFile.java @@ -47,4 +47,9 @@ public Map details() { return Map.of(CommonDetailKeys.ENCRYPTED_PATH, nameFile.toString(), // "Encrypted Long Name", longName); } + + @Override + public String toString() { + return String.format("Encrypted filename \"%s\" stored in %s contains trailing bytes.", longName, nameFile); + } } From 6d4f8d8c69d22192f9518955d7aaf4d1be3da736 Mon Sep 17 00:00:00 2001 From: Snyk bot Date: Fri, 14 Jan 2022 21:06:16 +0000 Subject: [PATCH 03/28] bump java-jwt version (#123) The following vulnerabilities are fixed with an upgrade: - https://snyk.io/vuln/SNYK-JAVA-COMFASTERXMLJACKSONCORE-2326698 --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 42fbb42b..383b8ab1 100644 --- a/pom.xml +++ b/pom.xml @@ -19,7 +19,7 @@ 2.0.3 - 3.18.1 + 3.18.3 2.37 30.1.1-jre 1.7.31 @@ -35,7 +35,7 @@ 1.6.8 - + From 0eff5927fb6f4410425f9ffb781fb0a85dfe2bbd Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Tue, 25 Jan 2022 21:13:10 +0100 Subject: [PATCH 04/28] cleanup --- .../java/org/cryptomator/cryptofs/CopyOperationTest.java | 1 - .../CryptoFileChannelWriteReadIntegrationTest.java | 4 ++-- src/test/java/org/cryptomator/cryptofs/CryptoPathTest.java | 1 - .../java/org/cryptomator/cryptofs/MoveOperationTest.java | 1 - .../cryptomator/cryptofs/ch/CleartextFileChannelTest.java | 2 +- .../cryptomator/cryptofs/matchers/ByteBufferMatcher.java | 7 +++---- 6 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/test/java/org/cryptomator/cryptofs/CopyOperationTest.java b/src/test/java/org/cryptomator/cryptofs/CopyOperationTest.java index bd0c71aa..2d565f96 100644 --- a/src/test/java/org/cryptomator/cryptofs/CopyOperationTest.java +++ b/src/test/java/org/cryptomator/cryptofs/CopyOperationTest.java @@ -26,7 +26,6 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; -import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; public class CopyOperationTest { diff --git a/src/test/java/org/cryptomator/cryptofs/CryptoFileChannelWriteReadIntegrationTest.java b/src/test/java/org/cryptomator/cryptofs/CryptoFileChannelWriteReadIntegrationTest.java index 3b12c4c0..2c1ee279 100644 --- a/src/test/java/org/cryptomator/cryptofs/CryptoFileChannelWriteReadIntegrationTest.java +++ b/src/test/java/org/cryptomator/cryptofs/CryptoFileChannelWriteReadIntegrationTest.java @@ -144,7 +144,7 @@ public void beforeAll() throws IOException, MasterkeyLoadingFailedException { Mockito.when(keyLoader.loadKey(Mockito.any())).thenAnswer(ignored -> new Masterkey(new byte[64])); var properties = cryptoFileSystemProperties().withKeyLoader(keyLoader).build(); CryptoFileSystemProvider.initialize(vaultPath, properties, URI.create("test:key")); - fileSystem = new CryptoFileSystemProvider().newFileSystem(vaultPath, properties); + fileSystem = CryptoFileSystemProvider.newFileSystem(vaultPath, properties); file = fileSystem.getPath("/test.txt"); } @@ -179,7 +179,7 @@ public void testTryLockEmptyChannel() throws IOException { // tests https://github.com/cryptomator/cryptofs/issues/55 @Test - public void testCreateNewFileSetsLastModifiedToNow() throws IOException, InterruptedException { + public void testCreateNewFileSetsLastModifiedToNow() throws IOException { Instant t0, t1, t2; t0 = Instant.now().truncatedTo(ChronoUnit.SECONDS); diff --git a/src/test/java/org/cryptomator/cryptofs/CryptoPathTest.java b/src/test/java/org/cryptomator/cryptofs/CryptoPathTest.java index 2900032a..77fd9237 100644 --- a/src/test/java/org/cryptomator/cryptofs/CryptoPathTest.java +++ b/src/test/java/org/cryptomator/cryptofs/CryptoPathTest.java @@ -36,7 +36,6 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verifyNoInteractions; -import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; public class CryptoPathTest { diff --git a/src/test/java/org/cryptomator/cryptofs/MoveOperationTest.java b/src/test/java/org/cryptomator/cryptofs/MoveOperationTest.java index 4744bb43..92d89692 100644 --- a/src/test/java/org/cryptomator/cryptofs/MoveOperationTest.java +++ b/src/test/java/org/cryptomator/cryptofs/MoveOperationTest.java @@ -25,7 +25,6 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; -import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; public class MoveOperationTest { diff --git a/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java b/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java index 2856e31f..7a30e5dd 100644 --- a/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java +++ b/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java @@ -61,7 +61,7 @@ public class CleartextFileChannelTest { private boolean mustWriteHeader = true; private EffectiveOpenOptions options = mock(EffectiveOpenOptions.class); private AtomicLong fileSize = new AtomicLong(100); - private AtomicReference lastModified = new AtomicReference(Instant.ofEpochMilli(0)); + private AtomicReference lastModified = new AtomicReference<>(Instant.ofEpochMilli(0)); private Supplier attributeViewSupplier = mock(Supplier.class); private BasicFileAttributeView attributeView = mock(BasicFileAttributeView.class); private ExceptionsDuringWrite exceptionsDuringWrite = mock(ExceptionsDuringWrite.class); diff --git a/src/test/java/org/cryptomator/cryptofs/matchers/ByteBufferMatcher.java b/src/test/java/org/cryptomator/cryptofs/matchers/ByteBufferMatcher.java index e49b44d4..49bf6a54 100644 --- a/src/test/java/org/cryptomator/cryptofs/matchers/ByteBufferMatcher.java +++ b/src/test/java/org/cryptomator/cryptofs/matchers/ByteBufferMatcher.java @@ -27,10 +27,9 @@ public static Matcher contains(ByteBuffer data) { public static Matcher contains(byte[] data) { return matcher("remaining data", is(data), buffer -> { - int position = buffer.position(); - byte[] remaining = new byte[buffer.remaining()]; - buffer.get(remaining); - buffer.position(position); + ByteBuffer buf = buffer.asReadOnlyBuffer(); + byte[] remaining = new byte[buf.remaining()]; + buf.get(remaining); return remaining; }); } From 8b7f6ba82b29f65cf52500543338334fd1c4bc55 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Tue, 25 Jan 2022 21:34:34 +0100 Subject: [PATCH 05/28] Use pooled chunks to reduce allocations --- .../cryptofs/ch/CleartextFileChannel.java | 20 +- .../cryptomator/cryptofs/fh/BufferPool.java | 57 +++++ .../cryptomator/cryptofs/fh/ChunkCache.java | 5 +- .../cryptomator/cryptofs/fh/ChunkData.java | 92 ++----- .../cryptomator/cryptofs/fh/ChunkLoader.java | 34 ++- .../cryptomator/cryptofs/fh/ChunkSaver.java | 12 +- .../cryptofs/ch/CleartextFileChannelTest.java | 3 +- .../cryptofs/fh/ChunkCacheTest.java | 24 +- .../cryptofs/fh/ChunkDataTest.java | 224 +----------------- .../cryptofs/fh/ChunkLoaderTest.java | 64 +++-- .../cryptofs/fh/ChunkSaverTest.java | 38 ++- 11 files changed, 209 insertions(+), 364 deletions(-) create mode 100644 src/main/java/org/cryptomator/cryptofs/fh/BufferPool.java diff --git a/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java b/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java index 4700182b..ab855aea 100644 --- a/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java +++ b/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java @@ -104,9 +104,9 @@ protected int readLocked(ByteBuffer dst, long position) throws IOException { long pos = position + read; long chunkIndex = pos / payloadSize; // floor by int-truncation int offsetInChunk = (int) (pos % payloadSize); // known to fit in int, because payloadSize is int - int len = min(dst.remaining(), payloadSize - offsetInChunk); // known to fit in int, because second argument is int - final ChunkData chunkData = chunkCache.get(chunkIndex); - chunkData.copyDataStartingAt(offsetInChunk).to(dst); + ByteBuffer data = chunkCache.get(chunkIndex).data().duplicate().position(offsetInChunk); + int len = min(dst.remaining(), data.remaining()); // known to fit in int, because second argument is int + dst.put(data.limit(data.position() + len)); read += len; } dst.limit(origLimit); @@ -148,8 +148,10 @@ private long writeLockedInternal(ByteSource src, long position) throws IOExcepti assert len <= cleartextChunkSize; if (offsetInChunk == 0 && len == cleartextChunkSize) { // complete chunk, no need to load and decrypt from file - ChunkData chunkData = ChunkData.emptyWithSize(cleartextChunkSize); - chunkData.copyData().from(src); + ByteBuffer cleartextChunk = ByteBuffer.allocate(cleartextChunkSize); // TODO: use BufferPool + src.copyTo(cleartextChunk); + cleartextChunk.flip(); + ChunkData chunkData = new ChunkData(cleartextChunk, true); chunkCache.set(chunkIndex, chunkData); } else { /* @@ -158,7 +160,9 @@ private long writeLockedInternal(ByteSource src, long position) throws IOExcepti * It would suffice if store the written data and do reading when storing the chunk. */ ChunkData chunkData = chunkCache.get(chunkIndex); - chunkData.copyDataStartingAt(offsetInChunk).from(src); + chunkData.data().limit(Math.max(chunkData.data().limit(), offsetInChunk + len)); // increase limit (if needed) + src.copyTo(chunkData.data().duplicate().position(offsetInChunk)); // work on duplicate using correct offset + chunkData.dirty().set(true); } written += len; } @@ -190,7 +194,9 @@ protected void truncateLocked(long newSize) throws IOException { long indexOfLastChunk = (newSize + cleartextChunkSize - 1) / cleartextChunkSize - 1; int sizeOfIncompleteChunk = (int) (newSize % cleartextChunkSize); // known to fit in int, because cleartextChunkSize is int if (sizeOfIncompleteChunk > 0) { - chunkCache.get(indexOfLastChunk).truncate(sizeOfIncompleteChunk); + var chunk = chunkCache.get(indexOfLastChunk); + chunk.data().limit(sizeOfIncompleteChunk); + chunk.dirty().set(true); } long ciphertextFileSize = cryptor.fileHeaderCryptor().headerSize() + cryptor.fileContentCryptor().ciphertextSize(newSize); chunkCache.invalidateAll(); // make sure no chunks _after_ newSize exist that would otherwise be written during the next cache eviction diff --git a/src/main/java/org/cryptomator/cryptofs/fh/BufferPool.java b/src/main/java/org/cryptomator/cryptofs/fh/BufferPool.java new file mode 100644 index 00000000..73585547 --- /dev/null +++ b/src/main/java/org/cryptomator/cryptofs/fh/BufferPool.java @@ -0,0 +1,57 @@ +package org.cryptomator.cryptofs.fh; + +import org.cryptomator.cryptofs.CryptoFileSystemScoped; +import org.cryptomator.cryptolib.api.Cryptor; + +import javax.inject.Inject; +import java.lang.ref.WeakReference; +import java.nio.ByteBuffer; +import java.util.Optional; +import java.util.Queue; +import java.util.concurrent.ConcurrentLinkedQueue; + +/** + * A pool of ByteBuffers for cleartext and ciphertext chunks to avoid on-heap allocation. + */ +@CryptoFileSystemScoped +public class BufferPool { + + private final int ciphertextChunkSize; + private final int cleartextChunkSize; + private final Queue> ciphertextBuffers = new ConcurrentLinkedQueue<>(); + private final Queue> cleartextBuffers = new ConcurrentLinkedQueue<>(); + + @Inject + public BufferPool(Cryptor cryptor) { + this.ciphertextChunkSize = cryptor.fileContentCryptor().ciphertextChunkSize(); + this.cleartextChunkSize = cryptor.fileContentCryptor().cleartextChunkSize(); + } + + private Optional dequeueFrom(Queue> queue) { + WeakReference ref; + while ((ref = queue.poll()) != null) { + ByteBuffer cached = ref.get(); + if (cached != null) { + cached.clear(); + return Optional.of(cached); + } + } + return Optional.empty(); + } + + public ByteBuffer getCiphertextBuffer() { + return dequeueFrom(ciphertextBuffers).orElseGet(() -> ByteBuffer.allocate(ciphertextChunkSize)); + } + + public ByteBuffer getCleartextBuffer() { + return dequeueFrom(cleartextBuffers).orElseGet(() -> ByteBuffer.allocate(cleartextChunkSize)); + } + + public void recycle(ByteBuffer buffer) { + if (buffer.capacity() == ciphertextChunkSize) { + ciphertextBuffers.add(new WeakReference<>(buffer)); + } else if (buffer.capacity() == cleartextChunkSize) { + cleartextBuffers.add(new WeakReference<>(buffer)); + } + } +} diff --git a/src/main/java/org/cryptomator/cryptofs/fh/ChunkCache.java b/src/main/java/org/cryptomator/cryptofs/fh/ChunkCache.java index aae1bd77..1fb5d459 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/ChunkCache.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/ChunkCache.java @@ -19,13 +19,15 @@ public class ChunkCache { private final ChunkLoader chunkLoader; private final ChunkSaver chunkSaver; private final CryptoFileSystemStats stats; + private final BufferPool bufferPool; private final Cache chunks; @Inject - public ChunkCache(ChunkLoader chunkLoader, ChunkSaver chunkSaver, CryptoFileSystemStats stats) { + public ChunkCache(ChunkLoader chunkLoader, ChunkSaver chunkSaver, CryptoFileSystemStats stats, BufferPool bufferPool) { this.chunkLoader = chunkLoader; this.chunkSaver = chunkSaver; this.stats = stats; + this.bufferPool = bufferPool; this.chunks = CacheBuilder.newBuilder() // .maximumSize(MAX_CACHED_CLEARTEXT_CHUNKS) // .removalListener(this::removeChunk) // @@ -45,6 +47,7 @@ private ChunkData loadChunk(long chunkIndex) throws IOException { private void removeChunk(RemovalNotification removal) { try { chunkSaver.save(removal.getKey(), removal.getValue()); + bufferPool.recycle(removal.getValue().data()); } catch (IOException e) { throw new UncheckedIOException(e); } diff --git a/src/main/java/org/cryptomator/cryptofs/fh/ChunkData.java b/src/main/java/org/cryptomator/cryptofs/fh/ChunkData.java index 5cbc1f7b..9b3281ba 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/ChunkData.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/ChunkData.java @@ -9,93 +9,33 @@ package org.cryptomator.cryptofs.fh; import java.nio.ByteBuffer; +import java.util.concurrent.atomic.AtomicBoolean; -import static java.lang.Math.max; -import static java.lang.Math.min; import static java.lang.String.format; -public class ChunkData { +/** + * A chunk of plaintext data. It has these rules: + *
    + *
  1. Capacity of {@code data} is always the cleartext chunk size
    2. + *
  2. During creation, {@code data}'s limit is the chunk's size (last chunk of a file may be smaller)
    3. + *
  3. Writes need to adjust the limit and mark this chunk dirty
    4. + *
  4. Reads need to respec the limit and must not change it
    5. + *
  5. When no longer used, the cleartext ByteBuffer may be recycled
    6. + *
+ */ +public record ChunkData(ByteBuffer data, AtomicBoolean dirty) { - private final ByteBuffer bytes; - private boolean dirty; - private int length; - - public static ChunkData wrap(ByteBuffer bytes) { - return new ChunkData(bytes, bytes.limit()); - } - - public static ChunkData emptyWithSize(int size) { - return new ChunkData(ByteBuffer.allocate(size), 0); - } - - private ChunkData(ByteBuffer bytes, int length) { - this.bytes = bytes; - this.dirty = false; - this.length = length; + public ChunkData(ByteBuffer data, boolean dirty) { + this(data, new AtomicBoolean(dirty)); } public boolean isDirty() { - return dirty; - } - - public void truncate(int length) { - if (this.length > length) { - this.length = length; - this.dirty = true; - } - } - - public CopyWithoutDirection copyData() { - return copyDataStartingAt(0); - } - - public CopyWithoutDirection copyDataStartingAt(int offset) { - return new CopyWithoutDirection() { - @Override - public void to(ByteBuffer target) { - ByteBuffer buf = bytes.asReadOnlyBuffer(); - buf.limit(min(length, target.remaining() + offset)); - buf.position(offset); - target.put(buf); - } - - @Override - public void from(ByteBuffer source) { - from(ByteSource.from(source)); - } - - @Override - public void from(ByteSource source) { - ByteBuffer buf = bytes.duplicate(); - buf.limit(buf.capacity()); - buf.position(offset); - source.copyTo(buf); - dirty = true; - length = max(length, buf.position()); - } - }; - } - - public ByteBuffer asReadOnlyBuffer() { - ByteBuffer readOnlyBuffer = bytes.asReadOnlyBuffer(); - readOnlyBuffer.position(0); - readOnlyBuffer.limit(length); - return readOnlyBuffer; - } - - public interface CopyWithoutDirection { - - void to(ByteBuffer target); - - void from(ByteBuffer source); - - void from(ByteSource source); - + return dirty.get(); } @Override public String toString() { - return format("ChunkData(dirty: %s, length: %d, capacity: %d)", dirty, length, bytes.capacity()); + return format("ChunkData(dirty: %s, length: %d)", dirty, data.limit()); } } diff --git a/src/main/java/org/cryptomator/cryptofs/fh/ChunkLoader.java b/src/main/java/org/cryptomator/cryptofs/fh/ChunkLoader.java index 278fa68b..b38f247a 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/ChunkLoader.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/ChunkLoader.java @@ -15,38 +15,36 @@ class ChunkLoader { private final ChunkIO ciphertext; private final FileHeaderHolder headerHolder; private final CryptoFileSystemStats stats; + private final BufferPool bufferPool; @Inject - public ChunkLoader(Cryptor cryptor, ChunkIO ciphertext, FileHeaderHolder headerHolder, CryptoFileSystemStats stats) { + public ChunkLoader(Cryptor cryptor, ChunkIO ciphertext, FileHeaderHolder headerHolder, CryptoFileSystemStats stats, BufferPool bufferPool) { this.cryptor = cryptor; this.ciphertext = ciphertext; this.headerHolder = headerHolder; this.stats = stats; + this.bufferPool = bufferPool; } public ChunkData load(Long chunkIndex) throws IOException, AuthenticationFailedException { stats.addChunkCacheMiss(); - int payloadSize = cryptor.fileContentCryptor().cleartextChunkSize(); int chunkSize = cryptor.fileContentCryptor().ciphertextChunkSize(); long ciphertextPos = chunkIndex * chunkSize + cryptor.fileHeaderCryptor().headerSize(); - ByteBuffer ciphertextBuf = ByteBuffer.allocate(chunkSize); - int read = ciphertext.read(ciphertextBuf, ciphertextPos); - if (read == -1) { - // append - return ChunkData.emptyWithSize(payloadSize); - } else { - ciphertextBuf.flip(); - ByteBuffer cleartextBuf = cryptor.fileContentCryptor().decryptChunk(ciphertextBuf, chunkIndex, headerHolder.get(), true); - stats.addBytesDecrypted(cleartextBuf.remaining()); - ByteBuffer cleartextBufWhichCanHoldFullChunk; - if (cleartextBuf.capacity() < payloadSize) { - cleartextBufWhichCanHoldFullChunk = ByteBuffer.allocate(payloadSize); - cleartextBufWhichCanHoldFullChunk.put(cleartextBuf); - cleartextBufWhichCanHoldFullChunk.flip(); + ByteBuffer ciphertextBuf = bufferPool.getCiphertextBuffer(); + ByteBuffer cleartextBuf = bufferPool.getCleartextBuffer(); + try { + int read = ciphertext.read(ciphertextBuf, ciphertextPos); + if (read == -1) { + cleartextBuf.limit(0); } else { - cleartextBufWhichCanHoldFullChunk = cleartextBuf; + ciphertextBuf.flip(); + cryptor.fileContentCryptor().decryptChunk(ciphertextBuf, cleartextBuf, chunkIndex, headerHolder.get(), true); + cleartextBuf.flip(); + stats.addBytesDecrypted(cleartextBuf.remaining()); } - return ChunkData.wrap(cleartextBufWhichCanHoldFullChunk); + return new ChunkData(cleartextBuf, false); + } finally { + bufferPool.recycle(ciphertextBuf); } } diff --git a/src/main/java/org/cryptomator/cryptofs/fh/ChunkSaver.java b/src/main/java/org/cryptomator/cryptofs/fh/ChunkSaver.java index 88bbf819..3fed4f2d 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/ChunkSaver.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/ChunkSaver.java @@ -15,26 +15,32 @@ class ChunkSaver { private final FileHeaderHolder headerHolder; private final ExceptionsDuringWrite exceptionsDuringWrite; private final CryptoFileSystemStats stats; + private final BufferPool bufferPool; @Inject - public ChunkSaver(Cryptor cryptor, ChunkIO ciphertext, FileHeaderHolder headerHolder, ExceptionsDuringWrite exceptionsDuringWrite, CryptoFileSystemStats stats) { + public ChunkSaver(Cryptor cryptor, ChunkIO ciphertext, FileHeaderHolder headerHolder, ExceptionsDuringWrite exceptionsDuringWrite, CryptoFileSystemStats stats, BufferPool bufferPool) { this.cryptor = cryptor; this.ciphertext = ciphertext; this.headerHolder = headerHolder; this.exceptionsDuringWrite = exceptionsDuringWrite; this.stats = stats; + this.bufferPool = bufferPool; } public void save(long chunkIndex, ChunkData chunkData) throws IOException { if (chunkData.isDirty()) { long ciphertextPos = chunkIndex * cryptor.fileContentCryptor().ciphertextChunkSize() + cryptor.fileHeaderCryptor().headerSize(); - ByteBuffer cleartextBuf = chunkData.asReadOnlyBuffer(); + ByteBuffer cleartextBuf = chunkData.data().duplicate(); stats.addBytesEncrypted(cleartextBuf.remaining()); - ByteBuffer ciphertextBuf = cryptor.fileContentCryptor().encryptChunk(cleartextBuf, chunkIndex, headerHolder.get()); + ByteBuffer ciphertextBuf = bufferPool.getCiphertextBuffer(); try { + cryptor.fileContentCryptor().encryptChunk(cleartextBuf, ciphertextBuf, chunkIndex, headerHolder.get()); + ciphertextBuf.flip(); ciphertext.write(ciphertextBuf, ciphertextPos); } catch (IOException e) { exceptionsDuringWrite.add(e); + } finally { + bufferPool.recycle(ciphertextBuf); } // unchecked exceptions will be propagated to the thread causing removal } } diff --git a/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java b/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java index 7a30e5dd..d2692a79 100644 --- a/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java +++ b/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java @@ -74,7 +74,7 @@ public class CleartextFileChannelTest { public void setUp() throws IOException { when(cryptor.fileHeaderCryptor()).thenReturn(fileHeaderCryptor); when(cryptor.fileContentCryptor()).thenReturn(fileContentCryptor); - when(chunkCache.get(Mockito.anyLong())).then(invocation -> ChunkData.wrap(ByteBuffer.allocate(100))); + when(chunkCache.get(Mockito.anyLong())).then(invocation -> new ChunkData(ByteBuffer.allocate(100), false)); when(fileHeaderCryptor.headerSize()).thenReturn(50); when(fileContentCryptor.cleartextChunkSize()).thenReturn(100); when(fileContentCryptor.ciphertextChunkSize()).thenReturn(110); @@ -135,7 +135,6 @@ public void testTruncateKeepsPositionIfNewSizeGreaterCurrentPosition() throws IO inTest.truncate(newSize); - MatcherAssert.assertThat(inTest.position(), is(currentPosition)); } diff --git a/src/test/java/org/cryptomator/cryptofs/fh/ChunkCacheTest.java b/src/test/java/org/cryptomator/cryptofs/fh/ChunkCacheTest.java index 2e5bc541..b6d22ee8 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/ChunkCacheTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/ChunkCacheTest.java @@ -7,6 +7,7 @@ import org.mockito.Mockito; import java.io.IOException; +import java.nio.ByteBuffer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -15,11 +16,11 @@ public class ChunkCacheTest { - private final ChunkLoader chunkLoader = mock(ChunkLoader.class); private final ChunkSaver chunkSaver = mock(ChunkSaver.class); private final CryptoFileSystemStats stats = mock(CryptoFileSystemStats.class); - private final ChunkCache inTest = new ChunkCache(chunkLoader, chunkSaver, stats); + private final BufferPool bufferPool = mock(BufferPool.class); + private final ChunkCache inTest = new ChunkCache(chunkLoader, chunkSaver, stats, bufferPool); @Test public void testGetInvokesLoaderIfEntryNotInCache() throws IOException, AuthenticationFailedException { @@ -68,6 +69,7 @@ public void testGetInvokesSaverIfMaxEntriesInCacheAreReachedAndAnEntryNotInCache verify(stats).addChunkCacheAccess(); verify(chunkSaver).save(firstIndex, firstData); + verify(bufferPool).recycle(firstData.data()); verifyNoMoreInteractions(chunkSaver); } @@ -84,6 +86,7 @@ public void testGetInvokesSaverIfMaxEntriesInCacheAreReachedAndAnEntryNotInCache inTest.set(indexNotInCache, mock(ChunkData.class)); verify(chunkSaver).save(firstIndex, firstData); + verify(bufferPool).recycle(firstData.data()); verifyNoMoreInteractions(chunkSaver); } @@ -100,6 +103,7 @@ public void testGetInvokesSaverIfMaxEntriesInCacheAreReachedAndAnEntryInCacheIsS inTest.set(firstIndex, mock(ChunkData.class)); verify(chunkSaver).save(firstIndex, firstData); + verify(bufferPool).recycle(firstData.data()); verifyNoMoreInteractions(chunkSaver); } @@ -131,16 +135,20 @@ public void testGetThrowsUncheckedExceptionFromLoader() throws IOException, Auth public void testInvalidateAllInvokesSaverForAllEntriesInCache() throws IOException, AuthenticationFailedException { long index = 42L; long index2 = 43L; - ChunkData data = mock(ChunkData.class); - ChunkData data2 = mock(ChunkData.class); - when(chunkLoader.load(index)).thenReturn(data); + ChunkData chunk1 = mock(ChunkData.class); + ChunkData chunk2 = mock(ChunkData.class); + when(chunk1.data()).thenReturn(ByteBuffer.allocate(42)); + when(chunk2.data()).thenReturn(ByteBuffer.allocate(23)); + when(chunkLoader.load(index)).thenReturn(chunk1); inTest.get(index); - inTest.set(index2, data2); + inTest.set(index2, chunk2); inTest.invalidateAll(); - verify(chunkSaver).save(index, data); - verify(chunkSaver).save(index2, data2); + verify(chunkSaver).save(index, chunk1); + verify(bufferPool).recycle(chunk1.data()); + verify(chunkSaver).save(index2, chunk2); + verify(bufferPool).recycle(chunk2.data()); } @Test diff --git a/src/test/java/org/cryptomator/cryptofs/fh/ChunkDataTest.java b/src/test/java/org/cryptomator/cryptofs/fh/ChunkDataTest.java index 99229cdb..9cf69f3c 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/ChunkDataTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/ChunkDataTest.java @@ -1,10 +1,7 @@ package org.cryptomator.cryptofs.fh; -import org.hamcrest.MatcherAssert; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.MethodSource; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; @@ -13,124 +10,42 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.atomic.LongAdder; -import java.util.stream.Stream; -import static org.cryptomator.cryptofs.matchers.ByteBufferMatcher.contains; import static org.cryptomator.cryptofs.util.ByteBuffers.repeat; -import static org.hamcrest.Matchers.is; public class ChunkDataTest { - private static final int SIZE = 200; - - public static Stream waysToCreateEmptyChunkData() { - return Stream.of( - new CreateEmptyChunkData(), // - new WrapEmptyBuffer() // - ); - } - - public static Stream waysToCreateChunkDataWithContent() { // - return Stream.of( - new WrapBuffer(), // - new CreateEmptyChunkDataAndCopyFromBuffer(), // - new WrapEmptyBufferAndCopyFromBuffer(), // - new WrapPartOfBufferAndCopyRemainingFromBuffer() // - ); - } - @Test public void testChunkDataWrappingBufferIsNotDirty() { ByteBuffer buffer = repeat(3).times(200).asByteBuffer(); - ChunkData inTest = ChunkData.wrap(buffer); + ChunkData inTest = new ChunkData(buffer, false); Assertions.assertFalse(inTest.isDirty()); } @Test public void testEmptyChunkDataIsNotDirty() { - ChunkData inTest = ChunkData.emptyWithSize(200); + ByteBuffer buffer = ByteBuffer.allocate(0); - Assertions.assertFalse(inTest.isDirty()); - } + ChunkData inTest = new ChunkData(buffer, false); - @Test - public void testWrittenChunkDataIsDirty() { - ChunkData inTest = ChunkData.emptyWithSize(200); - inTest.copyData().from(repeat(3).times(200).asByteBuffer()); - - Assertions.assertTrue(inTest.isDirty()); + Assertions.assertFalse(inTest.isDirty()); } @Test public void testToString() { - ChunkData inTest = ChunkData.emptyWithSize(150); - inTest.copyDataStartingAt(50).from(repeat(3).times(50).asByteBuffer()); - - MatcherAssert.assertThat(inTest.toString(), is("ChunkData(dirty: true, length: 100, capacity: 150)")); - } - - @ParameterizedTest - @MethodSource("waysToCreateEmptyChunkData") - public void testAsReadOnlyBufferReturnsEmptyBufferIfEmpty(WayToCreateEmptyChunkData wayToCreateEmptyChunkData) { - ChunkData inTest = wayToCreateEmptyChunkData.create(); - - MatcherAssert.assertThat(inTest.asReadOnlyBuffer(), contains(new byte[0])); - } - - @ParameterizedTest - @MethodSource("waysToCreateChunkDataWithContent") - public void testAsReadOnlyBufferReturnsContent(WayToCreateChunkDataWithContent wayToCreateChunkDataWithContent) { - ChunkData inTest = wayToCreateChunkDataWithContent.create(repeat(3).times(SIZE).asByteBuffer()); - - MatcherAssert.assertThat(inTest.asReadOnlyBuffer(), contains(repeat(3).times(SIZE).asByteBuffer())); - } - - @ParameterizedTest - @MethodSource("waysToCreateChunkDataWithContent") - public void testCopyToCopiesContent(WayToCreateChunkDataWithContent wayToCreateChunkDataWithContent) { - ChunkData inTest = wayToCreateChunkDataWithContent.create(repeat(3).times(SIZE).asByteBuffer()); - ByteBuffer target = ByteBuffer.allocate(200); - - inTest.copyData().to(target); - target.flip(); + ByteBuffer buffer = ByteBuffer.allocate(100); - MatcherAssert.assertThat(target, contains(repeat(3).times(SIZE).asByteBuffer())); - } - - @ParameterizedTest - @MethodSource("waysToCreateEmptyChunkData") - public void testCopyToCopiesNothingIfEmpty(WayToCreateEmptyChunkData wayToCreateEmptyChunkData) { - ChunkData inTest = wayToCreateEmptyChunkData.create(); - ByteBuffer target = ByteBuffer.allocate(SIZE); - - inTest.copyData().to(target); - - MatcherAssert.assertThat(target, contains(repeat(0).times(SIZE).asByteBuffer())); - } + ChunkData inTest = new ChunkData(buffer, true); - @ParameterizedTest - @MethodSource("waysToCreateChunkDataWithContent") - public void testCopyToWithOffsetCopiesContentFromOffset(WayToCreateChunkDataWithContent wayToCreateChunkDataWithContent) { - int offset = 70; - ChunkData inTest = wayToCreateChunkDataWithContent.create(repeat(3).times(SIZE).asByteBuffer()); - ByteBuffer target = ByteBuffer.allocate(SIZE); - - inTest.copyDataStartingAt(offset).to(target); - - target.limit(SIZE - offset); - target.position(0); - MatcherAssert.assertThat(target, contains(repeat(3).times(SIZE - offset).asByteBuffer())); - target.limit(SIZE); - target.position(SIZE - offset); - MatcherAssert.assertThat(target, contains(repeat(0).times(offset).asByteBuffer())); + Assertions.assertEquals("ChunkData(dirty: true, length: 100)", inTest.toString()); } @Test // https://github.com/cryptomator/cryptofs/issues/85 - public void testRaceConditionsDuringRead() throws InterruptedException { + public void testRaceConditionsDuringRead() { ByteBuffer src = StandardCharsets.US_ASCII.encode("abcdefg"); - ChunkData inTest = ChunkData.wrap(src); + ChunkData inTest = new ChunkData(src, false); int attempts = 4000; int threads = 6; @@ -144,8 +59,7 @@ public void testRaceConditionsDuringRead() throws InterruptedException { executor.execute(() -> { try { ByteBuffer dst = ByteBuffer.allocate(1); - inTest.copyDataStartingAt(offset).to(dst); - dst.flip(); + dst.put(0, inTest.data(), offset, 1); char actual = StandardCharsets.US_ASCII.decode(dst).charAt(0); if (expected == actual) { successfulTests.increment(); @@ -160,122 +74,4 @@ public void testRaceConditionsDuringRead() throws InterruptedException { Assertions.assertEquals(attempts, successfulTests.sum()); } - interface WayToCreateEmptyChunkData { - - ChunkData create(); - - } - - interface WayToCreateChunkDataWithContent { - - ChunkData create(ByteBuffer content); - - } - - private static class CreateEmptyChunkData implements WayToCreateEmptyChunkData { - - @Override - public ChunkData create() { - return ChunkData.emptyWithSize(SIZE); - } - - @Override - public String toString() { - return "CreateEmptyChunkData"; - } - - } - - private static class WrapEmptyBuffer implements WayToCreateEmptyChunkData { - - @Override - public ChunkData create() { - ByteBuffer buffer = ByteBuffer.allocate(SIZE); - buffer.limit(0); - return ChunkData.wrap(buffer); - } - - @Override - public String toString() { - return "WrapEmptyBuffer"; - } - - } - - private static class WrapBuffer implements WayToCreateChunkDataWithContent { - - @Override - public ChunkData create(ByteBuffer content) { - return ChunkData.wrap(content); - } - - @Override - public String toString() { - return "WrapBuffer"; - } - - } - - private static class CreateEmptyChunkDataAndCopyFromBuffer implements WayToCreateChunkDataWithContent { - - @Override - public ChunkData create(ByteBuffer content) { - ChunkData result = ChunkData.emptyWithSize(content.remaining()); - result.copyData().from(content); - return result; - } - - @Override - public String toString() { - return "CreateEmptyChunkDataAndCopyFromBuffer"; - } - - } - - private static class WrapEmptyBufferAndCopyFromBuffer implements WayToCreateChunkDataWithContent { - - @Override - public ChunkData create(ByteBuffer content) { - ByteBuffer buffer = ByteBuffer.allocate(content.remaining()); - buffer.limit(0); - ChunkData result = ChunkData.wrap(buffer); - result.copyData().from(content.asReadOnlyBuffer()); - return result; - } - - @Override - public String toString() { - return "WrapEmptyBufferAndCopyFromBuffer"; - } - - } - - private static class WrapPartOfBufferAndCopyRemainingFromBuffer implements WayToCreateChunkDataWithContent { - - @Override - public ChunkData create(ByteBuffer content) { - int position = content.position(); - int remaining = content.remaining(); - int halfRemaining = remaining / 2; - - ByteBuffer readOnlyContent = content.asReadOnlyBuffer(); - readOnlyContent.limit(position + halfRemaining); - ByteBuffer wrappedContent = ByteBuffer.allocate(remaining); - wrappedContent.put(readOnlyContent); - wrappedContent.limit(halfRemaining); - ChunkData result = ChunkData.wrap(wrappedContent); - - readOnlyContent.limit(position + remaining); - result.copyDataStartingAt(halfRemaining).from(readOnlyContent); - - return result; - } - - @Override - public String toString() { - return "WrapPartOfBufferAndCopyRemainingFromBuffer"; - } - - } - } diff --git a/src/test/java/org/cryptomator/cryptofs/fh/ChunkLoaderTest.java b/src/test/java/org/cryptomator/cryptofs/fh/ChunkLoaderTest.java index 0705da4c..0ce834c9 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/ChunkLoaderTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/ChunkLoaderTest.java @@ -6,8 +6,11 @@ import org.cryptomator.cryptolib.api.FileContentCryptor; import org.cryptomator.cryptolib.api.FileHeader; import org.cryptomator.cryptolib.api.FileHeaderCryptor; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; import org.mockito.stubbing.Answer; import java.io.IOException; @@ -18,7 +21,9 @@ import static org.cryptomator.cryptofs.matchers.ByteBufferMatcher.hasAtLeastRemaining; import static org.cryptomator.cryptofs.util.ByteBuffers.repeat; import static org.hamcrest.MatcherAssert.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -37,7 +42,8 @@ public class ChunkLoaderTest { private final CryptoFileSystemStats stats = mock(CryptoFileSystemStats.class); private final FileHeader header = mock(FileHeader.class); private final FileHeaderHolder headerHolder = mock(FileHeaderHolder.class); - private final ChunkLoader inTest = new ChunkLoader(cryptor, chunkIO, headerHolder, stats); + private final BufferPool bufferPool = mock(BufferPool.class); + private final ChunkLoader inTest = new ChunkLoader(cryptor, chunkIO, headerHolder, stats, bufferPool); @BeforeEach public void setup() throws IOException { @@ -47,58 +53,72 @@ public void setup() throws IOException { when(fileContentCryptor.ciphertextChunkSize()).thenReturn(CIPHERTEXT_CHUNK_SIZE); when(fileContentCryptor.cleartextChunkSize()).thenReturn(CLEARTEXT_CHUNK_SIZE); when(fileHeaderCryptor.headerSize()).thenReturn(HEADER_SIZE); + when(bufferPool.getCiphertextBuffer()).thenAnswer(invocation -> ByteBuffer.allocate(CIPHERTEXT_CHUNK_SIZE)); + when(bufferPool.getCleartextBuffer()).thenAnswer(invocation -> ByteBuffer.allocate(CLEARTEXT_CHUNK_SIZE)); } @Test - public void testChunkLoaderReturnsEmptyDataOfChunkAfterEndOfFile() throws IOException, AuthenticationFailedException { + @DisplayName("load() returns empty chunk when hitting EOF") + public void testLoadReturnsEmptyChunkAfterEOF() throws IOException, AuthenticationFailedException { long chunkIndex = 482L; long chunkOffset = chunkIndex * CIPHERTEXT_CHUNK_SIZE + HEADER_SIZE; when(chunkIO.read(argThat(hasAtLeastRemaining(CIPHERTEXT_CHUNK_SIZE)), eq(chunkOffset))).thenReturn(-1); - ChunkData data = inTest.load(chunkIndex); + ChunkData chunk = inTest.load(chunkIndex); verify(stats).addChunkCacheMiss(); - assertThat(data.asReadOnlyBuffer(), contains(ByteBuffer.allocate(0))); - data.copyData().from(repeat(9).times(CLEARTEXT_CHUNK_SIZE).asByteBuffer()); - assertThat(data.asReadOnlyBuffer(), contains(repeat(9).times(CLEARTEXT_CHUNK_SIZE).asByteBuffer())); // asserts that data has at least CLEARTEXT_CHUNK_SIZE capacity + Assertions.assertEquals(0, chunk.data().remaining()); + Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE, chunk.data().capacity()); } @Test - public void testChunkLoaderReturnsDecryptedDataOfChunkInsideFile() throws IOException, AuthenticationFailedException { + @DisplayName("load() returns full chunk when in middle of file") + public void testLoadReturnsDecryptedDataInsideFile() throws IOException, AuthenticationFailedException { long chunkIndex = 482L; long chunkOffset = chunkIndex * CIPHERTEXT_CHUNK_SIZE + HEADER_SIZE; Supplier decryptedData = () -> repeat(9).times(CLEARTEXT_CHUNK_SIZE).asByteBuffer(); when(chunkIO.read(argThat(hasAtLeastRemaining(CIPHERTEXT_CHUNK_SIZE)), eq(chunkOffset))).then(fillBufferWith((byte) 3, CIPHERTEXT_CHUNK_SIZE)); - when(fileContentCryptor.decryptChunk( // + doAnswer(invocation -> { + ByteBuffer cleartextBuf = invocation.getArgument(1); + cleartextBuf.put(decryptedData.get()); + return null; + }).when(fileContentCryptor).decryptChunk( argThat(contains(repeat(3).times(CIPHERTEXT_CHUNK_SIZE).asByteBuffer())), // - eq(chunkIndex), eq(header), eq(true)) // - ).thenReturn(decryptedData.get()); + Mockito.any(), eq(chunkIndex), eq(header), eq(true) // + ); - ChunkData data = inTest.load(chunkIndex); + ChunkData chunk = inTest.load(chunkIndex); verify(stats).addChunkCacheMiss(); - verify(stats).addBytesDecrypted(data.asReadOnlyBuffer().remaining()); - assertThat(data.asReadOnlyBuffer(), contains(decryptedData.get())); + verify(stats).addBytesDecrypted(chunk.data().remaining()); + assertThat(chunk.data(), contains(decryptedData.get())); + Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE, chunk.data().remaining()); + Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE, chunk.data().capacity()); } @Test - public void testChunkLoaderReturnsDecrytedDataOfChunkContainingEndOfFile() throws IOException, AuthenticationFailedException { + @DisplayName("load() returns partial chunk near EOF") + public void testLoadReturnsDecrytedDataNearEOF() throws IOException, AuthenticationFailedException { long chunkIndex = 482L; long chunkOffset = chunkIndex * CIPHERTEXT_CHUNK_SIZE + HEADER_SIZE; Supplier decryptedData = () -> repeat(9).times(CLEARTEXT_CHUNK_SIZE - 3).asByteBuffer(); when(chunkIO.read(argThat(hasAtLeastRemaining(CIPHERTEXT_CHUNK_SIZE)), eq(chunkOffset))).then(fillBufferWith((byte) 3, CIPHERTEXT_CHUNK_SIZE - 10)); - when(fileContentCryptor.decryptChunk( // + doAnswer(invocation -> { + ByteBuffer cleartextBuf = invocation.getArgument(1); + cleartextBuf.put(decryptedData.get()); + return null; + }).when(fileContentCryptor).decryptChunk( argThat(contains(repeat(3).times(CIPHERTEXT_CHUNK_SIZE - 10).asByteBuffer())), // - eq(chunkIndex), eq(header), eq(true)) // - ).thenReturn(decryptedData.get()); + any(ByteBuffer.class), eq(chunkIndex), eq(header), eq(true) // + ); - ChunkData data = inTest.load(chunkIndex); + ChunkData chunk = inTest.load(chunkIndex); verify(stats).addChunkCacheMiss(); - verify(stats).addBytesDecrypted(data.asReadOnlyBuffer().remaining()); - assertThat(data.asReadOnlyBuffer(), contains(decryptedData.get())); - data.copyData().from(repeat(9).times(CLEARTEXT_CHUNK_SIZE).asByteBuffer()); - assertThat(data.asReadOnlyBuffer(), contains(repeat(9).times(CLEARTEXT_CHUNK_SIZE).asByteBuffer())); // asserts that data has at least CLEARTEXT_CHUNK_SIZE capacity + verify(stats).addBytesDecrypted(chunk.data().remaining()); + assertThat(chunk.data(), contains(decryptedData.get())); + Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE - 3, chunk.data().remaining()); + Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE, chunk.data().capacity()); } private Answer fillBufferWith(byte value, int amount) { diff --git a/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java b/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java index c2a669ae..589c47f8 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java @@ -1,6 +1,5 @@ package org.cryptomator.cryptofs.fh; -import com.google.common.base.Supplier; import org.cryptomator.cryptofs.CryptoFileSystemStats; import org.cryptomator.cryptolib.api.Cryptor; import org.cryptomator.cryptolib.api.FileContentCryptor; @@ -12,14 +11,15 @@ import java.io.IOException; import java.nio.ByteBuffer; +import java.util.function.Supplier; import static org.cryptomator.cryptofs.matchers.ByteBufferMatcher.contains; import static org.cryptomator.cryptofs.util.ByteBuffers.repeat; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; -import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import static org.mockito.hamcrest.MockitoHamcrest.argThat; @@ -37,7 +37,8 @@ public class ChunkSaverTest { private final FileHeader header = mock(FileHeader.class); private final FileHeaderHolder headerHolder = mock(FileHeaderHolder.class); private final ExceptionsDuringWrite exceptionsDuringWrite = mock(ExceptionsDuringWrite.class); - private final ChunkSaver inTest = new ChunkSaver(cryptor, chunkIO, headerHolder, exceptionsDuringWrite, stats); + private final BufferPool bufferPool = mock(BufferPool.class); + private final ChunkSaver inTest = new ChunkSaver(cryptor, chunkIO, headerHolder, exceptionsDuringWrite, stats, bufferPool); @BeforeEach public void setup() throws IOException { @@ -47,17 +48,22 @@ public void setup() throws IOException { when(fileContentCryptor.ciphertextChunkSize()).thenReturn(CIPHERTEXT_CHUNK_SIZE); when(fileContentCryptor.cleartextChunkSize()).thenReturn(CLEARTEXT_CHUNK_SIZE); when(fileHeaderCryptor.headerSize()).thenReturn(HEADER_SIZE); + when(bufferPool.getCiphertextBuffer()).thenAnswer(invocation -> ByteBuffer.allocate(CIPHERTEXT_CHUNK_SIZE)); + when(bufferPool.getCleartextBuffer()).thenAnswer(invocation -> ByteBuffer.allocate(CLEARTEXT_CHUNK_SIZE)); } @Test public void testChunkInsideFileIsWritten() throws IOException { long chunkIndex = 43L; long expectedPosition = HEADER_SIZE + chunkIndex * CIPHERTEXT_CHUNK_SIZE; - ChunkData chunkData = ChunkData.emptyWithSize(CLEARTEXT_CHUNK_SIZE); Supplier cleartext = () -> repeat(42).times(CLEARTEXT_CHUNK_SIZE).asByteBuffer(); Supplier ciphertext = () -> repeat(50).times(CIPHERTEXT_CHUNK_SIZE).asByteBuffer(); - chunkData.copyData().from(cleartext.get()); - when(fileContentCryptor.encryptChunk(argThat(contains(cleartext.get())), eq(chunkIndex), eq(header))).thenReturn(ciphertext.get()); + ChunkData chunkData = new ChunkData(cleartext.get(), true); + doAnswer(invocation -> { + ByteBuffer ciphertextBuf = invocation.getArgument(1); + ciphertextBuf.put(ciphertext.get()); + return null; + }).when(fileContentCryptor).encryptChunk(argThat(contains(cleartext.get())), Mockito.any(), eq(chunkIndex), eq(header)); inTest.save(chunkIndex, chunkData); @@ -69,11 +75,14 @@ public void testChunkInsideFileIsWritten() throws IOException { public void testChunkContainingEndOfFileIsWrittenTillEndOfFile() throws IOException { long chunkIndex = 43L; long expectedPosition = HEADER_SIZE + chunkIndex * CIPHERTEXT_CHUNK_SIZE; - ChunkData chunkData = ChunkData.emptyWithSize(CLEARTEXT_CHUNK_SIZE); Supplier cleartext = () -> repeat(42).times(CLEARTEXT_CHUNK_SIZE - 10).asByteBuffer(); Supplier ciphertext = () -> repeat(50).times(CIPHERTEXT_CHUNK_SIZE - 10).asByteBuffer(); - chunkData.copyData().from(cleartext.get()); - when(fileContentCryptor.encryptChunk(argThat(contains(cleartext.get())), eq(chunkIndex), eq(header))).thenReturn(ciphertext.get()); + ChunkData chunkData = new ChunkData(cleartext.get(), true); + doAnswer(invocation -> { + ByteBuffer ciphertextBuf = invocation.getArgument(1); + ciphertextBuf.put(ciphertext.get()); + return null; + }).when(fileContentCryptor).encryptChunk(argThat(contains(cleartext.get())), Mockito.any(), eq(chunkIndex), eq(header)); inTest.save(chunkIndex, chunkData); @@ -84,7 +93,7 @@ public void testChunkContainingEndOfFileIsWrittenTillEndOfFile() throws IOExcept @Test public void testChunkThatWasNotWrittenIsNotWritten() throws IOException { Long chunkIndex = 43L; - ChunkData chunkData = ChunkData.emptyWithSize(CLEARTEXT_CHUNK_SIZE); + ChunkData chunkData = new ChunkData(ByteBuffer.allocate(CLEARTEXT_CHUNK_SIZE), false); inTest.save(chunkIndex, chunkData); @@ -97,11 +106,14 @@ public void testIOExceptionsDuringWriteAreAddedToExceptionsDuringWrite() throws IOException ioException = new IOException(); long chunkIndex = 43L; long expectedPosition = HEADER_SIZE + chunkIndex * CIPHERTEXT_CHUNK_SIZE; - ChunkData chunkData = ChunkData.emptyWithSize(CLEARTEXT_CHUNK_SIZE); Supplier cleartext = () -> repeat(42).times(CLEARTEXT_CHUNK_SIZE).asByteBuffer(); Supplier ciphertext = () -> repeat(50).times(CIPHERTEXT_CHUNK_SIZE).asByteBuffer(); - chunkData.copyData().from(cleartext.get()); - when(fileContentCryptor.encryptChunk(argThat(contains(cleartext.get())), eq(chunkIndex), eq(header))).thenReturn(ciphertext.get()); + ChunkData chunkData = new ChunkData(cleartext.get(), true); + doAnswer(invocation -> { + ByteBuffer ciphertextBuf = invocation.getArgument(1); + ciphertextBuf.put(ciphertext.get()); + return null; + }).when(fileContentCryptor).encryptChunk(argThat(contains(cleartext.get())), Mockito.any(), eq(chunkIndex), eq(header)); when(chunkIO.write(argThat(contains(ciphertext.get())), eq(expectedPosition))).thenThrow(ioException); inTest.save(chunkIndex, chunkData); From 35ffccf5039fe96640213925d670dcdb72355b0d Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 26 Jan 2022 16:02:21 +0100 Subject: [PATCH 06/28] renamed class --- .../cryptofs/ch/CleartextFileChannel.java | 14 ++--- .../fh/{ChunkData.java => Chunk.java} | 4 +- .../cryptomator/cryptofs/fh/ChunkCache.java | 10 ++-- .../cryptomator/cryptofs/fh/ChunkLoader.java | 4 +- .../cryptomator/cryptofs/fh/ChunkSaver.java | 2 +- .../cryptofs/ch/CleartextFileChannelTest.java | 4 +- .../cryptofs/fh/ChunkCacheTest.java | 58 +++++++++---------- .../cryptofs/fh/ChunkLoaderTest.java | 6 +- .../cryptofs/fh/ChunkSaverTest.java | 16 ++--- .../fh/{ChunkDataTest.java => ChunkTest.java} | 10 ++-- 10 files changed, 64 insertions(+), 64 deletions(-) rename src/main/java/org/cryptomator/cryptofs/fh/{ChunkData.java => Chunk.java} (89%) rename src/test/java/org/cryptomator/cryptofs/fh/{ChunkDataTest.java => ChunkTest.java} (89%) diff --git a/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java b/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java index ab855aea..fcafd640 100644 --- a/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java +++ b/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java @@ -5,7 +5,7 @@ import org.cryptomator.cryptofs.EffectiveOpenOptions; import org.cryptomator.cryptofs.fh.ByteSource; import org.cryptomator.cryptofs.fh.ChunkCache; -import org.cryptomator.cryptofs.fh.ChunkData; +import org.cryptomator.cryptofs.fh.Chunk; import org.cryptomator.cryptofs.fh.ExceptionsDuringWrite; import org.cryptomator.cryptofs.fh.OpenFileModifiedDate; import org.cryptomator.cryptofs.fh.OpenFileSize; @@ -151,18 +151,18 @@ private long writeLockedInternal(ByteSource src, long position) throws IOExcepti ByteBuffer cleartextChunk = ByteBuffer.allocate(cleartextChunkSize); // TODO: use BufferPool src.copyTo(cleartextChunk); cleartextChunk.flip(); - ChunkData chunkData = new ChunkData(cleartextChunk, true); - chunkCache.set(chunkIndex, chunkData); + Chunk chunk = new Chunk(cleartextChunk, true); + chunkCache.set(chunkIndex, chunk); } else { /* * TODO performance: * We don't actually need to read the current data into the cache. * It would suffice if store the written data and do reading when storing the chunk. */ - ChunkData chunkData = chunkCache.get(chunkIndex); - chunkData.data().limit(Math.max(chunkData.data().limit(), offsetInChunk + len)); // increase limit (if needed) - src.copyTo(chunkData.data().duplicate().position(offsetInChunk)); // work on duplicate using correct offset - chunkData.dirty().set(true); + Chunk chunk = chunkCache.get(chunkIndex); + chunk.data().limit(Math.max(chunk.data().limit(), offsetInChunk + len)); // increase limit (if needed) + src.copyTo(chunk.data().duplicate().position(offsetInChunk)); // work on duplicate using correct offset + chunk.dirty().set(true); } written += len; } diff --git a/src/main/java/org/cryptomator/cryptofs/fh/ChunkData.java b/src/main/java/org/cryptomator/cryptofs/fh/Chunk.java similarity index 89% rename from src/main/java/org/cryptomator/cryptofs/fh/ChunkData.java rename to src/main/java/org/cryptomator/cryptofs/fh/Chunk.java index 9b3281ba..37f0f56d 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/ChunkData.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/Chunk.java @@ -23,9 +23,9 @@ *
  • When no longer used, the cleartext ByteBuffer may be recycled
    • * */ -public record ChunkData(ByteBuffer data, AtomicBoolean dirty) { +public record Chunk(ByteBuffer data, AtomicBoolean dirty) { - public ChunkData(ByteBuffer data, boolean dirty) { + public Chunk(ByteBuffer data, boolean dirty) { this(data, new AtomicBoolean(dirty)); } diff --git a/src/main/java/org/cryptomator/cryptofs/fh/ChunkCache.java b/src/main/java/org/cryptomator/cryptofs/fh/ChunkCache.java index 1fb5d459..215b58a6 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/ChunkCache.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/ChunkCache.java @@ -20,7 +20,7 @@ public class ChunkCache { private final ChunkSaver chunkSaver; private final CryptoFileSystemStats stats; private final BufferPool bufferPool; - private final Cache chunks; + private final Cache chunks; @Inject public ChunkCache(ChunkLoader chunkLoader, ChunkSaver chunkSaver, CryptoFileSystemStats stats, BufferPool bufferPool) { @@ -34,7 +34,7 @@ public ChunkCache(ChunkLoader chunkLoader, ChunkSaver chunkSaver, CryptoFileSyst .build(); } - private ChunkData loadChunk(long chunkIndex) throws IOException { + private Chunk loadChunk(long chunkIndex) throws IOException { stats.addChunkCacheMiss(); try { return chunkLoader.load(chunkIndex); @@ -44,7 +44,7 @@ private ChunkData loadChunk(long chunkIndex) throws IOException { } } - private void removeChunk(RemovalNotification removal) { + private void removeChunk(RemovalNotification removal) { try { chunkSaver.save(removal.getKey(), removal.getValue()); bufferPool.recycle(removal.getValue().data()); @@ -53,7 +53,7 @@ private void removeChunk(RemovalNotification removal) { } } - public ChunkData get(long chunkIndex) throws IOException { + public Chunk get(long chunkIndex) throws IOException { try { stats.addChunkCacheAccess(); return chunks.get(chunkIndex, () -> loadChunk(chunkIndex)); @@ -63,7 +63,7 @@ public ChunkData get(long chunkIndex) throws IOException { } } - public void set(long chunkIndex, ChunkData data) { + public void set(long chunkIndex, Chunk data) { chunks.put(chunkIndex, data); } diff --git a/src/main/java/org/cryptomator/cryptofs/fh/ChunkLoader.java b/src/main/java/org/cryptomator/cryptofs/fh/ChunkLoader.java index b38f247a..3524e95a 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/ChunkLoader.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/ChunkLoader.java @@ -26,7 +26,7 @@ public ChunkLoader(Cryptor cryptor, ChunkIO ciphertext, FileHeaderHolder headerH this.bufferPool = bufferPool; } - public ChunkData load(Long chunkIndex) throws IOException, AuthenticationFailedException { + public Chunk load(Long chunkIndex) throws IOException, AuthenticationFailedException { stats.addChunkCacheMiss(); int chunkSize = cryptor.fileContentCryptor().ciphertextChunkSize(); long ciphertextPos = chunkIndex * chunkSize + cryptor.fileHeaderCryptor().headerSize(); @@ -42,7 +42,7 @@ public ChunkData load(Long chunkIndex) throws IOException, AuthenticationFailedE cleartextBuf.flip(); stats.addBytesDecrypted(cleartextBuf.remaining()); } - return new ChunkData(cleartextBuf, false); + return new Chunk(cleartextBuf, false); } finally { bufferPool.recycle(ciphertextBuf); } diff --git a/src/main/java/org/cryptomator/cryptofs/fh/ChunkSaver.java b/src/main/java/org/cryptomator/cryptofs/fh/ChunkSaver.java index 3fed4f2d..dbb798b3 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/ChunkSaver.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/ChunkSaver.java @@ -27,7 +27,7 @@ public ChunkSaver(Cryptor cryptor, ChunkIO ciphertext, FileHeaderHolder headerHo this.bufferPool = bufferPool; } - public void save(long chunkIndex, ChunkData chunkData) throws IOException { + public void save(long chunkIndex, Chunk chunkData) throws IOException { if (chunkData.isDirty()) { long ciphertextPos = chunkIndex * cryptor.fileContentCryptor().ciphertextChunkSize() + cryptor.fileHeaderCryptor().headerSize(); ByteBuffer cleartextBuf = chunkData.data().duplicate(); diff --git a/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java b/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java index d2692a79..395c4d0e 100644 --- a/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java +++ b/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java @@ -3,7 +3,7 @@ import org.cryptomator.cryptofs.CryptoFileSystemStats; import org.cryptomator.cryptofs.EffectiveOpenOptions; import org.cryptomator.cryptofs.fh.ChunkCache; -import org.cryptomator.cryptofs.fh.ChunkData; +import org.cryptomator.cryptofs.fh.Chunk; import org.cryptomator.cryptofs.fh.ExceptionsDuringWrite; import org.cryptomator.cryptolib.api.Cryptor; import org.cryptomator.cryptolib.api.FileContentCryptor; @@ -74,7 +74,7 @@ public class CleartextFileChannelTest { public void setUp() throws IOException { when(cryptor.fileHeaderCryptor()).thenReturn(fileHeaderCryptor); when(cryptor.fileContentCryptor()).thenReturn(fileContentCryptor); - when(chunkCache.get(Mockito.anyLong())).then(invocation -> new ChunkData(ByteBuffer.allocate(100), false)); + when(chunkCache.get(Mockito.anyLong())).then(invocation -> new Chunk(ByteBuffer.allocate(100), false)); when(fileHeaderCryptor.headerSize()).thenReturn(50); when(fileContentCryptor.cleartextChunkSize()).thenReturn(100); when(fileContentCryptor.ciphertextChunkSize()).thenReturn(110); diff --git a/src/test/java/org/cryptomator/cryptofs/fh/ChunkCacheTest.java b/src/test/java/org/cryptomator/cryptofs/fh/ChunkCacheTest.java index b6d22ee8..f62dd41b 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/ChunkCacheTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/ChunkCacheTest.java @@ -25,21 +25,21 @@ public class ChunkCacheTest { @Test public void testGetInvokesLoaderIfEntryNotInCache() throws IOException, AuthenticationFailedException { long index = 42L; - ChunkData data = mock(ChunkData.class); - when(chunkLoader.load(index)).thenReturn(data); + Chunk chunk = mock(Chunk.class); + when(chunkLoader.load(index)).thenReturn(chunk); - Assertions.assertSame(data, inTest.get(index)); + Assertions.assertSame(chunk, inTest.get(index)); verify(stats).addChunkCacheAccess(); } @Test public void testGetDoesNotInvokeLoaderIfEntryInCacheFromPreviousGet() throws IOException, AuthenticationFailedException { long index = 42L; - ChunkData data = mock(ChunkData.class); - when(chunkLoader.load(index)).thenReturn(data); + Chunk chunk = mock(Chunk.class); + when(chunkLoader.load(index)).thenReturn(chunk); inTest.get(index); - Assertions.assertSame(data, inTest.get(index)); + Assertions.assertSame(chunk, inTest.get(index)); verify(stats, Mockito.times(2)).addChunkCacheAccess(); verify(chunkLoader).load(index); } @@ -47,10 +47,10 @@ public void testGetDoesNotInvokeLoaderIfEntryInCacheFromPreviousGet() throws IOE @Test public void testGetDoesNotInvokeLoaderIfEntryInCacheFromPreviousSet() throws IOException { long index = 42L; - ChunkData data = mock(ChunkData.class); - inTest.set(index, data); + Chunk chunk = mock(Chunk.class); + inTest.set(index, chunk); - Assertions.assertSame(data, inTest.get(index)); + Assertions.assertSame(chunk, inTest.get(index)); verify(stats).addChunkCacheAccess(); } @@ -58,18 +58,18 @@ public void testGetDoesNotInvokeLoaderIfEntryInCacheFromPreviousSet() throws IOE public void testGetInvokesSaverIfMaxEntriesInCacheAreReachedAndAnEntryNotInCacheIsRequested() throws IOException, AuthenticationFailedException { long firstIndex = 42L; long indexNotInCache = 40L; - ChunkData firstData = mock(ChunkData.class); - inTest.set(firstIndex, firstData); + Chunk chunk = mock(Chunk.class); + inTest.set(firstIndex, chunk); for (int i = 1; i < ChunkCache.MAX_CACHED_CLEARTEXT_CHUNKS; i++) { - inTest.set(firstIndex + i, mock(ChunkData.class)); + inTest.set(firstIndex + i, mock(Chunk.class)); } - when(chunkLoader.load(indexNotInCache)).thenReturn(mock(ChunkData.class)); + when(chunkLoader.load(indexNotInCache)).thenReturn(mock(Chunk.class)); inTest.get(indexNotInCache); verify(stats).addChunkCacheAccess(); - verify(chunkSaver).save(firstIndex, firstData); - verify(bufferPool).recycle(firstData.data()); + verify(chunkSaver).save(firstIndex, chunk); + verify(bufferPool).recycle(chunk.data()); verifyNoMoreInteractions(chunkSaver); } @@ -77,16 +77,16 @@ public void testGetInvokesSaverIfMaxEntriesInCacheAreReachedAndAnEntryNotInCache public void testGetInvokesSaverIfMaxEntriesInCacheAreReachedAndAnEntryNotInCacheIsSet() throws IOException { long firstIndex = 42L; long indexNotInCache = 40L; - ChunkData firstData = mock(ChunkData.class); - inTest.set(firstIndex, firstData); + Chunk chunk = mock(Chunk.class); + inTest.set(firstIndex, chunk); for (int i = 1; i < ChunkCache.MAX_CACHED_CLEARTEXT_CHUNKS; i++) { - inTest.set(firstIndex + i, mock(ChunkData.class)); + inTest.set(firstIndex + i, mock(Chunk.class)); } - inTest.set(indexNotInCache, mock(ChunkData.class)); + inTest.set(indexNotInCache, mock(Chunk.class)); - verify(chunkSaver).save(firstIndex, firstData); - verify(bufferPool).recycle(firstData.data()); + verify(chunkSaver).save(firstIndex, chunk); + verify(bufferPool).recycle(chunk.data()); verifyNoMoreInteractions(chunkSaver); } @@ -94,16 +94,16 @@ public void testGetInvokesSaverIfMaxEntriesInCacheAreReachedAndAnEntryNotInCache public void testGetInvokesSaverIfMaxEntriesInCacheAreReachedAndAnEntryInCacheIsSet() throws IOException { // TODO markuskreusch: this behaviour isn't actually needed, maybe we can somehow prevent saving in such situations? long firstIndex = 42L; - ChunkData firstData = mock(ChunkData.class); - inTest.set(firstIndex, firstData); + Chunk chunk = mock(Chunk.class); + inTest.set(firstIndex, chunk); for (int i = 1; i < ChunkCache.MAX_CACHED_CLEARTEXT_CHUNKS; i++) { - inTest.set(firstIndex + i, mock(ChunkData.class)); + inTest.set(firstIndex + i, mock(Chunk.class)); } - inTest.set(firstIndex, mock(ChunkData.class)); + inTest.set(firstIndex, mock(Chunk.class)); - verify(chunkSaver).save(firstIndex, firstData); - verify(bufferPool).recycle(firstData.data()); + verify(chunkSaver).save(firstIndex, chunk); + verify(bufferPool).recycle(chunk.data()); verifyNoMoreInteractions(chunkSaver); } @@ -135,8 +135,8 @@ public void testGetThrowsUncheckedExceptionFromLoader() throws IOException, Auth public void testInvalidateAllInvokesSaverForAllEntriesInCache() throws IOException, AuthenticationFailedException { long index = 42L; long index2 = 43L; - ChunkData chunk1 = mock(ChunkData.class); - ChunkData chunk2 = mock(ChunkData.class); + Chunk chunk1 = mock(Chunk.class); + Chunk chunk2 = mock(Chunk.class); when(chunk1.data()).thenReturn(ByteBuffer.allocate(42)); when(chunk2.data()).thenReturn(ByteBuffer.allocate(23)); when(chunkLoader.load(index)).thenReturn(chunk1); diff --git a/src/test/java/org/cryptomator/cryptofs/fh/ChunkLoaderTest.java b/src/test/java/org/cryptomator/cryptofs/fh/ChunkLoaderTest.java index 0ce834c9..2bd8687a 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/ChunkLoaderTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/ChunkLoaderTest.java @@ -64,7 +64,7 @@ public void testLoadReturnsEmptyChunkAfterEOF() throws IOException, Authenticati long chunkOffset = chunkIndex * CIPHERTEXT_CHUNK_SIZE + HEADER_SIZE; when(chunkIO.read(argThat(hasAtLeastRemaining(CIPHERTEXT_CHUNK_SIZE)), eq(chunkOffset))).thenReturn(-1); - ChunkData chunk = inTest.load(chunkIndex); + Chunk chunk = inTest.load(chunkIndex); verify(stats).addChunkCacheMiss(); Assertions.assertEquals(0, chunk.data().remaining()); @@ -87,7 +87,7 @@ public void testLoadReturnsDecryptedDataInsideFile() throws IOException, Authent Mockito.any(), eq(chunkIndex), eq(header), eq(true) // ); - ChunkData chunk = inTest.load(chunkIndex); + Chunk chunk = inTest.load(chunkIndex); verify(stats).addChunkCacheMiss(); verify(stats).addBytesDecrypted(chunk.data().remaining()); @@ -112,7 +112,7 @@ public void testLoadReturnsDecrytedDataNearEOF() throws IOException, Authenticat any(ByteBuffer.class), eq(chunkIndex), eq(header), eq(true) // ); - ChunkData chunk = inTest.load(chunkIndex); + Chunk chunk = inTest.load(chunkIndex); verify(stats).addChunkCacheMiss(); verify(stats).addBytesDecrypted(chunk.data().remaining()); diff --git a/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java b/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java index 589c47f8..eb461ea1 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java @@ -58,14 +58,14 @@ public void testChunkInsideFileIsWritten() throws IOException { long expectedPosition = HEADER_SIZE + chunkIndex * CIPHERTEXT_CHUNK_SIZE; Supplier cleartext = () -> repeat(42).times(CLEARTEXT_CHUNK_SIZE).asByteBuffer(); Supplier ciphertext = () -> repeat(50).times(CIPHERTEXT_CHUNK_SIZE).asByteBuffer(); - ChunkData chunkData = new ChunkData(cleartext.get(), true); + Chunk chunk = new Chunk(cleartext.get(), true); doAnswer(invocation -> { ByteBuffer ciphertextBuf = invocation.getArgument(1); ciphertextBuf.put(ciphertext.get()); return null; }).when(fileContentCryptor).encryptChunk(argThat(contains(cleartext.get())), Mockito.any(), eq(chunkIndex), eq(header)); - inTest.save(chunkIndex, chunkData); + inTest.save(chunkIndex, chunk); verify(chunkIO).write(argThat(contains(ciphertext.get())), eq(expectedPosition)); verify(stats).addBytesEncrypted(Mockito.anyLong()); @@ -77,14 +77,14 @@ public void testChunkContainingEndOfFileIsWrittenTillEndOfFile() throws IOExcept long expectedPosition = HEADER_SIZE + chunkIndex * CIPHERTEXT_CHUNK_SIZE; Supplier cleartext = () -> repeat(42).times(CLEARTEXT_CHUNK_SIZE - 10).asByteBuffer(); Supplier ciphertext = () -> repeat(50).times(CIPHERTEXT_CHUNK_SIZE - 10).asByteBuffer(); - ChunkData chunkData = new ChunkData(cleartext.get(), true); + Chunk chunk = new Chunk(cleartext.get(), true); doAnswer(invocation -> { ByteBuffer ciphertextBuf = invocation.getArgument(1); ciphertextBuf.put(ciphertext.get()); return null; }).when(fileContentCryptor).encryptChunk(argThat(contains(cleartext.get())), Mockito.any(), eq(chunkIndex), eq(header)); - inTest.save(chunkIndex, chunkData); + inTest.save(chunkIndex, chunk); verify(chunkIO).write(argThat(contains(ciphertext.get())), eq(expectedPosition)); verify(stats).addBytesEncrypted(Mockito.anyLong()); @@ -93,9 +93,9 @@ public void testChunkContainingEndOfFileIsWrittenTillEndOfFile() throws IOExcept @Test public void testChunkThatWasNotWrittenIsNotWritten() throws IOException { Long chunkIndex = 43L; - ChunkData chunkData = new ChunkData(ByteBuffer.allocate(CLEARTEXT_CHUNK_SIZE), false); + Chunk chunk = new Chunk(ByteBuffer.allocate(CLEARTEXT_CHUNK_SIZE), false); - inTest.save(chunkIndex, chunkData); + inTest.save(chunkIndex, chunk); verifyNoInteractions(chunkIO); verifyNoInteractions(stats); @@ -108,7 +108,7 @@ public void testIOExceptionsDuringWriteAreAddedToExceptionsDuringWrite() throws long expectedPosition = HEADER_SIZE + chunkIndex * CIPHERTEXT_CHUNK_SIZE; Supplier cleartext = () -> repeat(42).times(CLEARTEXT_CHUNK_SIZE).asByteBuffer(); Supplier ciphertext = () -> repeat(50).times(CIPHERTEXT_CHUNK_SIZE).asByteBuffer(); - ChunkData chunkData = new ChunkData(cleartext.get(), true); + Chunk chunk = new Chunk(cleartext.get(), true); doAnswer(invocation -> { ByteBuffer ciphertextBuf = invocation.getArgument(1); ciphertextBuf.put(ciphertext.get()); @@ -116,7 +116,7 @@ public void testIOExceptionsDuringWriteAreAddedToExceptionsDuringWrite() throws }).when(fileContentCryptor).encryptChunk(argThat(contains(cleartext.get())), Mockito.any(), eq(chunkIndex), eq(header)); when(chunkIO.write(argThat(contains(ciphertext.get())), eq(expectedPosition))).thenThrow(ioException); - inTest.save(chunkIndex, chunkData); + inTest.save(chunkIndex, chunk); verify(exceptionsDuringWrite).add(ioException); } diff --git a/src/test/java/org/cryptomator/cryptofs/fh/ChunkDataTest.java b/src/test/java/org/cryptomator/cryptofs/fh/ChunkTest.java similarity index 89% rename from src/test/java/org/cryptomator/cryptofs/fh/ChunkDataTest.java rename to src/test/java/org/cryptomator/cryptofs/fh/ChunkTest.java index 9cf69f3c..aa5c32b2 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/ChunkDataTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/ChunkTest.java @@ -13,13 +13,13 @@ import static org.cryptomator.cryptofs.util.ByteBuffers.repeat; -public class ChunkDataTest { +public class ChunkTest { @Test public void testChunkDataWrappingBufferIsNotDirty() { ByteBuffer buffer = repeat(3).times(200).asByteBuffer(); - ChunkData inTest = new ChunkData(buffer, false); + Chunk inTest = new Chunk(buffer, false); Assertions.assertFalse(inTest.isDirty()); } @@ -28,7 +28,7 @@ public void testChunkDataWrappingBufferIsNotDirty() { public void testEmptyChunkDataIsNotDirty() { ByteBuffer buffer = ByteBuffer.allocate(0); - ChunkData inTest = new ChunkData(buffer, false); + Chunk inTest = new Chunk(buffer, false); Assertions.assertFalse(inTest.isDirty()); } @@ -37,7 +37,7 @@ public void testEmptyChunkDataIsNotDirty() { public void testToString() { ByteBuffer buffer = ByteBuffer.allocate(100); - ChunkData inTest = new ChunkData(buffer, true); + Chunk inTest = new Chunk(buffer, true); Assertions.assertEquals("ChunkData(dirty: true, length: 100)", inTest.toString()); } @@ -45,7 +45,7 @@ public void testToString() { @Test // https://github.com/cryptomator/cryptofs/issues/85 public void testRaceConditionsDuringRead() { ByteBuffer src = StandardCharsets.US_ASCII.encode("abcdefg"); - ChunkData inTest = new ChunkData(src, false); + Chunk inTest = new Chunk(src, false); int attempts = 4000; int threads = 6; From 34d87ce92bbe14020bb081bdf0c5ca5553e2a337 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 26 Jan 2022 16:44:08 +0100 Subject: [PATCH 07/28] added unit tests --- .../org/cryptomator/cryptofs/fh/Chunk.java | 5 - .../cryptofs/fh/BufferPoolTest.java | 107 ++++++++++++++++++ .../cryptofs/fh/ChunkLoaderTest.java | 4 + .../cryptofs/fh/ChunkSaverTest.java | 4 + .../cryptomator/cryptofs/fh/ChunkTest.java | 9 -- .../cryptofs/matchers/ByteBufferMatcher.java | 4 + 6 files changed, 119 insertions(+), 14 deletions(-) create mode 100644 src/test/java/org/cryptomator/cryptofs/fh/BufferPoolTest.java diff --git a/src/main/java/org/cryptomator/cryptofs/fh/Chunk.java b/src/main/java/org/cryptomator/cryptofs/fh/Chunk.java index 37f0f56d..f53540b6 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/Chunk.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/Chunk.java @@ -33,9 +33,4 @@ public boolean isDirty() { return dirty.get(); } - @Override - public String toString() { - return format("ChunkData(dirty: %s, length: %d)", dirty, data.limit()); - } - } diff --git a/src/test/java/org/cryptomator/cryptofs/fh/BufferPoolTest.java b/src/test/java/org/cryptomator/cryptofs/fh/BufferPoolTest.java new file mode 100644 index 00000000..141f564f --- /dev/null +++ b/src/test/java/org/cryptomator/cryptofs/fh/BufferPoolTest.java @@ -0,0 +1,107 @@ +package org.cryptomator.cryptofs.fh; + +import org.cryptomator.cryptolib.api.Cryptor; +import org.cryptomator.cryptolib.api.FileContentCryptor; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.Mockito; + +import java.nio.ByteBuffer; + +public class BufferPoolTest { + + private static final int CLEARTEXT_CHUNK_SIZE = 100; + private static final int CIPHERTEXT_CHUNK_SIZE = 150; + + private final Cryptor cryptor = Mockito.mock(Cryptor.class); + private final FileContentCryptor fileContentCryptor = Mockito.mock(FileContentCryptor.class); + private BufferPool bufferPool; + + @BeforeEach + public void setup() { + Mockito.doReturn(fileContentCryptor).when(cryptor).fileContentCryptor(); + Mockito.doReturn(CLEARTEXT_CHUNK_SIZE).when(fileContentCryptor).cleartextChunkSize(); + Mockito.doReturn(CIPHERTEXT_CHUNK_SIZE).when(fileContentCryptor).ciphertextChunkSize(); + bufferPool = new BufferPool(cryptor); + } + + @Test + @DisplayName("getCiphertextBuffer() with no cached item") + public void testGetUncachedCiphertextBuffer() { + try (var byteBufferClass = Mockito.mockStatic(ByteBuffer.class)) { + byteBufferClass.when(() -> ByteBuffer.allocate(Mockito.anyInt())).thenCallRealMethod(); + var buf = bufferPool.getCiphertextBuffer(); + + Assertions.assertEquals(CIPHERTEXT_CHUNK_SIZE, buf.capacity()); + byteBufferClass.verify(() -> ByteBuffer.allocate(CIPHERTEXT_CHUNK_SIZE)); + } + } + + @Test + @DisplayName("getCiphertextBuffer() after recycling ciphertext") + public void testGetCachedCiphertextBuffer() { + var buf0 = ByteBuffer.allocate(CIPHERTEXT_CHUNK_SIZE); + bufferPool.recycle(buf0); + buf0 = null; + System.gc(); // seems to be reliable on Temurin 17 with @RepeatedTest(1000) + + var buf1 = ByteBuffer.allocate(CIPHERTEXT_CHUNK_SIZE); + bufferPool.recycle(buf1); + + try (var byteBufferClass = Mockito.mockStatic(ByteBuffer.class)) { + var buf2 = bufferPool.getCiphertextBuffer(); + + Assertions.assertSame(buf1, buf2); + Assertions.assertEquals(0, buf2.position(), "expected recycled buffer to be cleared"); + Assertions.assertEquals(buf2.capacity(), buf2.limit(), "expected recycled buffer to be cleared"); + byteBufferClass.verifyNoInteractions(); + } + } + + @Test + @DisplayName("getCleartextBuffer() with no cached item") + public void testGetUncachedCleartextBuffer() { + try (var byteBufferClass = Mockito.mockStatic(ByteBuffer.class)) { + byteBufferClass.when(() -> ByteBuffer.allocate(Mockito.anyInt())).thenCallRealMethod(); + var buf = bufferPool.getCleartextBuffer(); + + Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE, buf.capacity()); + byteBufferClass.verify(() -> ByteBuffer.allocate(CLEARTEXT_CHUNK_SIZE)); + } + } + + @Test + @DisplayName("getCleartextBuffer() after recycling cleartext") + public void testGetCachedCleartextBuffer() { + var buf0 = ByteBuffer.allocate(CIPHERTEXT_CHUNK_SIZE); + bufferPool.recycle(buf0); + buf0 = null; + System.gc(); // seems to be reliable on Temurin 17 with @RepeatedTest(1000) + + var buf1 = ByteBuffer.allocate(CLEARTEXT_CHUNK_SIZE); + bufferPool.recycle(buf1); + + try (var byteBufferClass = Mockito.mockStatic(ByteBuffer.class)) { + var buf2 = bufferPool.getCleartextBuffer(); + + Assertions.assertSame(buf1, buf2); + Assertions.assertEquals(0, buf2.position(), "expected recycled buffer to be cleared"); + Assertions.assertEquals(buf2.capacity(), buf2.limit(), "expected recycled buffer to be cleared"); + byteBufferClass.verifyNoInteractions(); + } + } + + @DisplayName("recycle() accepts any size") + @ParameterizedTest + @ValueSource(ints = {CIPHERTEXT_CHUNK_SIZE, CLEARTEXT_CHUNK_SIZE, 42}) + public void testRecycle(int capacity) { + var buf = ByteBuffer.allocate(capacity); + + Assertions.assertDoesNotThrow(() -> bufferPool.recycle(buf)); + } + +} \ No newline at end of file diff --git a/src/test/java/org/cryptomator/cryptofs/fh/ChunkLoaderTest.java b/src/test/java/org/cryptomator/cryptofs/fh/ChunkLoaderTest.java index 2bd8687a..8d05b987 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/ChunkLoaderTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/ChunkLoaderTest.java @@ -1,6 +1,7 @@ package org.cryptomator.cryptofs.fh; import org.cryptomator.cryptofs.CryptoFileSystemStats; +import org.cryptomator.cryptofs.matchers.ByteBufferMatcher; import org.cryptomator.cryptolib.api.AuthenticationFailedException; import org.cryptomator.cryptolib.api.Cryptor; import org.cryptomator.cryptolib.api.FileContentCryptor; @@ -67,6 +68,7 @@ public void testLoadReturnsEmptyChunkAfterEOF() throws IOException, Authenticati Chunk chunk = inTest.load(chunkIndex); verify(stats).addChunkCacheMiss(); + verify(bufferPool).recycle(argThat(ByteBufferMatcher.hasCapacity(CIPHERTEXT_CHUNK_SIZE))); Assertions.assertEquals(0, chunk.data().remaining()); Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE, chunk.data().capacity()); } @@ -91,6 +93,7 @@ public void testLoadReturnsDecryptedDataInsideFile() throws IOException, Authent verify(stats).addChunkCacheMiss(); verify(stats).addBytesDecrypted(chunk.data().remaining()); + verify(bufferPool).recycle(argThat(ByteBufferMatcher.hasCapacity(CIPHERTEXT_CHUNK_SIZE))); assertThat(chunk.data(), contains(decryptedData.get())); Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE, chunk.data().remaining()); Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE, chunk.data().capacity()); @@ -116,6 +119,7 @@ public void testLoadReturnsDecrytedDataNearEOF() throws IOException, Authenticat verify(stats).addChunkCacheMiss(); verify(stats).addBytesDecrypted(chunk.data().remaining()); + verify(bufferPool).recycle(argThat(ByteBufferMatcher.hasCapacity(CIPHERTEXT_CHUNK_SIZE))); assertThat(chunk.data(), contains(decryptedData.get())); Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE - 3, chunk.data().remaining()); Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE, chunk.data().capacity()); diff --git a/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java b/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java index eb461ea1..57f25cd5 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java @@ -69,6 +69,7 @@ public void testChunkInsideFileIsWritten() throws IOException { verify(chunkIO).write(argThat(contains(ciphertext.get())), eq(expectedPosition)); verify(stats).addBytesEncrypted(Mockito.anyLong()); + verify(bufferPool).recycle(chunk.data()); } @Test @@ -88,6 +89,7 @@ public void testChunkContainingEndOfFileIsWrittenTillEndOfFile() throws IOExcept verify(chunkIO).write(argThat(contains(ciphertext.get())), eq(expectedPosition)); verify(stats).addBytesEncrypted(Mockito.anyLong()); + verify(bufferPool).recycle(chunk.data()); } @Test @@ -99,6 +101,7 @@ public void testChunkThatWasNotWrittenIsNotWritten() throws IOException { verifyNoInteractions(chunkIO); verifyNoInteractions(stats); + verifyNoInteractions(bufferPool); } @Test @@ -119,6 +122,7 @@ public void testIOExceptionsDuringWriteAreAddedToExceptionsDuringWrite() throws inTest.save(chunkIndex, chunk); verify(exceptionsDuringWrite).add(ioException); + verify(bufferPool).recycle(chunk.data()); } } diff --git a/src/test/java/org/cryptomator/cryptofs/fh/ChunkTest.java b/src/test/java/org/cryptomator/cryptofs/fh/ChunkTest.java index aa5c32b2..01716e12 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/ChunkTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/ChunkTest.java @@ -33,15 +33,6 @@ public void testEmptyChunkDataIsNotDirty() { Assertions.assertFalse(inTest.isDirty()); } - @Test - public void testToString() { - ByteBuffer buffer = ByteBuffer.allocate(100); - - Chunk inTest = new Chunk(buffer, true); - - Assertions.assertEquals("ChunkData(dirty: true, length: 100)", inTest.toString()); - } - @Test // https://github.com/cryptomator/cryptofs/issues/85 public void testRaceConditionsDuringRead() { ByteBuffer src = StandardCharsets.US_ASCII.encode("abcdefg"); diff --git a/src/test/java/org/cryptomator/cryptofs/matchers/ByteBufferMatcher.java b/src/test/java/org/cryptomator/cryptofs/matchers/ByteBufferMatcher.java index 49bf6a54..d15744a3 100644 --- a/src/test/java/org/cryptomator/cryptofs/matchers/ByteBufferMatcher.java +++ b/src/test/java/org/cryptomator/cryptofs/matchers/ByteBufferMatcher.java @@ -19,6 +19,10 @@ public static Matcher hasRemaining(int remaining) { return matcher("bytes remaining", is(remaining), ByteBuffer::remaining); } + public static Matcher hasCapacity(int capacity) { + return matcher("capacity", is(capacity), ByteBuffer::capacity); + } + public static Matcher contains(ByteBuffer data) { byte[] arrayData = new byte[data.remaining()]; data.get(arrayData); From 293d0b47a97cade19011c716e5e0013e7564eb53 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 26 Jan 2022 17:13:58 +0100 Subject: [PATCH 08/28] re-enable jacoco --- pom.xml | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/pom.xml b/pom.xml index 383b8ab1..123a816c 100644 --- a/pom.xml +++ b/pom.xml @@ -33,9 +33,6 @@ 6.2.2 0.8.7 1.6.8 - - - @@ -145,11 +142,9 @@ maven-surefire-plugin 3.0.0-M5 - - ${argLine} --add-opens=org.cryptomator.cryptofs/org.cryptomator.cryptofs.health.dirid=ALL-UNNAMED - --add-opens=org.cryptomator.cryptofs/org.cryptomator.cryptofs.health.type=ALL-UNNAMED - --add-opens=org.cryptomator.cryptofs/org.cryptomator.cryptofs.health.shortened=ALL-UNNAMED - + + false + org.apache.maven.plugins From 06a6b44a91b77a6e9e20da8f3e20541ca06a7937 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 26 Jan 2022 17:14:10 +0100 Subject: [PATCH 09/28] dependency update --- pom.xml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pom.xml b/pom.xml index 123a816c..54e5fab8 100644 --- a/pom.xml +++ b/pom.xml @@ -20,12 +20,12 @@ 2.0.3 3.18.3 - 2.37 - 30.1.1-jre - 1.7.31 + 2.40.5 + 31.0.1-jre + 1.7.33 - 5.7.2 + 5.8.2 3.11.2 2.2 @@ -115,7 +115,7 @@ com.google.jimfs jimfs - 1.1 + 1.2 test From 27368b3f6b5a8c66042028d241e7fc931742245a Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 26 Jan 2022 17:17:58 +0100 Subject: [PATCH 10/28] Update to new major version of Mockito --- pom.xml | 2 +- src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 54e5fab8..92f9eb0d 100644 --- a/pom.xml +++ b/pom.xml @@ -26,7 +26,7 @@ 5.8.2 - 3.11.2 + 4.3.1 2.2 diff --git a/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java b/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java index c2a669ae..1c42a02f 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java @@ -19,7 +19,6 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; -import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import static org.mockito.hamcrest.MockitoHamcrest.argThat; From 9fce43013cac264b229ff9f5146b7ef2e46509c1 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 26 Jan 2022 18:36:55 +0100 Subject: [PATCH 11/28] resolve TODO --- .../cryptofs/dir/C9rConflictResolver.java | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptofs/dir/C9rConflictResolver.java b/src/main/java/org/cryptomator/cryptofs/dir/C9rConflictResolver.java index 5e3fa055..5964ce34 100644 --- a/src/main/java/org/cryptomator/cryptofs/dir/C9rConflictResolver.java +++ b/src/main/java/org/cryptomator/cryptofs/dir/C9rConflictResolver.java @@ -154,16 +154,8 @@ private boolean hasSameFileContent(Path conflictingPath, Path canonicalPath, int if (!Files.isDirectory(conflictingPath.getParent()) || !Files.isDirectory(canonicalPath.getParent())) { return false; } - // TODO replace by Files.mismatch() when using JDK > 12 - try (ReadableByteChannel in1 = Files.newByteChannel(conflictingPath, StandardOpenOption.READ); // - ReadableByteChannel in2 = Files.newByteChannel(canonicalPath, StandardOpenOption.READ)) { - ByteBuffer buf1 = ByteBuffer.allocate(numBytesToCompare); - ByteBuffer buf2 = ByteBuffer.allocate(numBytesToCompare); - int read1 = in1.read(buf1); - int read2 = in2.read(buf2); - buf1.flip(); - buf2.flip(); - return read1 == read2 && buf1.compareTo(buf2) == 0; + try { + return -1L == Files.mismatch(conflictingPath, canonicalPath); } catch (NoSuchFileException e) { return false; } From 5bc62827bd977341b4647568c1cc145ac6e7a095 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 26 Jan 2022 19:56:28 +0100 Subject: [PATCH 12/28] fixed tests --- .../java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java b/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java index 57f25cd5..e3f9e885 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java @@ -1,6 +1,7 @@ package org.cryptomator.cryptofs.fh; import org.cryptomator.cryptofs.CryptoFileSystemStats; +import org.cryptomator.cryptofs.matchers.ByteBufferMatcher; import org.cryptomator.cryptolib.api.Cryptor; import org.cryptomator.cryptolib.api.FileContentCryptor; import org.cryptomator.cryptolib.api.FileHeader; @@ -69,7 +70,7 @@ public void testChunkInsideFileIsWritten() throws IOException { verify(chunkIO).write(argThat(contains(ciphertext.get())), eq(expectedPosition)); verify(stats).addBytesEncrypted(Mockito.anyLong()); - verify(bufferPool).recycle(chunk.data()); + verify(bufferPool).recycle(argThat(ByteBufferMatcher.hasCapacity(CIPHERTEXT_CHUNK_SIZE))); } @Test @@ -89,7 +90,7 @@ public void testChunkContainingEndOfFileIsWrittenTillEndOfFile() throws IOExcept verify(chunkIO).write(argThat(contains(ciphertext.get())), eq(expectedPosition)); verify(stats).addBytesEncrypted(Mockito.anyLong()); - verify(bufferPool).recycle(chunk.data()); + verify(bufferPool).recycle(argThat(ByteBufferMatcher.hasCapacity(CIPHERTEXT_CHUNK_SIZE))); } @Test @@ -122,7 +123,7 @@ public void testIOExceptionsDuringWriteAreAddedToExceptionsDuringWrite() throws inTest.save(chunkIndex, chunk); verify(exceptionsDuringWrite).add(ioException); - verify(bufferPool).recycle(chunk.data()); + verify(bufferPool).recycle(argThat(ByteBufferMatcher.hasCapacity(CIPHERTEXT_CHUNK_SIZE))); } } From 16f2d6731070ad3674aaa1d6e2b1e0341b605943 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Thu, 27 Jan 2022 17:54:54 +0100 Subject: [PATCH 13/28] fixes #124 --- .../org/cryptomator/cryptofs/CiphertextFilePath.java | 7 +++++-- .../org/cryptomator/cryptofs/LongFileNameProvider.java | 9 +-------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptofs/CiphertextFilePath.java b/src/main/java/org/cryptomator/cryptofs/CiphertextFilePath.java index e655861a..81f955d6 100644 --- a/src/main/java/org/cryptomator/cryptofs/CiphertextFilePath.java +++ b/src/main/java/org/cryptomator/cryptofs/CiphertextFilePath.java @@ -2,6 +2,7 @@ import org.cryptomator.cryptofs.common.Constants; +import java.io.IOException; import java.nio.file.Path; import java.util.Objects; import java.util.Optional; @@ -59,7 +60,9 @@ public String toString() { return path.toString(); } - public void persistLongFileName() { - deflatedFileName.ifPresent(LongFileNameProvider.DeflatedFileName::persist); + public void persistLongFileName() throws IOException { + if( deflatedFileName.isPresent()) { + deflatedFileName.get().persist(); + } } } diff --git a/src/main/java/org/cryptomator/cryptofs/LongFileNameProvider.java b/src/main/java/org/cryptomator/cryptofs/LongFileNameProvider.java index 419c5e58..17b183b6 100644 --- a/src/main/java/org/cryptomator/cryptofs/LongFileNameProvider.java +++ b/src/main/java/org/cryptomator/cryptofs/LongFileNameProvider.java @@ -101,16 +101,9 @@ public static class DeflatedFileName { this.readonlyFlag = readonlyFlag; } - public void persist() { + public void persist() throws IOException { readonlyFlag.assertWritable(); - try { - persistInternal(); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } - private void persistInternal() throws IOException { Path longNameFile = c9sPath.resolve(INFLATED_FILE_NAME); Files.createDirectories(c9sPath); Files.writeString(longNameFile, longName, UTF_8, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.WRITE, StandardOpenOption.CREATE); From a8126b2ef43a92c92b1d0a14306d4d6d3524ca69 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Sat, 5 Feb 2022 00:39:41 +0100 Subject: [PATCH 14/28] fixing memory leak :tada: Kudos to sonarcloud --- .../cryptofs/CryptoFileSystemImpl.java | 34 ++++++++++++------- .../cryptofs/CryptoFileSystemImplTest.java | 12 +++++-- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java index 5da6de99..76b624ad 100644 --- a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java +++ b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java @@ -353,24 +353,32 @@ private FileChannel newFileChannelFromFile(CryptoPath cleartextFilePath, Effecti } CiphertextFilePath ciphertextPath = cryptoPathMapper.getCiphertextFilePath(cleartextFilePath); Path ciphertextFilePath = ciphertextPath.getFilePath(); + if (options.createNew() && openCryptoFiles.get(ciphertextFilePath).isPresent()) { throw new FileAlreadyExistsException(cleartextFilePath.toString()); - } else { - if (ciphertextPath.isShortened() && options.createNew()) { - Files.createDirectory(ciphertextPath.getRawPath()); // might throw FileAlreadyExists - } else if (ciphertextPath.isShortened()) { - Files.createDirectories(ciphertextPath.getRawPath()); // suppresses FileAlreadyExists - } - FileChannel ch = openCryptoFiles.getOrCreate(ciphertextFilePath).newFileChannel(options); // might throw FileAlreadyExists - if (options.writable()) { + } + + //handle shortened files + if (ciphertextPath.isShortened() && options.createNew()) { + Files.createDirectory(ciphertextPath.getRawPath()); // might throw FileAlreadyExists + } else if (ciphertextPath.isShortened()) { + Files.createDirectories(ciphertextPath.getRawPath()); // suppresses FileAlreadyExists + } + + FileChannel ch = openCryptoFiles.getOrCreate(ciphertextFilePath).newFileChannel(options); // might throw FileAlreadyExists + if (options.writable()) { + try { ciphertextPath.persistLongFileName(); - stats.incrementAccessesWritten(); - } - if (options.readable()) { - stats.incrementAccessesRead(); + } catch (IOException e) { + ch.close(); + throw e; } - return ch; + stats.incrementAccessesWritten(); + } + if (options.readable()) { + stats.incrementAccessesRead(); } + return ch; } void delete(CryptoPath cleartextPath) throws IOException { diff --git a/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java b/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java index cc75fdea..3698bce0 100644 --- a/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java +++ b/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java @@ -6,7 +6,6 @@ import org.cryptomator.cryptofs.attr.AttributeViewProvider; import org.cryptomator.cryptofs.attr.AttributeViewType; import org.cryptomator.cryptofs.common.CiphertextFileType; -import org.cryptomator.cryptofs.common.Constants; import org.cryptomator.cryptofs.common.FinallyUtil; import org.cryptomator.cryptofs.common.RunnableThrowingException; import org.cryptomator.cryptofs.dir.CiphertextDirectoryDeleter; @@ -114,7 +113,7 @@ public void setup() { Path other = invocation.getArgument(0); return other; }); - + when(fileSystemProperties.maxCleartextNameLength()).thenReturn(32768); inTest = new CryptoFileSystemImpl(provider, cryptoFileSystems, pathToVault, cryptor, @@ -421,6 +420,15 @@ public void testNewFileChannelReadOnly() throws IOException { verify(readonlyFlag, Mockito.never()).assertWritable(); } + @Test + @DisplayName("newFileChannel read-write with long filename closed on failed long name persistence") + public void testNewFileChannelClosedOnErrorAfterCreation() throws IOException { + Mockito.doThrow(new IOException("ERROR")).when(ciphertextPath).persistLongFileName(); + + Assertions.assertThrows(IOException.class, () -> inTest.newFileChannel(cleartextPath, EnumSet.of(StandardOpenOption.WRITE))); + Mockito.verify(fileChannel).close(); + } + @Test @DisplayName("newFileChannel read-only with long filename") public void testNewFileChannelReadOnlyShortened() throws IOException { From 61e3fe365faa47483da6f494fafbf7fc8acc9ff2 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Sat, 5 Feb 2022 00:57:59 +0100 Subject: [PATCH 15/28] Fix code smells Co-authored-by: sonarcloud --- .../cryptofs/dir/C9rConflictResolver.java | 15 ++++++--------- .../org/cryptomator/cryptofs/dir/C9sInflator.java | 4 ++-- .../cryptofs/health/dirid/DirIdCheck.java | 2 +- .../health/shortened/NotDecodableLongName.java | 1 - 4 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptofs/dir/C9rConflictResolver.java b/src/main/java/org/cryptomator/cryptofs/dir/C9rConflictResolver.java index 5964ce34..957ad555 100644 --- a/src/main/java/org/cryptomator/cryptofs/dir/C9rConflictResolver.java +++ b/src/main/java/org/cryptomator/cryptofs/dir/C9rConflictResolver.java @@ -13,14 +13,11 @@ import javax.inject.Inject; import javax.inject.Named; import java.io.IOException; -import java.nio.ByteBuffer; -import java.nio.channels.ReadableByteChannel; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.StandardCopyOption; -import java.nio.file.StandardOpenOption; import java.util.stream.Stream; import static org.cryptomator.cryptofs.common.Constants.DIR_FILE_NAME; @@ -49,7 +46,7 @@ public C9rConflictResolver(Cryptor cryptor, @Named("dirId") String dirId, VaultC public Stream process(Node node) { Preconditions.checkArgument(node.extractedCiphertext != null, "Can only resolve conflicts if extractedCiphertext is set"); Preconditions.checkArgument(node.cleartextName != null, "Can only resolve conflicts if cleartextName is set"); - + String canonicalCiphertextFileName = node.extractedCiphertext + Constants.CRYPTOMATOR_FILE_SUFFIX; if (node.fullCiphertextFileName.equals(canonicalCiphertextFileName)) { // not a conflict: @@ -85,9 +82,9 @@ private Stream resolveConflict(Node conflicting, Path canonicalPath) throw /** * Resolves a conflict by renaming the conflicting file. * - * @param canonicalPath The path to the original (conflict-free) file. + * @param canonicalPath The path to the original (conflict-free) file. * @param conflictingPath The path to the potentially conflicting file. - * @param cleartext The cleartext name of the conflicting file. + * @param cleartext The cleartext name of the conflicting file. * @return The newly created Node after renaming the conflicting file. * @throws IOException */ @@ -121,7 +118,7 @@ private Node renameConflictingFile(Path canonicalPath, Path conflictingPath, Str /** * Tries to resolve a conflicting file without renaming the file. If successful, only the file with the canonical path will exist afterwards. * - * @param canonicalPath The path to the original (conflict-free) resource (must not exist). + * @param canonicalPath The path to the original (conflict-free) resource (must not exist). * @param conflictingPath The path to the potentially conflicting file (known to exist). * @return true if the conflict has been resolved. * @throws IOException @@ -144,8 +141,8 @@ private boolean resolveConflictTrivially(Path canonicalPath, Path conflictingPat } /** - * @param conflictingPath Path to a potentially conflicting file supposedly containing a directory id - * @param canonicalPath Path to the canonical file containing a directory id + * @param conflictingPath Path to a potentially conflicting file supposedly containing a directory id + * @param canonicalPath Path to the canonical file containing a directory id * @param numBytesToCompare Number of bytes to read from each file and compare to each other. * @return true if the first numBytesToCompare bytes are equal in both files. * @throws IOException If an I/O exception occurs while reading either file. diff --git a/src/main/java/org/cryptomator/cryptofs/dir/C9sInflator.java b/src/main/java/org/cryptomator/cryptofs/dir/C9sInflator.java index 0c699fb3..6be8a990 100644 --- a/src/main/java/org/cryptomator/cryptofs/dir/C9sInflator.java +++ b/src/main/java/org/cryptomator/cryptofs/dir/C9sInflator.java @@ -38,10 +38,10 @@ public Stream process(Node node) { node.cleartextName = cryptor.fileNameCryptor().decryptFilename(BaseEncoding.base64Url(), node.extractedCiphertext, dirId); return Stream.of(node); } catch (AuthenticationFailedException e) { - LOG.warn(node.ciphertextPath + "'s inflated filename could not be decrypted."); + LOG.warn("{}'s inflated filename could not be decrypted.", node.ciphertextPath); return Stream.empty(); } catch (IOException e) { - LOG.warn(node.ciphertextPath + " could not be inflated."); + LOG.warn("{} could not be inflated.",node.ciphertextPath); return Stream.empty(); } } diff --git a/src/main/java/org/cryptomator/cryptofs/health/dirid/DirIdCheck.java b/src/main/java/org/cryptomator/cryptofs/health/dirid/DirIdCheck.java index b0ed8947..4cc53321 100644 --- a/src/main/java/org/cryptomator/cryptofs/health/dirid/DirIdCheck.java +++ b/src/main/java/org/cryptomator/cryptofs/health/dirid/DirIdCheck.java @@ -104,7 +104,7 @@ private FileVisitResult visitDirFile(Path file, BasicFileAttributes attrs) throw LOG.warn("Encountered dir.c9r file of size {}", attrs.size()); resultCollector.accept(new ObeseDirFile(file, attrs.size())); } else if (attrs.size() == 0) { - LOG.warn("Empty dir.c9r file.", file); + LOG.warn("Empty dir.c9r file at {}.", file); resultCollector.accept(new EmptyDirFile(file)); } else { byte[] bytes = Files.readAllBytes(file); diff --git a/src/main/java/org/cryptomator/cryptofs/health/shortened/NotDecodableLongName.java b/src/main/java/org/cryptomator/cryptofs/health/shortened/NotDecodableLongName.java index f8f1012e..a272cc78 100644 --- a/src/main/java/org/cryptomator/cryptofs/health/shortened/NotDecodableLongName.java +++ b/src/main/java/org/cryptomator/cryptofs/health/shortened/NotDecodableLongName.java @@ -1,6 +1,5 @@ package org.cryptomator.cryptofs.health.shortened; -import org.cryptomator.cryptofs.common.Constants; import org.cryptomator.cryptofs.health.api.CommonDetailKeys; import org.cryptomator.cryptofs.health.api.DiagnosticResult; From 7299696db26097806fc88ab08fa893a8f2f07ea4 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Sat, 5 Feb 2022 01:13:25 +0100 Subject: [PATCH 16/28] update check-dependency-maven plugin --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 92f9eb0d..86692ae8 100644 --- a/pom.xml +++ b/pom.xml @@ -30,7 +30,7 @@ 2.2 - 6.2.2 + 6.5.3 0.8.7 1.6.8 From 2909ca9264adf56ad4c0a781bba5b03b266667cb Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Mon, 7 Feb 2022 13:05:22 +0100 Subject: [PATCH 17/28] refactor fix of a8126b2ef43a92c92b1d0a14306d4d6d3524ca69 to close channel on any exception --- .../cryptofs/CryptoFileSystemImpl.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java index 76b624ad..d1da6a78 100644 --- a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java +++ b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java @@ -366,19 +366,19 @@ private FileChannel newFileChannelFromFile(CryptoPath cleartextFilePath, Effecti } FileChannel ch = openCryptoFiles.getOrCreate(ciphertextFilePath).newFileChannel(options); // might throw FileAlreadyExists - if (options.writable()) { - try { + try { + if (options.writable()) { ciphertextPath.persistLongFileName(); - } catch (IOException e) { - ch.close(); - throw e; + stats.incrementAccessesWritten(); } - stats.incrementAccessesWritten(); - } - if (options.readable()) { - stats.incrementAccessesRead(); + if (options.readable()) { + stats.incrementAccessesRead(); + } + return ch; + } catch (Exception e){ + ch.close(); + throw e; } - return ch; } void delete(CryptoPath cleartextPath) throws IOException { From ae7d04d9d28731e5a05dda91a37fb6402d101faf Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Tue, 15 Mar 2022 10:20:22 +0100 Subject: [PATCH 18/28] dependency bump (fixes CVE-2020-36518) --- pom.xml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pom.xml b/pom.xml index 86692ae8..d92c8220 100644 --- a/pom.xml +++ b/pom.xml @@ -19,18 +19,18 @@ 2.0.3 - 3.18.3 - 2.40.5 - 31.0.1-jre - 1.7.33 + 3.19.0 + 2.41 + 31.1-jre + 1.7.36 5.8.2 - 4.3.1 + 4.4.0 2.2 - 6.5.3 + 7.0.0 0.8.7 1.6.8 From 761dae47f0c3124a2774f0474da18da209103080 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Tue, 15 Mar 2022 10:23:59 +0100 Subject: [PATCH 19/28] avoid false positive due to incorrectly matched CPE --- suppression.xml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/suppression.xml b/suppression.xml index cc17d63b..6861f6c9 100644 --- a/suppression.xml +++ b/suppression.xml @@ -1,4 +1,12 @@ + + + ^pkg:maven/org\.cryptomator/.*$ + cpe:/a:cryptomator:cryptomator + CVE-2022-25366 + \ No newline at end of file From 5465cd2e00e182a22801f656e126a616c7395eb5 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Tue, 15 Mar 2022 11:33:08 +0100 Subject: [PATCH 20/28] fix malformed suppression.xml --- suppression.xml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/suppression.xml b/suppression.xml index 6861f6c9..ebb877b8 100644 --- a/suppression.xml +++ b/suppression.xml @@ -1,12 +1,12 @@ - + - ^pkg:maven/org\.cryptomator/.*$ + org\.cryptomator:.* cpe:/a:cryptomator:cryptomator - CVE-2022-25366 + CVE-2022-25366 \ No newline at end of file From 82093cb359e37f80a4870315784735ec87025eec Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 17 Mar 2022 08:03:20 +0100 Subject: [PATCH 21/28] typo --- src/main/java/org/cryptomator/cryptofs/fh/Chunk.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/cryptomator/cryptofs/fh/Chunk.java b/src/main/java/org/cryptomator/cryptofs/fh/Chunk.java index f53540b6..b2af8c70 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/Chunk.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/Chunk.java @@ -19,7 +19,7 @@ *
  • Capacity of {@code data} is always the cleartext chunk size
    • *
  • During creation, {@code data}'s limit is the chunk's size (last chunk of a file may be smaller)
    • *
  • Writes need to adjust the limit and mark this chunk dirty
    • - *
  • Reads need to respec the limit and must not change it
    • + *
  • Reads need to respect the limit and must not change it
    • *
  • When no longer used, the cleartext ByteBuffer may be recycled
    • * */ From dca25ad81cd07eab00dbc41d0575de05b36100cf Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 17 Mar 2022 08:03:33 +0100 Subject: [PATCH 22/28] fixed TODO --- .../cryptomator/cryptofs/ch/CleartextFileChannel.java | 7 +++++-- .../cryptofs/ch/CleartextFileChannelTest.java | 9 ++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java b/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java index fcafd640..e99b61d8 100644 --- a/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java +++ b/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java @@ -3,6 +3,7 @@ import com.google.common.base.Preconditions; import org.cryptomator.cryptofs.CryptoFileSystemStats; import org.cryptomator.cryptofs.EffectiveOpenOptions; +import org.cryptomator.cryptofs.fh.BufferPool; import org.cryptomator.cryptofs.fh.ByteSource; import org.cryptomator.cryptofs.fh.ChunkCache; import org.cryptomator.cryptofs.fh.Chunk; @@ -42,6 +43,7 @@ public class CleartextFileChannel extends AbstractFileChannel { private final FileHeader fileHeader; private final Cryptor cryptor; private final ChunkCache chunkCache; + private final BufferPool bufferPool; private final EffectiveOpenOptions options; private final AtomicLong fileSize; private final AtomicReference lastModified; @@ -52,12 +54,13 @@ public class CleartextFileChannel extends AbstractFileChannel { private boolean mustWriteHeader; @Inject - public CleartextFileChannel(FileChannel ciphertextFileChannel, FileHeader fileHeader, @MustWriteHeader boolean mustWriteHeader, ReadWriteLock readWriteLock, Cryptor cryptor, ChunkCache chunkCache, EffectiveOpenOptions options, @OpenFileSize AtomicLong fileSize, @OpenFileModifiedDate AtomicReference lastModified, Supplier attrViewProvider, ExceptionsDuringWrite exceptionsDuringWrite, ChannelCloseListener closeListener, CryptoFileSystemStats stats) { + public CleartextFileChannel(FileChannel ciphertextFileChannel, FileHeader fileHeader, @MustWriteHeader boolean mustWriteHeader, ReadWriteLock readWriteLock, Cryptor cryptor, ChunkCache chunkCache, BufferPool bufferPool, EffectiveOpenOptions options, @OpenFileSize AtomicLong fileSize, @OpenFileModifiedDate AtomicReference lastModified, Supplier attrViewProvider, ExceptionsDuringWrite exceptionsDuringWrite, ChannelCloseListener closeListener, CryptoFileSystemStats stats) { super(readWriteLock); this.ciphertextFileChannel = ciphertextFileChannel; this.fileHeader = fileHeader; this.cryptor = cryptor; this.chunkCache = chunkCache; + this.bufferPool = bufferPool; this.options = options; this.fileSize = fileSize; this.lastModified = lastModified; @@ -148,7 +151,7 @@ private long writeLockedInternal(ByteSource src, long position) throws IOExcepti assert len <= cleartextChunkSize; if (offsetInChunk == 0 && len == cleartextChunkSize) { // complete chunk, no need to load and decrypt from file - ByteBuffer cleartextChunk = ByteBuffer.allocate(cleartextChunkSize); // TODO: use BufferPool + ByteBuffer cleartextChunk = bufferPool.getCleartextBuffer(); src.copyTo(cleartextChunk); cleartextChunk.flip(); Chunk chunk = new Chunk(cleartextChunk, true); diff --git a/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java b/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java index 395c4d0e..2acd90ad 100644 --- a/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java +++ b/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java @@ -2,6 +2,7 @@ import org.cryptomator.cryptofs.CryptoFileSystemStats; import org.cryptomator.cryptofs.EffectiveOpenOptions; +import org.cryptomator.cryptofs.fh.BufferPool; import org.cryptomator.cryptofs.fh.ChunkCache; import org.cryptomator.cryptofs.fh.Chunk; import org.cryptomator.cryptofs.fh.ExceptionsDuringWrite; @@ -50,6 +51,7 @@ public class CleartextFileChannelTest { private ChunkCache chunkCache = mock(ChunkCache.class); + private BufferPool bufferPool = mock(BufferPool.class); private ReadWriteLock readWriteLock = mock(ReadWriteLock.class); private Lock readLock = mock(Lock.class); private Lock writeLock = mock(Lock.class); @@ -75,6 +77,7 @@ public void setUp() throws IOException { when(cryptor.fileHeaderCryptor()).thenReturn(fileHeaderCryptor); when(cryptor.fileContentCryptor()).thenReturn(fileContentCryptor); when(chunkCache.get(Mockito.anyLong())).then(invocation -> new Chunk(ByteBuffer.allocate(100), false)); + when(bufferPool.getCleartextBuffer()).thenAnswer(invocation -> ByteBuffer.allocate(100)); when(fileHeaderCryptor.headerSize()).thenReturn(50); when(fileContentCryptor.cleartextChunkSize()).thenReturn(100); when(fileContentCryptor.ciphertextChunkSize()).thenReturn(110); @@ -82,7 +85,7 @@ public void setUp() throws IOException { when(readWriteLock.readLock()).thenReturn(readLock); when(readWriteLock.writeLock()).thenReturn(writeLock); - inTest = new CleartextFileChannel(ciphertextFileChannel, header, mustWriteHeader, readWriteLock, cryptor, chunkCache, options, fileSize, lastModified, attributeViewSupplier, exceptionsDuringWrite, closeListener, stats); + inTest = new CleartextFileChannel(ciphertextFileChannel, header, mustWriteHeader, readWriteLock, cryptor, chunkCache, bufferPool, options, fileSize, lastModified, attributeViewSupplier, exceptionsDuringWrite, closeListener, stats); } @Test @@ -334,7 +337,7 @@ public void testReadFromMultipleChunks() throws IOException { fileSize.set(5_000_000_100l); // initial cleartext size will be 5_000_000_100l when(options.readable()).thenReturn(true); - inTest = new CleartextFileChannel(ciphertextFileChannel, header, mustWriteHeader, readWriteLock, cryptor, chunkCache, options, fileSize, lastModified, attributeViewSupplier, exceptionsDuringWrite, closeListener, stats); + inTest = new CleartextFileChannel(ciphertextFileChannel, header, mustWriteHeader, readWriteLock, cryptor, chunkCache, bufferPool, options, fileSize, lastModified, attributeViewSupplier, exceptionsDuringWrite, closeListener, stats); ByteBuffer buf = ByteBuffer.allocate(10); // A read from frist chunk: @@ -488,7 +491,7 @@ public void testWriteHeaderIfNeeded() throws IOException { @DisplayName("don't write header if it is already written") public void testDontRewriteHeader() throws IOException { when(options.writable()).thenReturn(true); - inTest = new CleartextFileChannel(ciphertextFileChannel, header, false, readWriteLock, cryptor, chunkCache, options, fileSize, lastModified, attributeViewSupplier, exceptionsDuringWrite, closeListener, stats); + inTest = new CleartextFileChannel(ciphertextFileChannel, header, false, readWriteLock, cryptor, chunkCache, bufferPool, options, fileSize, lastModified, attributeViewSupplier, exceptionsDuringWrite, closeListener, stats); inTest.force(true); From 478a232502599f6863509fe7056f6bc60416e10c Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 17 Mar 2022 08:13:05 +0100 Subject: [PATCH 23/28] less verbose logging during tests --- pom.xml | 3 +++ .../org/cryptomator/cryptofs/CryptoFileSystemUriTest.java | 1 - .../cryptofs/attr/AttributeByNameProviderTest.java | 8 -------- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/pom.xml b/pom.xml index d92c8220..f8396b44 100644 --- a/pom.xml +++ b/pom.xml @@ -142,6 +142,9 @@ maven-surefire-plugin 3.0.0-M5 + + ERROR + false diff --git a/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemUriTest.java b/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemUriTest.java index 279f2ce1..c1e14313 100644 --- a/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemUriTest.java +++ b/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemUriTest.java @@ -63,7 +63,6 @@ public void testCreateWithPathComponents() throws URISyntaxException { Path absolutePathToVault = Paths.get("c").toAbsolutePath(); URI uri = CryptoFileSystemUri.create(absolutePathToVault, "a", "b", "c"); - System.out.println(uri); CryptoFileSystemUri parsed = CryptoFileSystemUri.parse(uri); Assertions.assertEquals(absolutePathToVault, parsed.pathToVault()); diff --git a/src/test/java/org/cryptomator/cryptofs/attr/AttributeByNameProviderTest.java b/src/test/java/org/cryptomator/cryptofs/attr/AttributeByNameProviderTest.java index efef67a9..fd7986e3 100644 --- a/src/test/java/org/cryptomator/cryptofs/attr/AttributeByNameProviderTest.java +++ b/src/test/java/org/cryptomator/cryptofs/attr/AttributeByNameProviderTest.java @@ -89,8 +89,6 @@ public void testReadBasicAttributesReturnsAttributeValues() throws IOException { Map values = inTest.readAttributes(path, "basic:creationTime"); - System.out.println(values); - MatcherAssert.assertThat(values.entrySet(), contains(entry(is("creationTime"), is(creationTime)))); } @@ -120,8 +118,6 @@ public void testReadAllBasicAttributesReturnsAttributeValues() throws IOExceptio Map values = inTest.readAttributes(path, "basic:*"); - System.out.println(values); - MatcherAssert.assertThat(values.entrySet(), containsInAnyOrder( // entry(is("creationTime"), is(creationTime)), // @@ -161,8 +157,6 @@ public void testReadAllBasicAttributesWithoutViewNameReturnsAttributeValues() th Map values = inTest.readAttributes(path, "*"); - System.out.println(values); - MatcherAssert.assertThat(values.entrySet(), containsInAnyOrder( // entry(is("creationTime"), is(creationTime)), // @@ -192,8 +186,6 @@ public void testReadDosAttributesReturnsAttributeValues() throws IOException { Map values = inTest.readAttributes(path, "dos:readOnly,hidden"); - System.out.println(values); - MatcherAssert.assertThat(values.entrySet(), containsInAnyOrder( // entry(is("readOnly"), is(readOnly)), // From 152d39b65aff391fad2080e494cb7bea06a4201b Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 17 Mar 2022 15:01:10 +0100 Subject: [PATCH 24/28] renamed variable --- .../org/cryptomator/cryptofs/ch/CleartextFileChannel.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java b/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java index e99b61d8..46bad90f 100644 --- a/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java +++ b/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java @@ -151,10 +151,10 @@ private long writeLockedInternal(ByteSource src, long position) throws IOExcepti assert len <= cleartextChunkSize; if (offsetInChunk == 0 && len == cleartextChunkSize) { // complete chunk, no need to load and decrypt from file - ByteBuffer cleartextChunk = bufferPool.getCleartextBuffer(); - src.copyTo(cleartextChunk); - cleartextChunk.flip(); - Chunk chunk = new Chunk(cleartextChunk, true); + ByteBuffer cleartextChunkData = bufferPool.getCleartextBuffer(); + src.copyTo(cleartextChunkData); + cleartextChunkData.flip(); + Chunk chunk = new Chunk(cleartextChunkData, true); chunkCache.set(chunkIndex, chunk); } else { /* From 20fa99bb85e664b5ce8bc732e79157caf9c3d36a Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Thu, 24 Mar 2022 17:29:17 +0100 Subject: [PATCH 25/28] Remove Codacy upload and badges --- .github/workflows/build.yml | 6 ------ README.md | 4 ++-- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ed85d8a5..ab78cff5 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -36,12 +36,6 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} - - name: Upload code coverage report - id: codacyCoverageReporter - run: bash <(curl -Ls https://coverage.codacy.com/get.sh) - env: - CODACY_PROJECT_TOKEN: ${{ secrets.CODACY_PROJECT_TOKEN }} - continue-on-error: true - uses: actions/upload-artifact@v2 with: name: artifacts diff --git a/README.md b/README.md index 0a600b5a..53e70212 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,8 @@ ![cryptomator](cryptomator.png) [![Build](https://github.com/cryptomator/cryptofs/workflows/Build/badge.svg)](https://github.com/cryptomator/cryptofs/actions?query=workflow%3ABuild) -[![Codacy Badge](https://api.codacy.com/project/badge/Grade/7248ca7d466843f785f79f33374302c2)](https://www.codacy.com/gh/cryptomator/cryptofs/dashboard) -[![Codacy Badge](https://api.codacy.com/project/badge/Coverage/7248ca7d466843f785f79f33374302c2)](https://www.codacy.com/gh/cryptomator/cryptofs/dashboard) +[![Quality Gate Status](https://sonarcloud.io/api/project_badges/measure?project=cryptomator_cryptofs&metric=alert_status)](https://sonarcloud.io/dashboard?id=cryptomator_cryptofs) +[![Coverage](https://sonarcloud.io/api/project_badges/measure?project=cryptomator_cryptofs&metric=coverage)](https://sonarcloud.io/dashboard?id=cryptomator_cryptofs) [![Known Vulnerabilities](https://snyk.io/test/github/cryptomator/cryptofs/badge.svg)](https://snyk.io/test/github/cryptomator/cryptofs) **CryptoFS:** Implementation of the [Cryptomator](https://github.com/cryptomator/cryptomator) encryption scheme. From 826738094d8c252e21c301ef0fa212080462445d Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 30 Mar 2022 14:32:39 +0200 Subject: [PATCH 26/28] Bump jwt package to fix vulnerability --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index f8396b44..8ea86eaa 100644 --- a/pom.xml +++ b/pom.xml @@ -19,7 +19,7 @@ 2.0.3 - 3.19.0 + 3.19.1 2.41 31.1-jre 1.7.36 From a4f567832f380ad60ee1b38b6b1d55931397f02d Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 30 Mar 2022 15:26:25 +0200 Subject: [PATCH 27/28] bump maven build plugins --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 8ea86eaa..47784299 100644 --- a/pom.xml +++ b/pom.xml @@ -30,9 +30,9 @@ 2.2 - 7.0.0 + 7.0.3 0.8.7 - 1.6.8 + 1.6.12 From d8d8c561eb0af9a42e40d1b0a6bcb03a5fe1d58b Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 30 Mar 2022 15:28:43 +0200 Subject: [PATCH 28/28] prepare 2.4.0 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 47784299..a393d0c2 100644 --- a/pom.xml +++ b/pom.xml @@ -2,7 +2,7 @@ 4.0.0 org.cryptomator cryptofs - 2.4.0-SNAPSHOT + 2.4.0 Cryptomator Crypto Filesystem This library provides the Java filesystem provider used by Cryptomator. https://github.com/cryptomator/cryptofs