From 086a4f44cc78c9f24be35bf3e64cbb9327515b05 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 22 May 2019 14:09:35 +0200 Subject: [PATCH 1/5] LongFileNameProvider will no longer implicitly create files in the "m" directory. This is now only done explicitly when a file/dir/symlink is created or moved. fixes #52 --- .../cryptofs/ConflictResolver.java | 4 +- .../cryptofs/CryptoFileSystemImpl.java | 16 +++++- .../cryptofs/LongFileNameProvider.java | 57 ++++++++++++------- .../org/cryptomator/cryptofs/Symlinks.java | 5 +- .../cryptofs/CryptoFileSystemImplTest.java | 3 +- .../cryptofs/LongFileNameProviderTest.java | 19 ++++--- .../cryptomator/cryptofs/SymlinksTest.java | 3 +- 7 files changed, 71 insertions(+), 36 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptofs/ConflictResolver.java b/src/main/java/org/cryptomator/cryptofs/ConflictResolver.java index 79ca7fd3..38ccec01 100644 --- a/src/main/java/org/cryptomator/cryptofs/ConflictResolver.java +++ b/src/main/java/org/cryptomator/cryptofs/ConflictResolver.java @@ -121,7 +121,9 @@ private Path renameConflictingFile(Path canonicalPath, Path conflictingPath, Str alternativePath = canonicalPath.resolveSibling(alternativeCiphertextFileName); } LOG.info("Moving conflicting file {} to {}", conflictingPath, alternativePath); - return Files.move(conflictingPath, alternativePath, StandardCopyOption.ATOMIC_MOVE); + Path resolved = Files.move(conflictingPath, alternativePath, StandardCopyOption.ATOMIC_MOVE); + longFileNameProvider.persistCachedIfDeflated(resolved); + return resolved; } catch (AuthenticationFailedException e) { // not decryptable, no need to resolve any kind of conflict LOG.info("Found valid Base32 string, which is an unauthentic ciphertext: {}", conflictingPath); diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java index 18d9cc67..4322b0ba 100644 --- a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java +++ b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java @@ -75,6 +75,7 @@ class CryptoFileSystemImpl extends CryptoFileSystem { private final CryptoFileStore fileStore; private final CryptoFileSystemStats stats; private final CryptoPathMapper cryptoPathMapper; + private final LongFileNameProvider longFileNameProvider; private final CryptoPathFactory cryptoPathFactory; private final PathMatcherFactory pathMatcherFactory; private final DirectoryStreamFactory directoryStreamFactory; @@ -95,7 +96,7 @@ class CryptoFileSystemImpl extends CryptoFileSystem { @Inject public CryptoFileSystemImpl(CryptoFileSystemProvider provider, CryptoFileSystems cryptoFileSystems, @PathToVault Path pathToVault, Cryptor cryptor, - CryptoFileStore fileStore, CryptoFileSystemStats stats, CryptoPathMapper cryptoPathMapper, CryptoPathFactory cryptoPathFactory, + CryptoFileStore fileStore, CryptoFileSystemStats stats, CryptoPathMapper cryptoPathMapper, LongFileNameProvider longFileNameProvider, CryptoPathFactory cryptoPathFactory, PathMatcherFactory pathMatcherFactory, DirectoryStreamFactory directoryStreamFactory, DirectoryIdProvider dirIdProvider, AttributeProvider fileAttributeProvider, AttributeByNameProvider fileAttributeByNameProvider, AttributeViewProvider fileAttributeViewProvider, OpenCryptoFiles openCryptoFiles, Symlinks symlinks, FinallyUtil finallyUtil, CiphertextDirectoryDeleter ciphertextDirDeleter, ReadonlyFlag readonlyFlag, RootDirectoryInitializer rootDirectoryInitializer) { @@ -106,6 +107,7 @@ public CryptoFileSystemImpl(CryptoFileSystemProvider provider, CryptoFileSystems this.fileStore = fileStore; this.stats = stats; this.cryptoPathMapper = cryptoPathMapper; + this.longFileNameProvider = longFileNameProvider; this.cryptoPathFactory = cryptoPathFactory; this.pathMatcherFactory = pathMatcherFactory; this.directoryStreamFactory = directoryStreamFactory; @@ -302,6 +304,7 @@ void createDirectory(CryptoPath cleartextDir, FileAttribute... attrs) throws // create dir if and only if the dirFile has been created right now (not if it has been created before): try { Files.createDirectories(ciphertextDir.path); + longFileNameProvider.persistCachedIfDeflated(ciphertextDirFile); } catch (IOException e) { // make sure there is no orphan dir file: Files.delete(ciphertextDirFile); @@ -351,7 +354,9 @@ private FileChannel newFileChannel(CryptoPath cleartextFilePath, EffectiveOpenOp throw new FileAlreadyExistsException(cleartextFilePath.toString()); } else { // might also throw FileAlreadyExists: - return openCryptoFiles.getOrCreate(ciphertextPath).newFileChannel(options); + FileChannel ch = openCryptoFiles.getOrCreate(ciphertextPath).newFileChannel(options); + longFileNameProvider.persistCachedIfDeflated(ciphertextPath); + return ch; } } @@ -419,6 +424,7 @@ private void copySymlink(CryptoPath cleartextSource, CryptoPath cleartextTarget, Path ciphertextTargetFile = cryptoPathMapper.getCiphertextFilePath(cleartextTarget, CiphertextFileType.SYMLINK); CopyOption[] resolvedOptions = ArrayUtils.without(options, LinkOption.NOFOLLOW_LINKS).toArray(CopyOption[]::new); Files.copy(ciphertextSourceFile, ciphertextTargetFile, resolvedOptions); + longFileNameProvider.persistCachedIfDeflated(ciphertextTargetFile); } else { CryptoPath resolvedSource = symlinks.resolveRecursively(cleartextSource); CryptoPath resolvedTarget = symlinks.resolveRecursively(cleartextTarget); @@ -431,6 +437,7 @@ private void copyFile(CryptoPath cleartextSource, CryptoPath cleartextTarget, Co Path ciphertextSourceFile = cryptoPathMapper.getCiphertextFilePath(cleartextSource, CiphertextFileType.FILE); Path ciphertextTargetFile = cryptoPathMapper.getCiphertextFilePath(cleartextTarget, CiphertextFileType.FILE); Files.copy(ciphertextSourceFile, ciphertextTargetFile, options); + longFileNameProvider.persistCachedIfDeflated(ciphertextTargetFile); } private void copyDirectory(CryptoPath cleartextSource, CryptoPath cleartextTarget, CopyOption[] options) throws IOException { @@ -439,6 +446,7 @@ private void copyDirectory(CryptoPath cleartextSource, CryptoPath cleartextTarge if (Files.notExists(ciphertextTargetDirFile)) { // create new: createDirectory(cleartextTarget); + longFileNameProvider.persistCachedIfDeflated(ciphertextTargetDirFile); } else if (ArrayUtils.contains(options, StandardCopyOption.REPLACE_EXISTING)) { // keep existing (if empty): Path ciphertextTargetDir = cryptoPathMapper.getCiphertextDir(cleartextTarget).path; @@ -517,6 +525,7 @@ private void moveSymlink(CryptoPath cleartextSource, CryptoPath cleartextTarget, Path ciphertextTargetFile = cryptoPathMapper.getCiphertextFilePath(cleartextTarget, CiphertextFileType.SYMLINK); try (OpenCryptoFiles.TwoPhaseMove twoPhaseMove = openCryptoFiles.prepareMove(ciphertextSourceFile, ciphertextTargetFile)) { Files.move(ciphertextSourceFile, ciphertextTargetFile, options); + longFileNameProvider.persistCachedIfDeflated(ciphertextTargetFile); twoPhaseMove.commit(); } } @@ -528,6 +537,7 @@ private void moveFile(CryptoPath cleartextSource, CryptoPath cleartextTarget, Co Path ciphertextTargetFile = cryptoPathMapper.getCiphertextFilePath(cleartextTarget, CiphertextFileType.FILE); try (OpenCryptoFiles.TwoPhaseMove twoPhaseMove = openCryptoFiles.prepareMove(ciphertextSourceFile, ciphertextTargetFile)) { Files.move(ciphertextSourceFile, ciphertextTargetFile, options); + longFileNameProvider.persistCachedIfDeflated(ciphertextTargetFile); twoPhaseMove.commit(); } } @@ -540,6 +550,7 @@ private void moveDirectory(CryptoPath cleartextSource, CryptoPath cleartextTarge if (!ArrayUtils.contains(options, StandardCopyOption.REPLACE_EXISTING)) { // try to move, don't replace: Files.move(ciphertextSourceDirFile, ciphertextTargetDirFile, options); + longFileNameProvider.persistCachedIfDeflated(ciphertextTargetDirFile); } else if (ArrayUtils.contains(options, StandardCopyOption.ATOMIC_MOVE)) { // replace atomically (impossible): assert ArrayUtils.contains(options, StandardCopyOption.REPLACE_EXISTING); @@ -558,6 +569,7 @@ private void moveDirectory(CryptoPath cleartextSource, CryptoPath cleartextTarge Files.delete(ciphertextTargetDir); } Files.move(ciphertextSourceDirFile, ciphertextTargetDirFile, options); + longFileNameProvider.persistCachedIfDeflated(ciphertextTargetDirFile); } dirIdProvider.move(ciphertextSourceDirFile, ciphertextTargetDirFile); cryptoPathMapper.invalidatePathMapping(cleartextSource); diff --git a/src/main/java/org/cryptomator/cryptofs/LongFileNameProvider.java b/src/main/java/org/cryptomator/cryptofs/LongFileNameProvider.java index db9fa99e..cb4cad3b 100644 --- a/src/main/java/org/cryptomator/cryptofs/LongFileNameProvider.java +++ b/src/main/java/org/cryptomator/cryptofs/LongFileNameProvider.java @@ -8,6 +8,7 @@ *******************************************************************************/ package org.cryptomator.cryptofs; +import com.google.common.base.Throwables; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; @@ -24,6 +25,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; +import java.time.Duration; import java.util.Arrays; import java.util.concurrent.ExecutionException; @@ -34,7 +36,7 @@ class LongFileNameProvider { private static final BaseEncoding BASE32 = BaseEncoding.base32(); - private static final int MAX_CACHE_SIZE = 5000; + private static final Duration MAX_CACHE_AGE = Duration.ofMinutes(1); public static final String LONG_NAME_FILE_EXT = ".lng"; private final Path metadataRoot; @@ -43,7 +45,7 @@ class LongFileNameProvider { @Inject public LongFileNameProvider(@PathToVault Path pathToVault) { this.metadataRoot = pathToVault.resolve(METADATA_DIR_NAME); - this.longNames = CacheBuilder.newBuilder().maximumSize(MAX_CACHE_SIZE).build(new Loader()); + this.longNames = CacheBuilder.newBuilder().expireAfterAccess(MAX_CACHE_AGE).build(new Loader()); } private class Loader extends CacheLoader { @@ -64,36 +66,49 @@ public String inflate(String shortFileName) throws IOException { try { return longNames.get(shortFileName); } catch (ExecutionException e) { - if (e.getCause() instanceof IOException || e.getCause() instanceof UncheckedIOException) { - throw new IOException(e); - } else { - throw new UncheckedExecutionException("Unexpected exception", e); - } + Throwables.throwIfInstanceOf(e.getCause(), IOException.class); + throw new IllegalStateException("Unexpected exception", e); } } - public String deflate(String longFileName) throws IOException { + public String deflate(String longFileName) { byte[] longFileNameBytes = longFileName.getBytes(UTF_8); byte[] hash = MessageDigestSupplier.SHA1.get().digest(longFileNameBytes); String shortName = BASE32.encode(hash) + LONG_NAME_FILE_EXT; - if (longNames.getIfPresent(shortName) == null) { + String cachedLongName = longNames.getIfPresent(shortName); + if (cachedLongName == null) { longNames.put(shortName, longFileName); - // TODO markuskreusch, overheadhunter: do we really want to persist this at this point?... - // ...maybe the caller only wanted to know if a file exists without creating anything. - Path file = resolveMetadataFile(shortName); - Path fileDir = file.getParent(); - assert fileDir != null : "resolveMetadataFile returned path to a file"; - Files.createDirectories(fileDir); - try (WritableByteChannel ch = Files.newByteChannel(file, StandardOpenOption.WRITE, StandardOpenOption.CREATE_NEW)) { - ch.write(ByteBuffer.wrap(longFileNameBytes)); - } catch (FileAlreadyExistsException e) { - // no-op: if the file already exists, we assume its content to be what we want (or we found a SHA1 collision ;-)) - assert Arrays.equals(Files.readAllBytes(file), longFileNameBytes); - } + } else { + assert cachedLongName.equals(longFileName); } return shortName; } + public void persistCachedIfDeflated(Path ciphertextFile) throws IOException { + String filename = ciphertextFile.getFileName().toString(); + if (isDeflated(filename)) { + persistCached(filename); + } + } + + // visible for testing + void persistCached(String shortName) throws IOException { + String longName = longNames.getIfPresent(shortName); + if (longName == null) { + throw new IllegalStateException("Long name for " + shortName + " has not been shortened within the last " + MAX_CACHE_AGE); + } + Path file = resolveMetadataFile(shortName); + Path fileDir = file.getParent(); + assert fileDir != null : "resolveMetadataFile returned path to a file"; + Files.createDirectories(fileDir); + try (WritableByteChannel ch = Files.newByteChannel(file, StandardOpenOption.WRITE, StandardOpenOption.CREATE_NEW)) { + ch.write(UTF_8.encode(longName)); + } catch (FileAlreadyExistsException e) { + // no-op: if the file already exists, we assume its content to be what we want (or we found a SHA1 collision ;-)) + assert Arrays.equals(Files.readAllBytes(file), longName.getBytes(UTF_8)); + } + } + private Path resolveMetadataFile(String shortName) { return metadataRoot.resolve(shortName.substring(0, 2)).resolve(shortName.substring(2, 4)).resolve(shortName); } diff --git a/src/main/java/org/cryptomator/cryptofs/Symlinks.java b/src/main/java/org/cryptomator/cryptofs/Symlinks.java index 69f841e5..8af6bfcb 100644 --- a/src/main/java/org/cryptomator/cryptofs/Symlinks.java +++ b/src/main/java/org/cryptomator/cryptofs/Symlinks.java @@ -22,12 +22,14 @@ public class Symlinks { private final CryptoPathMapper cryptoPathMapper; + private final LongFileNameProvider longFileNameProvider; private final OpenCryptoFiles openCryptoFiles; private final ReadonlyFlag readonlyFlag; @Inject - Symlinks(CryptoPathMapper cryptoPathMapper, OpenCryptoFiles openCryptoFiles, ReadonlyFlag readonlyFlag) { + Symlinks(CryptoPathMapper cryptoPathMapper, LongFileNameProvider longFileNameProvider, OpenCryptoFiles openCryptoFiles, ReadonlyFlag readonlyFlag) { this.cryptoPathMapper = cryptoPathMapper; + this.longFileNameProvider = longFileNameProvider; this.openCryptoFiles = openCryptoFiles; this.readonlyFlag = readonlyFlag; } @@ -41,6 +43,7 @@ public void createSymbolicLink(CryptoPath cleartextPath, Path target, FileAttrib EffectiveOpenOptions openOptions = EffectiveOpenOptions.from(EnumSet.of(StandardOpenOption.WRITE, StandardOpenOption.CREATE_NEW), readonlyFlag); ByteBuffer content = UTF_8.encode(target.toString()); openCryptoFiles.writeCiphertextFile(ciphertextSymlinkFile, openOptions, content); + longFileNameProvider.persistCachedIfDeflated(ciphertextSymlinkFile); } public CryptoPath readSymbolicLink(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 c2c6febf..60460c81 100644 --- a/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java +++ b/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java @@ -80,6 +80,7 @@ public class CryptoFileSystemImplTest { private final OpenCryptoFiles openCryptoFiles = mock(OpenCryptoFiles.class); private final Symlinks symlinks = mock(Symlinks.class); private final CryptoPathMapper cryptoPathMapper = mock(CryptoPathMapper.class); + private final LongFileNameProvider longFileNameProvider = Mockito.mock(LongFileNameProvider.class); private final DirectoryIdProvider dirIdProvider = mock(DirectoryIdProvider.class); private final AttributeProvider fileAttributeProvider = mock(AttributeProvider.class); private final AttributeByNameProvider fileAttributeByNameProvider = mock(AttributeByNameProvider.class); @@ -104,7 +105,7 @@ public void setup() { when(cryptoPathFactory.emptyFor(any())).thenReturn(empty); inTest = new CryptoFileSystemImpl(provider, cryptoFileSystems, pathToVault, cryptor, - fileStore, stats, cryptoPathMapper, cryptoPathFactory, + fileStore, stats, cryptoPathMapper, longFileNameProvider, cryptoPathFactory, pathMatcherFactory, directoryStreamFactory, dirIdProvider, fileAttributeProvider, fileAttributeByNameProvider, fileAttributeViewProvider, openCryptoFiles, symlinks, finallyUtil, ciphertextDirDeleter, readonlyFlag, rootDirectoryInitializer); diff --git a/src/test/java/org/cryptomator/cryptofs/LongFileNameProviderTest.java b/src/test/java/org/cryptomator/cryptofs/LongFileNameProviderTest.java index 9e039efd..a90fe9b7 100644 --- a/src/test/java/org/cryptomator/cryptofs/LongFileNameProviderTest.java +++ b/src/test/java/org/cryptomator/cryptofs/LongFileNameProviderTest.java @@ -15,6 +15,7 @@ import java.io.IOException; import java.nio.file.FileVisitResult; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; @@ -47,20 +48,25 @@ public void testIsDeflated(@TempDir Path tmpPath) throws IOException { @Test public void testDeflateAndInflate(@TempDir Path tmpPath) throws IOException { String orig = "longName"; - Assertions.assertEquals(0, countFiles(tmpPath)); LongFileNameProvider prov1 = new LongFileNameProvider(tmpPath); String deflated = prov1.deflate(orig); + String inflated1 = prov1.inflate(deflated); + Assertions.assertEquals(orig, inflated1); + + Assertions.assertEquals(0, countFiles(tmpPath)); + prov1.persistCached(deflated); Assertions.assertEquals(1, countFiles(tmpPath)); + LongFileNameProvider prov2 = new LongFileNameProvider(tmpPath); - String inflated = prov2.inflate(deflated); - Assertions.assertEquals(orig, inflated); + String inflated2 = prov2.inflate(deflated); + Assertions.assertEquals(orig, inflated2); } @Test public void testInflateNonExisting(@TempDir Path tmpPath) throws IOException { LongFileNameProvider prov = new LongFileNameProvider(tmpPath); - Assertions.assertThrows(IOException.class, () -> { + Assertions.assertThrows(NoSuchFileException.class, () -> { prov.inflate("doesNotExist"); }); } @@ -69,15 +75,10 @@ public void testInflateNonExisting(@TempDir Path tmpPath) throws IOException { public void testDeflateMultipleTimes(@TempDir Path tmpPath) throws IOException { LongFileNameProvider prov = new LongFileNameProvider(tmpPath); String orig = "longName"; - Assertions.assertEquals(0, countFiles(tmpPath)); prov.deflate(orig); - Assertions.assertEquals(1, countFiles(tmpPath)); prov.deflate(orig); - Assertions.assertEquals(1, countFiles(tmpPath)); prov.deflate(orig); - Assertions.assertEquals(1, countFiles(tmpPath)); prov.deflate(orig); - Assertions.assertEquals(1, countFiles(tmpPath)); } } diff --git a/src/test/java/org/cryptomator/cryptofs/SymlinksTest.java b/src/test/java/org/cryptomator/cryptofs/SymlinksTest.java index f1e10408..89e3d0d8 100644 --- a/src/test/java/org/cryptomator/cryptofs/SymlinksTest.java +++ b/src/test/java/org/cryptomator/cryptofs/SymlinksTest.java @@ -18,6 +18,7 @@ public class SymlinksTest { private final CryptoPathMapper cryptoPathMapper = Mockito.mock(CryptoPathMapper.class); + private final LongFileNameProvider longFileNameProvider = Mockito.mock(LongFileNameProvider.class); private final OpenCryptoFiles openCryptoFiles = Mockito.mock(OpenCryptoFiles.class); private final ReadonlyFlag readonlyFlag = Mockito.mock(ReadonlyFlag.class); @@ -30,7 +31,7 @@ public class SymlinksTest { @BeforeEach public void setup() throws IOException { - inTest = new Symlinks(cryptoPathMapper, openCryptoFiles, readonlyFlag); + inTest = new Symlinks(cryptoPathMapper, longFileNameProvider, openCryptoFiles, readonlyFlag); Mockito.when(cleartextPath.getFileSystem()).thenReturn(fs); Mockito.when(openCryptoFiles.getOrCreate(ciphertextPath)).thenReturn(ciphertextFile); From d711d01a4f87f5211c6b9343ca4c7a1ed0ad5b63 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 22 May 2019 14:33:03 +0200 Subject: [PATCH 2/5] fixes #53 --- .../cryptofs/LongFileNameProvider.java | 5 ++- .../cryptofs/LongFileNameProviderTest.java | 34 +++++++++++++------ 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptofs/LongFileNameProvider.java b/src/main/java/org/cryptomator/cryptofs/LongFileNameProvider.java index cb4cad3b..5dc965ec 100644 --- a/src/main/java/org/cryptomator/cryptofs/LongFileNameProvider.java +++ b/src/main/java/org/cryptomator/cryptofs/LongFileNameProvider.java @@ -40,11 +40,13 @@ class LongFileNameProvider { public static final String LONG_NAME_FILE_EXT = ".lng"; private final Path metadataRoot; + private final ReadonlyFlag readonlyFlag; private final LoadingCache longNames; @Inject - public LongFileNameProvider(@PathToVault Path pathToVault) { + public LongFileNameProvider(@PathToVault Path pathToVault, ReadonlyFlag readonlyFlag) { this.metadataRoot = pathToVault.resolve(METADATA_DIR_NAME); + this.readonlyFlag = readonlyFlag; this.longNames = CacheBuilder.newBuilder().expireAfterAccess(MAX_CACHE_AGE).build(new Loader()); } @@ -93,6 +95,7 @@ public void persistCachedIfDeflated(Path ciphertextFile) throws IOException { // visible for testing void persistCached(String shortName) throws IOException { + readonlyFlag.assertWritable(); String longName = longNames.getIfPresent(shortName); if (longName == null) { throw new IllegalStateException("Long name for " + shortName + " has not been shortened within the last " + MAX_CACHE_AGE); diff --git a/src/test/java/org/cryptomator/cryptofs/LongFileNameProviderTest.java b/src/test/java/org/cryptomator/cryptofs/LongFileNameProviderTest.java index a90fe9b7..1518f4dd 100644 --- a/src/test/java/org/cryptomator/cryptofs/LongFileNameProviderTest.java +++ b/src/test/java/org/cryptomator/cryptofs/LongFileNameProviderTest.java @@ -11,12 +11,14 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mockito; import java.io.IOException; import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Path; +import java.nio.file.ReadOnlyFileSystemException; import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; import java.util.Collections; @@ -24,12 +26,14 @@ public class LongFileNameProviderTest { + private final ReadonlyFlag readonlyFlag = Mockito.mock(ReadonlyFlag.class); + private int countFiles(Path dir) throws IOException { AtomicInteger count = new AtomicInteger(); Files.walkFileTree(dir, Collections.emptySet(), 2, new SimpleFileVisitor() { @Override - public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { count.incrementAndGet(); return FileVisitResult.CONTINUE; } @@ -39,16 +43,16 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO } @Test - public void testIsDeflated(@TempDir Path tmpPath) throws IOException { + public void testIsDeflated(@TempDir Path tmpPath) { Path aPath = tmpPath.resolve("foo"); - Assertions.assertTrue(new LongFileNameProvider(aPath).isDeflated("foo.lng")); - Assertions.assertFalse(new LongFileNameProvider(aPath).isDeflated("foo.txt")); + Assertions.assertTrue(new LongFileNameProvider(aPath, readonlyFlag).isDeflated("foo.lng")); + Assertions.assertFalse(new LongFileNameProvider(aPath, readonlyFlag).isDeflated("foo.txt")); } @Test public void testDeflateAndInflate(@TempDir Path tmpPath) throws IOException { String orig = "longName"; - LongFileNameProvider prov1 = new LongFileNameProvider(tmpPath); + LongFileNameProvider prov1 = new LongFileNameProvider(tmpPath, readonlyFlag); String deflated = prov1.deflate(orig); String inflated1 = prov1.inflate(deflated); Assertions.assertEquals(orig, inflated1); @@ -57,14 +61,14 @@ public void testDeflateAndInflate(@TempDir Path tmpPath) throws IOException { prov1.persistCached(deflated); Assertions.assertEquals(1, countFiles(tmpPath)); - LongFileNameProvider prov2 = new LongFileNameProvider(tmpPath); + LongFileNameProvider prov2 = new LongFileNameProvider(tmpPath, readonlyFlag); String inflated2 = prov2.inflate(deflated); Assertions.assertEquals(orig, inflated2); } @Test - public void testInflateNonExisting(@TempDir Path tmpPath) throws IOException { - LongFileNameProvider prov = new LongFileNameProvider(tmpPath); + public void testInflateNonExisting(@TempDir Path tmpPath) { + LongFileNameProvider prov = new LongFileNameProvider(tmpPath, readonlyFlag); Assertions.assertThrows(NoSuchFileException.class, () -> { prov.inflate("doesNotExist"); @@ -72,8 +76,8 @@ public void testInflateNonExisting(@TempDir Path tmpPath) throws IOException { } @Test - public void testDeflateMultipleTimes(@TempDir Path tmpPath) throws IOException { - LongFileNameProvider prov = new LongFileNameProvider(tmpPath); + public void testDeflateMultipleTimes(@TempDir Path tmpPath) { + LongFileNameProvider prov = new LongFileNameProvider(tmpPath, readonlyFlag); String orig = "longName"; prov.deflate(orig); prov.deflate(orig); @@ -81,4 +85,14 @@ public void testDeflateMultipleTimes(@TempDir Path tmpPath) throws IOException { prov.deflate(orig); } + @Test + public void testPerstistCachedFailsOnReadOnlyFileSystems(@TempDir Path tmpPath) { + LongFileNameProvider prov = new LongFileNameProvider(tmpPath, readonlyFlag); + + Mockito.doThrow(new ReadOnlyFileSystemException()).when(readonlyFlag).assertWritable(); + Assertions.assertThrows(ReadOnlyFileSystemException.class, () -> { + prov.persistCached("whatever"); + }); + } + } From 1b8bf367fec58562051175b3cc8cc7869f7789e0 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 22 May 2019 15:43:34 +0200 Subject: [PATCH 3/5] Write the file header only if it is a new file header fixes #58 --- .../cryptofs/ch/ChannelComponent.java | 7 ++ .../cryptofs/ch/CleartextFileChannel.java | 19 +++--- .../cryptofs/ch/MustWriteHeader.java | 13 ++++ .../org/cryptomator/cryptofs/fh/ChunkIO.java | 4 -- .../cryptomator/cryptofs/fh/ChunkLoader.java | 8 +-- .../cryptomator/cryptofs/fh/ChunkSaver.java | 8 +-- .../cryptofs/fh/FileHeaderHolder.java | 64 +++++++++++++++++++ .../cryptofs/fh/FileHeaderLoader.java | 63 ------------------ .../cryptofs/fh/OpenCryptoFile.java | 17 ++++- .../cryptofs/ch/CleartextFileChannelTest.java | 9 +-- .../cryptofs/fh/ChunkLoaderTest.java | 6 +- .../cryptofs/fh/ChunkSaverTest.java | 6 +- ...derTest.java => FileHeaderHolderTest.java} | 19 +++--- .../cryptofs/fh/OpenCryptoFileTest.java | 15 +++-- 14 files changed, 145 insertions(+), 113 deletions(-) create mode 100644 src/main/java/org/cryptomator/cryptofs/ch/MustWriteHeader.java create mode 100644 src/main/java/org/cryptomator/cryptofs/fh/FileHeaderHolder.java delete mode 100644 src/main/java/org/cryptomator/cryptofs/fh/FileHeaderLoader.java rename src/test/java/org/cryptomator/cryptofs/fh/{FileHeaderLoaderTest.java => FileHeaderHolderTest.java} (86%) diff --git a/src/main/java/org/cryptomator/cryptofs/ch/ChannelComponent.java b/src/main/java/org/cryptomator/cryptofs/ch/ChannelComponent.java index a6db717f..a3f419da 100644 --- a/src/main/java/org/cryptomator/cryptofs/ch/ChannelComponent.java +++ b/src/main/java/org/cryptomator/cryptofs/ch/ChannelComponent.java @@ -3,6 +3,7 @@ import dagger.BindsInstance; import dagger.Subcomponent; import org.cryptomator.cryptofs.EffectiveOpenOptions; +import org.cryptomator.cryptolib.api.FileHeader; import java.nio.channels.FileChannel; @@ -24,6 +25,12 @@ interface Builder { @BindsInstance Builder ciphertextChannel(FileChannel ciphertextChannel); + @BindsInstance + Builder mustWriteHeader(@MustWriteHeader boolean mustWriteHeader); + + @BindsInstance + Builder fileHeader(FileHeader fileHeader); + ChannelComponent build(); } diff --git a/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java b/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java index aad291e5..6da94530 100644 --- a/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java +++ b/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java @@ -7,10 +7,10 @@ import org.cryptomator.cryptofs.fh.ChunkCache; import org.cryptomator.cryptofs.fh.ChunkData; import org.cryptomator.cryptofs.fh.ExceptionsDuringWrite; -import org.cryptomator.cryptofs.fh.FileHeaderLoader; import org.cryptomator.cryptofs.fh.OpenFileModifiedDate; import org.cryptomator.cryptofs.fh.OpenFileSize; import org.cryptomator.cryptolib.api.Cryptor; +import org.cryptomator.cryptolib.api.FileHeader; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -40,7 +40,7 @@ public class CleartextFileChannel extends AbstractFileChannel { private static final Logger LOG = LoggerFactory.getLogger(CleartextFileChannel.class); private final FileChannel ciphertextFileChannel; - private final FileHeaderLoader fileHeaderLoader; + private final FileHeader fileHeader; private final Cryptor cryptor; private final ChunkCache chunkCache; private final EffectiveOpenOptions options; @@ -50,13 +50,13 @@ public class CleartextFileChannel extends AbstractFileChannel { private final ExceptionsDuringWrite exceptionsDuringWrite; private final ChannelCloseListener closeListener; private final CryptoFileSystemStats stats; - private boolean headerWritten; + private boolean mustWriteHeader; @Inject - public CleartextFileChannel(FileChannel ciphertextFileChannel, FileHeaderLoader fileHeaderLoader, 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, EffectiveOpenOptions options, @OpenFileSize AtomicLong fileSize, @OpenFileModifiedDate AtomicReference lastModified, Supplier attrViewProvider, ExceptionsDuringWrite exceptionsDuringWrite, ChannelCloseListener closeListener, CryptoFileSystemStats stats) { super(readWriteLock); this.ciphertextFileChannel = ciphertextFileChannel; - this.fileHeaderLoader = fileHeaderLoader; + this.fileHeader = fileHeader; this.cryptor = cryptor; this.chunkCache = chunkCache; this.options = options; @@ -69,7 +69,7 @@ public CleartextFileChannel(FileChannel ciphertextFileChannel, FileHeaderLoader if (options.append()) { position = fileSize.get(); } - this.headerWritten = !options.writable(); + this.mustWriteHeader = mustWriteHeader; if (options.createNew() || options.create()) { lastModified.compareAndSet(Instant.EPOCH, Instant.now()); } @@ -174,10 +174,11 @@ private long writeLockedInternal(ByteSource src, long position) throws IOExcepti } private void writeHeaderIfNeeded() throws IOException { - if (!headerWritten) { + if (mustWriteHeader) { LOG.trace("{} - Writing file header.", this); - ciphertextFileChannel.write(cryptor.fileHeaderCryptor().encryptHeader(fileHeaderLoader.get()), 0); - headerWritten = true; + ByteBuffer encryptedHeader = cryptor.fileHeaderCryptor().encryptHeader(fileHeader); + ciphertextFileChannel.write(encryptedHeader, 0); + mustWriteHeader = false; // write the header only once! } } diff --git a/src/main/java/org/cryptomator/cryptofs/ch/MustWriteHeader.java b/src/main/java/org/cryptomator/cryptofs/ch/MustWriteHeader.java new file mode 100644 index 00000000..9df5388a --- /dev/null +++ b/src/main/java/org/cryptomator/cryptofs/ch/MustWriteHeader.java @@ -0,0 +1,13 @@ +package org.cryptomator.cryptofs.ch; + +import javax.inject.Qualifier; +import java.lang.annotation.Documented; +import java.lang.annotation.Retention; + +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +@Qualifier +@Documented +@Retention(RUNTIME) +@interface MustWriteHeader { +} diff --git a/src/main/java/org/cryptomator/cryptofs/fh/ChunkIO.java b/src/main/java/org/cryptomator/cryptofs/fh/ChunkIO.java index 9c0f2c79..8ec707f8 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/ChunkIO.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/ChunkIO.java @@ -43,10 +43,6 @@ public void unregisterChannel(FileChannel channel) { writableChannels.remove(channel); } - long size() throws IOException { - return getReadableChannel().size(); - } - int read(ByteBuffer dst, long position) throws IOException { return getReadableChannel().read(dst, position); } diff --git a/src/main/java/org/cryptomator/cryptofs/fh/ChunkLoader.java b/src/main/java/org/cryptomator/cryptofs/fh/ChunkLoader.java index 9f9c1cda..fb7e9157 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/ChunkLoader.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/ChunkLoader.java @@ -12,14 +12,14 @@ class ChunkLoader { private final Cryptor cryptor; private final ChunkIO ciphertext; - private final FileHeaderLoader headerLoader; + private final FileHeaderHolder headerHolder; private final CryptoFileSystemStats stats; @Inject - public ChunkLoader(Cryptor cryptor, ChunkIO ciphertext, FileHeaderLoader headerLoader, CryptoFileSystemStats stats) { + public ChunkLoader(Cryptor cryptor, ChunkIO ciphertext, FileHeaderHolder headerHolder, CryptoFileSystemStats stats) { this.cryptor = cryptor; this.ciphertext = ciphertext; - this.headerLoader = headerLoader; + this.headerHolder = headerHolder; this.stats = stats; } @@ -35,7 +35,7 @@ public ChunkData load(Long chunkIndex) throws IOException { return ChunkData.emptyWithSize(payloadSize); } else { ciphertextBuf.flip(); - ByteBuffer cleartextBuf = cryptor.fileContentCryptor().decryptChunk(ciphertextBuf, chunkIndex, headerLoader.get(), true); + ByteBuffer cleartextBuf = cryptor.fileContentCryptor().decryptChunk(ciphertextBuf, chunkIndex, headerHolder.get(), true); stats.addBytesDecrypted(cleartextBuf.remaining()); ByteBuffer cleartextBufWhichCanHoldFullChunk; if (cleartextBuf.capacity() < payloadSize) { diff --git a/src/main/java/org/cryptomator/cryptofs/fh/ChunkSaver.java b/src/main/java/org/cryptomator/cryptofs/fh/ChunkSaver.java index 757be336..88bbf819 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/ChunkSaver.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/ChunkSaver.java @@ -12,15 +12,15 @@ class ChunkSaver { private final Cryptor cryptor; private final ChunkIO ciphertext; - private final FileHeaderLoader headerLoader; + private final FileHeaderHolder headerHolder; private final ExceptionsDuringWrite exceptionsDuringWrite; private final CryptoFileSystemStats stats; @Inject - public ChunkSaver(Cryptor cryptor, ChunkIO ciphertext, FileHeaderLoader headerLoader, ExceptionsDuringWrite exceptionsDuringWrite, CryptoFileSystemStats stats) { + public ChunkSaver(Cryptor cryptor, ChunkIO ciphertext, FileHeaderHolder headerHolder, ExceptionsDuringWrite exceptionsDuringWrite, CryptoFileSystemStats stats) { this.cryptor = cryptor; this.ciphertext = ciphertext; - this.headerLoader = headerLoader; + this.headerHolder = headerHolder; this.exceptionsDuringWrite = exceptionsDuringWrite; this.stats = stats; } @@ -30,7 +30,7 @@ public void save(long chunkIndex, ChunkData chunkData) throws IOException { long ciphertextPos = chunkIndex * cryptor.fileContentCryptor().ciphertextChunkSize() + cryptor.fileHeaderCryptor().headerSize(); ByteBuffer cleartextBuf = chunkData.asReadOnlyBuffer(); stats.addBytesEncrypted(cleartextBuf.remaining()); - ByteBuffer ciphertextBuf = cryptor.fileContentCryptor().encryptChunk(cleartextBuf, chunkIndex, headerLoader.get()); + ByteBuffer ciphertextBuf = cryptor.fileContentCryptor().encryptChunk(cleartextBuf, chunkIndex, headerHolder.get()); try { ciphertext.write(ciphertextBuf, ciphertextPos); } catch (IOException e) { diff --git a/src/main/java/org/cryptomator/cryptofs/fh/FileHeaderHolder.java b/src/main/java/org/cryptomator/cryptofs/fh/FileHeaderHolder.java new file mode 100644 index 00000000..8849392d --- /dev/null +++ b/src/main/java/org/cryptomator/cryptofs/fh/FileHeaderHolder.java @@ -0,0 +1,64 @@ +package org.cryptomator.cryptofs.fh; + +import org.cryptomator.cryptolib.api.CryptoException; +import org.cryptomator.cryptolib.api.Cryptor; +import org.cryptomator.cryptolib.api.FileHeader; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.inject.Inject; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.channels.FileChannel; +import java.nio.file.Path; +import java.util.concurrent.atomic.AtomicReference; + +@OpenFileScoped +public class FileHeaderHolder { + + private static final Logger LOG = LoggerFactory.getLogger(FileHeaderHolder.class); + + private final Cryptor cryptor; + private final AtomicReference path; + private final AtomicReference header = new AtomicReference<>(); + + @Inject + public FileHeaderHolder(Cryptor cryptor, @CurrentOpenFilePath AtomicReference path) { + this.cryptor = cryptor; + this.path = path; + } + + public FileHeader get() { + FileHeader result = header.get(); + if (result == null) { + throw new IllegalStateException("Header not set."); + } + return result; + } + + public FileHeader createNew() { + LOG.trace("Generating file header for {}", path.get()); + FileHeader newHeader = cryptor.fileHeaderCryptor().create(); + if (header.compareAndSet(null, newHeader)) { + return newHeader; + } else { + return header.get(); + } + } + + public FileHeader loadExisting(FileChannel ch) throws IOException { + LOG.trace("Reading file header from {}", path.get()); + ByteBuffer existingHeaderBuf = ByteBuffer.allocate(cryptor.fileHeaderCryptor().headerSize()); + int read = ch.read(existingHeaderBuf, 0); + assert read == existingHeaderBuf.capacity(); + existingHeaderBuf.flip(); + try { + FileHeader existingHeader = cryptor.fileHeaderCryptor().decryptHeader(existingHeaderBuf); + header.set(existingHeader); + return existingHeader; + } catch (IllegalArgumentException | CryptoException e) { + throw new IOException("Unable to decrypt header of file " + path.get(), e); + } + } + +} diff --git a/src/main/java/org/cryptomator/cryptofs/fh/FileHeaderLoader.java b/src/main/java/org/cryptomator/cryptofs/fh/FileHeaderLoader.java deleted file mode 100644 index 9ea87eb8..00000000 --- a/src/main/java/org/cryptomator/cryptofs/fh/FileHeaderLoader.java +++ /dev/null @@ -1,63 +0,0 @@ -package org.cryptomator.cryptofs.fh; - -import org.cryptomator.cryptolib.api.CryptoException; -import org.cryptomator.cryptolib.api.Cryptor; -import org.cryptomator.cryptolib.api.FileHeader; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import javax.inject.Inject; -import java.io.IOException; -import java.io.UncheckedIOException; -import java.nio.ByteBuffer; -import java.nio.file.Path; -import java.util.concurrent.atomic.AtomicReference; - -@OpenFileScoped -public class FileHeaderLoader { - - private static final Logger LOG = LoggerFactory.getLogger(FileHeaderLoader.class); - - private final ChunkIO ciphertext; - private final Cryptor cryptor; - private final AtomicReference path; - private final AtomicReference header = new AtomicReference<>(); - - @Inject - public FileHeaderLoader(ChunkIO ciphertext, Cryptor cryptor, @CurrentOpenFilePath AtomicReference path) { - this.ciphertext = ciphertext; - this.cryptor = cryptor; - this.path = path; - } - - public FileHeader get() throws IOException { - try { - return header.updateAndGet(cached -> cached == null ? load() : cached); - } catch (UncheckedIOException e) { - throw new IOException(e); - } - } - - private FileHeader load() throws UncheckedIOException { - try { - if (ciphertext.size() == 0) { // i.e. TRUNCATE_EXISTING, CREATE OR CREATE_NEW - LOG.trace("Generating file header for {}", path.get()); - return cryptor.fileHeaderCryptor().create(); - } else { - LOG.trace("Reading file header from {}", path.get()); - ByteBuffer existingHeaderBuf = ByteBuffer.allocate(cryptor.fileHeaderCryptor().headerSize()); - int read = ciphertext.read(existingHeaderBuf, 0); - assert read == existingHeaderBuf.capacity(); - existingHeaderBuf.flip(); - try { - return cryptor.fileHeaderCryptor().decryptHeader(existingHeaderBuf); - } catch (IllegalArgumentException | CryptoException e) { - throw new IOException("Unable to decrypt header of file " + path.get(), e); - } - } - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } - -} diff --git a/src/main/java/org/cryptomator/cryptofs/fh/OpenCryptoFile.java b/src/main/java/org/cryptomator/cryptofs/fh/OpenCryptoFile.java index 6d79f199..fb54964e 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/OpenCryptoFile.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/OpenCryptoFile.java @@ -9,12 +9,12 @@ package org.cryptomator.cryptofs.fh; import com.google.common.base.Preconditions; -import com.google.common.io.MoreFiles; import org.cryptomator.cryptofs.EffectiveOpenOptions; import org.cryptomator.cryptofs.ch.ChannelComponent; import org.cryptomator.cryptofs.ch.CleartextFileChannel; import org.cryptomator.cryptolib.Cryptors; import org.cryptomator.cryptolib.api.Cryptor; +import org.cryptomator.cryptolib.api.FileHeader; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -39,6 +39,7 @@ public class OpenCryptoFile implements Closeable { private final AtomicReference lastModified; private final ChunkCache chunkCache; private final Cryptor cryptor; + private final FileHeaderHolder headerHolder; private final ChunkIO chunkIO; private final AtomicReference currentFilePath; private final AtomicLong fileSize; @@ -46,10 +47,11 @@ public class OpenCryptoFile implements Closeable { private final ConcurrentMap openChannels = new ConcurrentHashMap<>(); @Inject - public OpenCryptoFile(FileCloseListener listener, ChunkCache chunkCache, Cryptor cryptor, ChunkIO chunkIO, @CurrentOpenFilePath AtomicReference currentFilePath, @OpenFileSize AtomicLong fileSize, @OpenFileModifiedDate AtomicReference lastModified, OpenCryptoFileComponent component) { + public OpenCryptoFile(FileCloseListener listener, ChunkCache chunkCache, Cryptor cryptor, FileHeaderHolder headerHolder, ChunkIO chunkIO, @CurrentOpenFilePath AtomicReference currentFilePath, @OpenFileSize AtomicLong fileSize, @OpenFileModifiedDate AtomicReference lastModified, OpenCryptoFileComponent component) { this.listener = listener; this.chunkCache = chunkCache; this.cryptor = cryptor; + this.headerHolder = headerHolder; this.chunkIO = chunkIO; this.currentFilePath = currentFilePath; this.fileSize = fileSize; @@ -75,11 +77,22 @@ public synchronized FileChannel newFileChannel(EffectiveOpenOptions options) thr CleartextFileChannel cleartextFileChannel = null; try { ciphertextFileChannel = path.getFileSystem().provider().newFileChannel(path, options.createOpenOptionsForEncryptedFile()); + final FileHeader header; + final boolean isNewHeader; + if (ciphertextFileChannel.size() == 0l) { + header = headerHolder.createNew(); + isNewHeader = true; + } else { + header = headerHolder.loadExisting(ciphertextFileChannel); + isNewHeader = false; + } initFileSize(ciphertextFileChannel); ChannelComponent channelComponent = component.newChannelComponent() // .ciphertextChannel(ciphertextFileChannel) // .openOptions(options) // .onClose(this::channelClosed) // + .mustWriteHeader(isNewHeader) // + .fileHeader(header) // .build(); cleartextFileChannel = channelComponent.channel(); } finally { diff --git a/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java b/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java index 3a42aa2b..18c5e8bc 100644 --- a/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java +++ b/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java @@ -5,9 +5,9 @@ import org.cryptomator.cryptofs.fh.ChunkCache; import org.cryptomator.cryptofs.fh.ChunkData; import org.cryptomator.cryptofs.fh.ExceptionsDuringWrite; -import org.cryptomator.cryptofs.fh.FileHeaderLoader; import org.cryptomator.cryptolib.api.Cryptor; import org.cryptomator.cryptolib.api.FileContentCryptor; +import org.cryptomator.cryptolib.api.FileHeader; import org.cryptomator.cryptolib.api.FileHeaderCryptor; import org.hamcrest.MatcherAssert; import org.junit.jupiter.api.Assertions; @@ -55,7 +55,8 @@ public class CleartextFileChannelTest { private FileHeaderCryptor fileHeaderCryptor = mock(FileHeaderCryptor.class); private FileContentCryptor fileContentCryptor = mock(FileContentCryptor.class); private FileChannel ciphertextFileChannel = mock(FileChannel.class); - private FileHeaderLoader headerLoader = mock(FileHeaderLoader.class); + private FileHeader header = mock(FileHeader.class); + private boolean mustWriteHeader = true; private EffectiveOpenOptions options = mock(EffectiveOpenOptions.class); private AtomicLong fileSize = new AtomicLong(100); private AtomicReference lastModified = new AtomicReference(Instant.ofEpochMilli(0)); @@ -79,7 +80,7 @@ public void setUp() throws IOException { when(readWriteLock.readLock()).thenReturn(readLock); when(readWriteLock.writeLock()).thenReturn(writeLock); - inTest = new CleartextFileChannel(ciphertextFileChannel, headerLoader, readWriteLock, cryptor, chunkCache, options, fileSize, lastModified, attributeViewSupplier, exceptionsDuringWrite, closeListener, stats); + inTest = new CleartextFileChannel(ciphertextFileChannel, header, mustWriteHeader, readWriteLock, cryptor, chunkCache, options, fileSize, lastModified, attributeViewSupplier, exceptionsDuringWrite, closeListener, stats); } @Test @@ -303,7 +304,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, headerLoader, readWriteLock, cryptor, chunkCache, options, fileSize, lastModified, attributeViewSupplier, exceptionsDuringWrite, closeListener, stats); + inTest = new CleartextFileChannel(ciphertextFileChannel, header, mustWriteHeader, readWriteLock, cryptor, chunkCache, options, fileSize, lastModified, attributeViewSupplier, exceptionsDuringWrite, closeListener, stats); ByteBuffer buf = ByteBuffer.allocate(10); // A read from frist chunk: diff --git a/src/test/java/org/cryptomator/cryptofs/fh/ChunkLoaderTest.java b/src/test/java/org/cryptomator/cryptofs/fh/ChunkLoaderTest.java index 1f2b802b..498e1b24 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/ChunkLoaderTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/ChunkLoaderTest.java @@ -35,14 +35,14 @@ public class ChunkLoaderTest { private final Cryptor cryptor = mock(Cryptor.class); private final CryptoFileSystemStats stats = mock(CryptoFileSystemStats.class); private final FileHeader header = mock(FileHeader.class); - private final FileHeaderLoader headerLoader = mock(FileHeaderLoader.class); - private final ChunkLoader inTest = new ChunkLoader(cryptor, chunkIO, headerLoader, stats); + private final FileHeaderHolder headerHolder = mock(FileHeaderHolder.class); + private final ChunkLoader inTest = new ChunkLoader(cryptor, chunkIO, headerHolder, stats); @BeforeEach public void setup() throws IOException { when(cryptor.fileContentCryptor()).thenReturn(fileContentCryptor); when(cryptor.fileHeaderCryptor()).thenReturn(fileHeaderCryptor); - when(headerLoader.get()).thenReturn(header); + when(headerHolder.get()).thenReturn(header); when(fileContentCryptor.ciphertextChunkSize()).thenReturn(CIPHERTEXT_CHUNK_SIZE); when(fileContentCryptor.cleartextChunkSize()).thenReturn(CLEARTEXT_CHUNK_SIZE); when(fileHeaderCryptor.headerSize()).thenReturn(HEADER_SIZE); diff --git a/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java b/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java index d9330550..3b8b2830 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java @@ -34,15 +34,15 @@ public class ChunkSaverTest { private final Cryptor cryptor = mock(Cryptor.class); private final CryptoFileSystemStats stats = mock(CryptoFileSystemStats.class); private final FileHeader header = mock(FileHeader.class); - private final FileHeaderLoader headerLoader = mock(FileHeaderLoader.class); + private final FileHeaderHolder headerHolder = mock(FileHeaderHolder.class); private final ExceptionsDuringWrite exceptionsDuringWrite = mock(ExceptionsDuringWrite.class); - private final ChunkSaver inTest = new ChunkSaver(cryptor, chunkIO, headerLoader, exceptionsDuringWrite, stats); + private final ChunkSaver inTest = new ChunkSaver(cryptor, chunkIO, headerHolder, exceptionsDuringWrite, stats); @BeforeEach public void setup() throws IOException { when(cryptor.fileContentCryptor()).thenReturn(fileContentCryptor); when(cryptor.fileHeaderCryptor()).thenReturn(fileHeaderCryptor); - when(headerLoader.get()).thenReturn(header); + when(headerHolder.get()).thenReturn(header); when(fileContentCryptor.ciphertextChunkSize()).thenReturn(CIPHERTEXT_CHUNK_SIZE); when(fileContentCryptor.cleartextChunkSize()).thenReturn(CLEARTEXT_CHUNK_SIZE); when(fileHeaderCryptor.headerSize()).thenReturn(HEADER_SIZE); diff --git a/src/test/java/org/cryptomator/cryptofs/fh/FileHeaderLoaderTest.java b/src/test/java/org/cryptomator/cryptofs/fh/FileHeaderHolderTest.java similarity index 86% rename from src/test/java/org/cryptomator/cryptofs/fh/FileHeaderLoaderTest.java rename to src/test/java/org/cryptomator/cryptofs/fh/FileHeaderHolderTest.java index c8f82c4f..1f22043f 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/FileHeaderLoaderTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/FileHeaderHolderTest.java @@ -13,6 +13,7 @@ import java.io.IOException; import java.nio.ByteBuffer; +import java.nio.channels.FileChannel; import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.util.concurrent.atomic.AtomicReference; @@ -22,21 +23,20 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -class FileHeaderLoaderTest { +class FileHeaderHolderTest { static { - System.setProperty("org.slf4j.simpleLogger.log.org.cryptomator.cryptofs.ch.FileHeaderLoader", "trace"); + System.setProperty("org.slf4j.simpleLogger.log.org.cryptomator.cryptofs.ch.FileHeaderHolder", "trace"); System.setProperty("org.slf4j.simpleLogger.showDateTime", "true"); System.setProperty("org.slf4j.simpleLogger.dateTimeFormat", "HH:mm:ss.SSS"); } private final FileHeaderCryptor fileHeaderCryptor = mock(FileHeaderCryptor.class); - private final ChunkIO chunkIO = mock(ChunkIO.class); private final Cryptor cryptor = mock(Cryptor.class); private final Path path = mock(Path.class, "openFile.txt"); private final AtomicReference pathRef = new AtomicReference<>(path); - private final FileHeaderLoader inTest = new FileHeaderLoader(chunkIO, cryptor, pathRef); + private final FileHeaderHolder inTest = new FileHeaderHolder(cryptor, pathRef); @BeforeEach public void setup() throws IOException { @@ -48,13 +48,13 @@ public void setup() throws IOException { class ExistingHeader { private FileHeader headerToLoad = Mockito.mock(FileHeader.class); + private FileChannel channel = Mockito.mock(FileChannel.class); @BeforeEach public void setup() throws IOException { byte[] headerBytes = "leHeader".getBytes(StandardCharsets.US_ASCII); - when(chunkIO.size()).thenReturn(100l); when(fileHeaderCryptor.headerSize()).thenReturn(headerBytes.length); - when(chunkIO.read(Mockito.any(ByteBuffer.class), Mockito.eq(0l))).thenAnswer(invocation -> { + when(channel.read(Mockito.any(ByteBuffer.class), Mockito.eq(0l))).thenAnswer(invocation -> { ByteBuffer buf = invocation.getArgument(0); Assertions.assertEquals(headerBytes.length, buf.capacity()); buf.put(headerBytes); @@ -66,7 +66,7 @@ public void setup() throws IOException { @Test @DisplayName("load") public void testLoadExisting() throws IOException { - FileHeader loadedHeader1 = inTest.get(); + FileHeader loadedHeader1 = inTest.loadExisting(channel); FileHeader loadedHeader2 = inTest.get(); FileHeader loadedHeader3 = inTest.get(); Assertions.assertSame(headerToLoad, loadedHeader1); @@ -86,7 +86,6 @@ class NewHeader { @BeforeEach public void setup() throws IOException { - when(chunkIO.size()).thenReturn(0l); when(fileHeaderCryptor.create()).thenReturn(headerToCreate); } @@ -97,8 +96,8 @@ public void tearDown() { @Test @DisplayName("create") - public void testCreateNew() throws IOException { - FileHeader createdHeader1 = inTest.get(); + public void testCreateNew() { + FileHeader createdHeader1 = inTest.createNew(); FileHeader createdHeader2 = inTest.get(); FileHeader createdHeader3 = inTest.get(); Assertions.assertSame(headerToCreate, createdHeader1); diff --git a/src/test/java/org/cryptomator/cryptofs/fh/OpenCryptoFileTest.java b/src/test/java/org/cryptomator/cryptofs/fh/OpenCryptoFileTest.java index 0550d096..2332b748 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/OpenCryptoFileTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/OpenCryptoFileTest.java @@ -7,13 +7,11 @@ import org.cryptomator.cryptofs.ch.ChannelComponent; import org.cryptomator.cryptofs.ch.CleartextFileChannel; import org.cryptomator.cryptolib.api.Cryptor; -import org.cryptomator.cryptolib.api.FileContentCryptor; -import org.cryptomator.cryptolib.api.FileHeaderCryptor; +import org.cryptomator.cryptolib.api.FileHeader; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.MethodOrderer; import org.junit.jupiter.api.Nested; @@ -37,7 +35,6 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; public class OpenCryptoFileTest { @@ -47,6 +44,8 @@ public class OpenCryptoFileTest { private FileCloseListener closeListener = mock(FileCloseListener.class); private ChunkCache chunkCache = mock(ChunkCache.class); private Cryptor cryptor = mock(Cryptor.class); + private FileHeaderHolder headerHolder = mock(FileHeaderHolder.class); + private FileHeader header = mock(FileHeader.class); private ChunkIO chunkIO = mock(ChunkIO.class); private AtomicLong fileSize = new AtomicLong(-1l); private AtomicReference lastModified = new AtomicReference(Instant.ofEpochMilli(0)); @@ -67,7 +66,7 @@ public static void tearDown() throws IOException { @Test public void testCloseTriggersCloseListener() { - OpenCryptoFile openCryptoFile = new OpenCryptoFile(closeListener, chunkCache, cryptor, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent); + OpenCryptoFile openCryptoFile = new OpenCryptoFile(closeListener, chunkCache, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent); openCryptoFile.close(); verify(closeListener).close(CURRENT_FILE_PATH.get(), openCryptoFile); } @@ -78,7 +77,7 @@ public void testCloseImmediatelyIfOpeningFirstChannelFails() { UncheckedIOException expectedException = new UncheckedIOException(new IOException("fail!")); EffectiveOpenOptions options = Mockito.mock(EffectiveOpenOptions.class); Mockito.when(options.createOpenOptionsForEncryptedFile()).thenThrow(expectedException); - OpenCryptoFile openCryptoFile = new OpenCryptoFile(closeListener, chunkCache, cryptor, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent); + OpenCryptoFile openCryptoFile = new OpenCryptoFile(closeListener, chunkCache, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent); UncheckedIOException exception = Assertions.assertThrows(UncheckedIOException.class, () -> { openCryptoFile.newFileChannel(options); @@ -100,7 +99,7 @@ class FileChannelFactoryTest { @BeforeAll public void setup() throws IOException { - openCryptoFile = new OpenCryptoFile(closeListener, chunkCache, cryptor, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent); + openCryptoFile = new OpenCryptoFile(closeListener, chunkCache, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent); cleartextFileChannel = mock(CleartextFileChannel.class); listener = new AtomicReference<>(); ciphertextChannel = new AtomicReference<>(); @@ -115,6 +114,8 @@ public void setup() throws IOException { listener.set(invocation.getArgument(0)); return channelComponentBuilder; }); + Mockito.when(channelComponentBuilder.fileHeader(Mockito.any())).thenReturn(channelComponentBuilder); + Mockito.when(channelComponentBuilder.mustWriteHeader(Mockito.anyBoolean())).thenReturn(channelComponentBuilder); Mockito.when(channelComponentBuilder.build()).thenReturn(channelComponent); Mockito.when(channelComponent.channel()).thenReturn(cleartextFileChannel); } From 591609311a4a2087d41ab7ef71741c4ce970309f Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 22 May 2019 15:53:15 +0200 Subject: [PATCH 4/5] added unit tests for #58 --- .../cryptofs/ch/CleartextFileChannelTest.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java b/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java index 18c5e8bc..7e7fc89a 100644 --- a/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java +++ b/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java @@ -442,6 +442,29 @@ public void writeAfterEof() throws IOException { Assertions.assertEquals(210l, inTest.size()); } + @Test + @DisplayName("write header if it isn't already written") + public void testWriteHeaderIfNeeded() throws IOException { + when(options.writable()).thenReturn(true); + + inTest.force(true); + inTest.force(true); + inTest.force(true); + + Mockito.verify(ciphertextFileChannel, Mockito.times(1)).write(Mockito.any(), Mockito.eq(0l)); + } + + @Test + @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.force(true); + + Mockito.verify(ciphertextFileChannel, Mockito.never()).write(Mockito.any(), Mockito.eq(0l)); + } + } @Nested From b1c723d8c7c4a2e139c28c30747909cdb20b7127 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 22 May 2019 16:00:42 +0200 Subject: [PATCH 5/5] Preparing 1.8.5 --- .gitignore | 4 ++++ pom.xml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 4568d7a7..4b5ce24a 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,10 @@ target/ test-output/ +# Maven # +target/ +pom.xml.versionsBackup + # IntelliJ Settings Files # .idea/ out/ diff --git a/pom.xml b/pom.xml index ab3e4011..f4965755 100644 --- a/pom.xml +++ b/pom.xml @@ -2,7 +2,7 @@ 4.0.0 org.cryptomator cryptofs - 1.9.0-SNAPSHOT + 1.8.5 Cryptomator Crypto Filesystem This library provides the Java filesystem provider used by Cryptomator. https://github.com/cryptomator/cryptofs