From fe3ac2827fbec844c500ac50f906b7cf8110a4ee Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Mon, 29 Nov 2021 16:48:05 +0100 Subject: [PATCH 01/14] impl idea --- .../cryptofs/CryptoFileSystemImpl.java | 17 +++++++----- .../cryptofs/DirectoryIdBackup.java | 26 +++++++++++++++++++ .../cryptofs/common/Constants.java | 1 + .../cryptofs/dir/DirectoryStreamFactory.java | 15 ++++++++++- .../cryptofs/CryptoFileSystemImplTest.java | 2 ++ 5 files changed, 54 insertions(+), 7 deletions(-) create mode 100644 src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java index 5da6de99..35052047 100644 --- a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java +++ b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java @@ -15,6 +15,7 @@ import org.cryptomator.cryptofs.attr.AttributeViewType; import org.cryptomator.cryptofs.common.ArrayUtils; import org.cryptomator.cryptofs.common.CiphertextFileType; +import org.cryptomator.cryptofs.common.Constants; import org.cryptomator.cryptofs.common.DeletingFileVisitor; import org.cryptomator.cryptofs.common.FinallyUtil; import org.cryptomator.cryptofs.dir.CiphertextDirectoryDeleter; @@ -68,7 +69,7 @@ @CryptoFileSystemScoped class CryptoFileSystemImpl extends CryptoFileSystem { - + private final CryptoFileSystemProvider provider; private final CryptoFileSystems cryptoFileSystems; private final Path pathToVault; @@ -80,6 +81,7 @@ class CryptoFileSystemImpl extends CryptoFileSystem { private final PathMatcherFactory pathMatcherFactory; private final DirectoryStreamFactory directoryStreamFactory; private final DirectoryIdProvider dirIdProvider; + private final DirectoryIdBackup dirIdBackup; private final AttributeProvider fileAttributeProvider; private final AttributeByNameProvider fileAttributeByNameProvider; private final AttributeViewProvider fileAttributeViewProvider; @@ -98,7 +100,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, - PathMatcherFactory pathMatcherFactory, DirectoryStreamFactory directoryStreamFactory, DirectoryIdProvider dirIdProvider, + PathMatcherFactory pathMatcherFactory, DirectoryStreamFactory directoryStreamFactory, DirectoryIdProvider dirIdProvider, DirectoryIdBackup dirIdBackup, AttributeProvider fileAttributeProvider, AttributeByNameProvider fileAttributeByNameProvider, AttributeViewProvider fileAttributeViewProvider, OpenCryptoFiles openCryptoFiles, Symlinks symlinks, FinallyUtil finallyUtil, CiphertextDirectoryDeleter ciphertextDirDeleter, ReadonlyFlag readonlyFlag, CryptoFileSystemProperties fileSystemProperties) { @@ -113,6 +115,7 @@ public CryptoFileSystemImpl(CryptoFileSystemProvider provider, CryptoFileSystems this.pathMatcherFactory = pathMatcherFactory; this.directoryStreamFactory = directoryStreamFactory; this.dirIdProvider = dirIdProvider; + this.dirIdBackup = dirIdBackup; this.fileAttributeProvider = fileAttributeProvider; this.fileAttributeByNameProvider = fileAttributeByNameProvider; this.fileAttributeViewProvider = fileAttributeViewProvider; @@ -235,8 +238,8 @@ A readAttributes(CryptoPath cleartextPath, Class /** * @param cleartextPath the path to the file - * @param type the Class object corresponding to the file attribute view - * @param options future use + * @param type the Class object corresponding to the file attribute view + * @param options future use * @return a file attribute view of the specified type, or null if the attribute view type is not available * @see AttributeViewProvider#getAttributeView(CryptoPath, Class, LinkOption...) */ @@ -302,6 +305,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); + dirIdBackup.execute(ciphertextDir); ciphertextPath.persistLongFileName(); } catch (IOException e) { // make sure there is no orphan dir file: @@ -312,8 +316,9 @@ void createDirectory(CryptoPath cleartextDir, FileAttribute... attrs) throws } } + //TODO: where should the filter decorator be placed? Here or DirectoryStreamFactory? DirectoryStream newDirectoryStream(CryptoPath cleartextDir, Filter filter) throws IOException { - return directoryStreamFactory.newDirectoryStream(cleartextDir, filter); + return directoryStreamFactory.newDirectoryStream(cleartextDir, entry -> !entry.getFileName().equals(Constants.DIR_ID_FILE) && filter.accept(entry)); } FileChannel newFileChannel(CryptoPath cleartextPath, Set optionsSet, FileAttribute... attrs) throws IOException { @@ -574,7 +579,7 @@ private void moveDirectory(CryptoPath cleartextSource, CryptoPath cleartextTarge } Files.walkFileTree(ciphertextTarget.getRawPath(), DeletingFileVisitor.INSTANCE); } - + // no exceptions until this point, so MOVE: Files.move(ciphertextSource.getRawPath(), ciphertextTarget.getRawPath(), options); if (ciphertextTarget.isShortened()) { diff --git a/src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java b/src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java new file mode 100644 index 00000000..ba4b5407 --- /dev/null +++ b/src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java @@ -0,0 +1,26 @@ +package org.cryptomator.cryptofs; + +import org.cryptomator.cryptofs.common.Constants; +import org.cryptomator.cryptolib.api.Cryptor; +import org.cryptomator.cryptolib.common.EncryptingWritableByteChannel; + +import javax.inject.Inject; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.StandardOpenOption; + +@CryptoFileSystemScoped +public class DirectoryIdBackup { + + @Inject + Cryptor cryptor; + + public void execute(CryptoPathMapper.CiphertextDirectory ciphertextDirectory) throws IOException { + try (var channel = Files.newByteChannel(ciphertextDirectory.path.resolve(Constants.DIR_ID_FILE), StandardOpenOption.CREATE_NEW); // + var encryptingChannel = new EncryptingWritableByteChannel(channel, cryptor);) { + encryptingChannel.write(ByteBuffer.wrap(ciphertextDirectory.dirId.getBytes(StandardCharsets.UTF_8))); + } + } +} diff --git a/src/main/java/org/cryptomator/cryptofs/common/Constants.java b/src/main/java/org/cryptomator/cryptofs/common/Constants.java index dd95f02c..c36b95ca 100644 --- a/src/main/java/org/cryptomator/cryptofs/common/Constants.java +++ b/src/main/java/org/cryptomator/cryptofs/common/Constants.java @@ -32,4 +32,5 @@ private Constants() { public static final String SEPARATOR = "/"; public static final String RECOVERY_DIR_NAME = "CRYPTOMATOR_RECOVERY"; + public static final String DIR_ID_FILE = "id.c9r"; } diff --git a/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java b/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java index d16bbeb8..709e3418 100644 --- a/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java +++ b/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java @@ -4,6 +4,9 @@ import org.cryptomator.cryptofs.CryptoPath; import org.cryptomator.cryptofs.CryptoPathMapper; import org.cryptomator.cryptofs.CryptoPathMapper.CiphertextDirectory; +import org.cryptomator.cryptofs.DirectoryIdBackup; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import javax.inject.Inject; import java.io.IOException; @@ -19,16 +22,20 @@ @CryptoFileSystemScoped public class DirectoryStreamFactory { + private static final Logger LOG = LoggerFactory.getLogger(DirectoryStreamFactory.class); + private final CryptoPathMapper cryptoPathMapper; private final DirectoryStreamComponent.Builder directoryStreamComponentBuilder; // sharing reusable builder via synchronized + private final DirectoryIdBackup dirIdBackup; private final Map streams = new HashMap<>(); private volatile boolean closed = false; @Inject - public DirectoryStreamFactory(CryptoPathMapper cryptoPathMapper, DirectoryStreamComponent.Builder directoryStreamComponentBuilder) { + public DirectoryStreamFactory(CryptoPathMapper cryptoPathMapper, DirectoryStreamComponent.Builder directoryStreamComponentBuilder, DirectoryIdBackup dirIdBackup) { this.cryptoPathMapper = cryptoPathMapper; this.directoryStreamComponentBuilder = directoryStreamComponentBuilder; + this.dirIdBackup = dirIdBackup; } public synchronized CryptoDirectoryStream newDirectoryStream(CryptoPath cleartextDir, Filter filter) throws IOException { @@ -36,6 +43,12 @@ public synchronized CryptoDirectoryStream newDirectoryStream(CryptoPath cleartex throw new ClosedFileSystemException(); } CiphertextDirectory ciphertextDir = cryptoPathMapper.getCiphertextDir(cleartextDir); + //TODO: should we really do this implicit backup? + try{ + dirIdBackup.execute(ciphertextDir); + } catch (IOException e) { + LOG.info("Cannot create dirId backup of {}. Probably read only."); + } DirectoryStream ciphertextDirStream = Files.newDirectoryStream(ciphertextDir.path); CryptoDirectoryStream cleartextDirStream = directoryStreamComponentBuilder // .dirId(ciphertextDir.dirId) // diff --git a/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java b/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java index cc75fdea..9749e6be 100644 --- a/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java +++ b/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java @@ -1095,6 +1095,8 @@ public void createDirectoryClearsDirIdAndDeletesDirFileIfCreatingDirFails() thro verify(cryptoPathMapper).invalidatePathMapping(path); } + //TODO: write tests for backupDirId + } @Nested From d31e32c256ca44768689281e72fcaded5ad15995 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 26 Jan 2022 18:34:38 +0100 Subject: [PATCH 02/14] fix unit tests and document backup class --- .../cryptofs/DirectoryIdBackup.java | 29 +++++++++++++++++-- .../cryptofs/CryptoFileSystemImplTest.java | 3 +- ...ptyCiphertextDirectoryIntegrationTest.java | 2 +- .../dir/DirectoryStreamFactoryTest.java | 4 ++- 4 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java b/src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java index ba4b5407..63c4e868 100644 --- a/src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java +++ b/src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java @@ -7,20 +7,43 @@ import javax.inject.Inject; import java.io.IOException; import java.nio.ByteBuffer; +import java.nio.channels.ByteChannel; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.StandardOpenOption; +import java.util.function.BiFunction; +import java.util.function.Function; +/** + * Single purpose class to backup the directory id of an encrypted directory when it is created. + */ @CryptoFileSystemScoped public class DirectoryIdBackup { + //visible and existing for testing + private Cryptor cryptor; + @Inject - Cryptor cryptor; + public DirectoryIdBackup(Cryptor cryptor) { + this.cryptor = cryptor; + } + /** + * Performs the backup operation for the given {@link CryptoPathMapper.CiphertextDirectory} object. + *

+ * The directory id is written via an encrypting channel to the file {@link CryptoPathMapper.CiphertextDirectory#path}/{@value Constants#DIR_ID_FILE}. + * + * @param ciphertextDirectory The cipher dir object containing the dir id and the encrypted content root + * @throws IOException if an IOException is raised during the write operation + */ public void execute(CryptoPathMapper.CiphertextDirectory ciphertextDirectory) throws IOException { - try (var channel = Files.newByteChannel(ciphertextDirectory.path.resolve(Constants.DIR_ID_FILE), StandardOpenOption.CREATE_NEW); // - var encryptingChannel = new EncryptingWritableByteChannel(channel, cryptor);) { + try (var channel = Files.newByteChannel(ciphertextDirectory.path.resolve(Constants.DIR_ID_FILE), StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE); // + var encryptingChannel = wrapEncryptionAround(channel,cryptor)) { encryptingChannel.write(ByteBuffer.wrap(ciphertextDirectory.dirId.getBytes(StandardCharsets.UTF_8))); } } + + static EncryptingWritableByteChannel wrapEncryptionAround(ByteChannel channel, Cryptor cryptor) { + return new EncryptingWritableByteChannel(channel, cryptor); + } } diff --git a/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java b/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java index 9749e6be..d01eeff6 100644 --- a/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java +++ b/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java @@ -89,6 +89,7 @@ public class CryptoFileSystemImplTest { private final Symlinks symlinks = mock(Symlinks.class); private final CryptoPathMapper cryptoPathMapper = mock(CryptoPathMapper.class); private final DirectoryIdProvider dirIdProvider = mock(DirectoryIdProvider.class); + private final DirectoryIdBackup dirIdBackup = mock(DirectoryIdBackup.class); private final AttributeProvider fileAttributeProvider = mock(AttributeProvider.class); private final AttributeByNameProvider fileAttributeByNameProvider = mock(AttributeByNameProvider.class); private final AttributeViewProvider fileAttributeViewProvider = mock(AttributeViewProvider.class); @@ -119,7 +120,7 @@ public void setup() { inTest = new CryptoFileSystemImpl(provider, cryptoFileSystems, pathToVault, cryptor, fileStore, stats, cryptoPathMapper, cryptoPathFactory, - pathMatcherFactory, directoryStreamFactory, dirIdProvider, + pathMatcherFactory, directoryStreamFactory, dirIdProvider, dirIdBackup, fileAttributeProvider, fileAttributeByNameProvider, fileAttributeViewProvider, openCryptoFiles, symlinks, finallyUtil, ciphertextDirDeleter, readonlyFlag, fileSystemProperties); diff --git a/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java b/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java index a52f9c15..9fa1e308 100644 --- a/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java +++ b/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java @@ -168,7 +168,7 @@ private Path firstEmptyCiphertextDirectory() throws IOException { private boolean isEmptyDirectory(Path path) { try (Stream files = Files.list(path)) { - return files.count() == 0; + return files.filter(p -> p.getFileName().toString().equals(Constants.DIR_ID_FILE)).count() == 0; } catch (IOException e) { throw new UncheckedIOException(e); } diff --git a/src/test/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactoryTest.java b/src/test/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactoryTest.java index 565f4947..f0d2429e 100644 --- a/src/test/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactoryTest.java +++ b/src/test/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactoryTest.java @@ -3,6 +3,7 @@ import org.cryptomator.cryptofs.CryptoPath; import org.cryptomator.cryptofs.CryptoPathMapper; import org.cryptomator.cryptofs.CryptoPathMapper.CiphertextDirectory; +import org.cryptomator.cryptofs.DirectoryIdBackup; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -30,8 +31,9 @@ public class DirectoryStreamFactoryTest { private final CryptoPathMapper cryptoPathMapper = mock(CryptoPathMapper.class); private final DirectoryStreamComponent directoryStreamComp = mock(DirectoryStreamComponent.class); private final DirectoryStreamComponent.Builder directoryStreamBuilder = mock(DirectoryStreamComponent.Builder.class); + private final DirectoryIdBackup directoryIdBackup = mock(DirectoryIdBackup.class); - private final DirectoryStreamFactory inTest = new DirectoryStreamFactory(cryptoPathMapper, directoryStreamBuilder); + private final DirectoryStreamFactory inTest = new DirectoryStreamFactory(cryptoPathMapper, directoryStreamBuilder, directoryIdBackup); @SuppressWarnings("unchecked") From c86e5f1490213a8755a0d7d6200d12b7a815774a Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 26 Jan 2022 19:39:56 +0100 Subject: [PATCH 03/14] add unit test for dirIdBackup class --- pom.xml | 6 ++ .../cryptofs/DirectoryIdBackupTest.java | 70 +++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 src/test/java/org/cryptomator/cryptofs/DirectoryIdBackupTest.java diff --git a/pom.xml b/pom.xml index 92f9eb0d..e1ce0643 100644 --- a/pom.xml +++ b/pom.xml @@ -100,6 +100,12 @@ ${mockito.version} test + + org.mockito + mockito-junit-jupiter + ${mockito.version} + test + org.hamcrest hamcrest diff --git a/src/test/java/org/cryptomator/cryptofs/DirectoryIdBackupTest.java b/src/test/java/org/cryptomator/cryptofs/DirectoryIdBackupTest.java new file mode 100644 index 00000000..51bd12a7 --- /dev/null +++ b/src/test/java/org/cryptomator/cryptofs/DirectoryIdBackupTest.java @@ -0,0 +1,70 @@ +package org.cryptomator.cryptofs; + +import org.cryptomator.cryptofs.common.Constants; +import org.cryptomator.cryptolib.api.Cryptor; +import org.cryptomator.cryptolib.common.EncryptingWritableByteChannel; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; + +@ExtendWith(MockitoExtension.class) +public class DirectoryIdBackupTest { + + @TempDir + Path contentPath; + + private String dirId = "12345678"; + private CryptoPathMapper.CiphertextDirectory cipherDirObject; + @Mock + private EncryptingWritableByteChannel encChannel; + @Mock + private Cryptor cryptor; + + private DirectoryIdBackup dirIdBackup; + + + @BeforeEach + public void init() { + cipherDirObject = new CryptoPathMapper.CiphertextDirectory(dirId, contentPath); + dirIdBackup = new DirectoryIdBackup(cryptor); + } + + @Test + public void testIdFileCreated() throws IOException { + try (MockedStatic backupMock = Mockito.mockStatic(DirectoryIdBackup.class)) { + backupMock.when(() -> DirectoryIdBackup.wrapEncryptionAround(Mockito.any(), Mockito.eq(cryptor))).thenReturn(encChannel); + Mockito.when(encChannel.write(Mockito.any())).thenReturn(0); + + dirIdBackup.execute(cipherDirObject); + + Assertions.assertTrue(Files.exists(contentPath.resolve(Constants.DIR_ID_FILE))); + } + } + + @Test + public void testContentIsWritten() throws IOException { + Mockito.when(encChannel.write(Mockito.any())).thenReturn(0); + var expectedWrittenContent = ByteBuffer.wrap(dirId.getBytes(StandardCharsets.UTF_8)); + + try (MockedStatic backupMock = Mockito.mockStatic(DirectoryIdBackup.class)) { + backupMock.when(() -> DirectoryIdBackup.wrapEncryptionAround(Mockito.any(), Mockito.eq(cryptor))).thenReturn(encChannel); + + dirIdBackup.execute(cipherDirObject); + + Mockito.verify(encChannel, Mockito.times(1)).write(Mockito.argThat(b -> b.equals(expectedWrittenContent))); + } + } + +} From d31a661119229cc7378ca1a5b6ac5a710004e62b Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Thu, 27 Jan 2022 18:06:18 +0100 Subject: [PATCH 04/14] add integration test for dirBackup --- .../cryptofs/CryptoFileSystemImplTest.java | 34 +++++++++++++++++-- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java b/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java index d01eeff6..72d1a203 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; @@ -115,7 +114,7 @@ public void setup() { Path other = invocation.getArgument(0); return other; }); - + when(fileSystemProperties.maxCleartextNameLength()).thenReturn(32768); inTest = new CryptoFileSystemImpl(provider, cryptoFileSystems, pathToVault, cryptor, @@ -1096,7 +1095,36 @@ public void createDirectoryClearsDirIdAndDeletesDirFileIfCreatingDirFails() thro verify(cryptoPathMapper).invalidatePathMapping(path); } - //TODO: write tests for backupDirId + @Test + public void createDirectoryBackupsDirIdInCiphertextDirPath() throws IOException { + Path ciphertextParent = mock(Path.class, "ciphertextParent"); + Path ciphertextRawPath = mock(Path.class, "d/00/00/path.c9r"); + Path ciphertextDirFile = mock(Path.class, "d/00/00/path.c9r/dir.c9r"); + Path ciphertextDirPath = mock(Path.class, "d/FF/FF/"); + CiphertextFilePath ciphertextPath = mock(CiphertextFilePath.class, "ciphertext"); + String dirId = "DirId1234ABC"; + CiphertextDirectory cipherDirObject = new CiphertextDirectory(dirId, ciphertextDirPath); + FileChannelMock channel = new FileChannelMock(100); + when(ciphertextRawPath.resolve("dir.c9r")).thenReturn(ciphertextDirFile); + when(cryptoPathMapper.getCiphertextFilePath(path)).thenReturn(ciphertextPath); + when(cryptoPathMapper.getCiphertextDir(path)).thenReturn(cipherDirObject); + when(cryptoPathMapper.getCiphertextDir(parent)).thenReturn(new CiphertextDirectory("parentDirId", ciphertextDirPath)); + when(cryptoPathMapper.getCiphertextFileType(path)).thenThrow(NoSuchFileException.class); + when(ciphertextPath.getRawPath()).thenReturn(ciphertextRawPath); + when(ciphertextPath.getDirFilePath()).thenReturn(ciphertextDirFile); + when(ciphertextParent.getFileSystem()).thenReturn(fileSystem); + when(ciphertextRawPath.getFileSystem()).thenReturn(fileSystem); + when(ciphertextDirFile.getFileSystem()).thenReturn(fileSystem); + when(ciphertextDirPath.getFileSystem()).thenReturn(fileSystem); + when(ciphertextDirFile.getName(3)).thenReturn(mock(Path.class, "path.c9r")); + when(provider.newFileChannel(ciphertextDirFile, EnumSet.of(StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE))).thenReturn(channel); + + inTest.createDirectory(path); + + verify(readonlyFlag).assertWritable(); + verify(dirIdBackup, Mockito.times(1)).execute(cipherDirObject); + } + } From 76534338d6fd37e2544ae34eb20d81fdc69b7085 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Fri, 4 Feb 2022 17:47:04 +0100 Subject: [PATCH 05/14] do not backup dirId implicitly --- .../cryptofs/dir/DirectoryStreamFactory.java | 11 ++--------- .../cryptofs/dir/CryptoDirectoryStreamTest.java | 5 ++--- .../cryptofs/dir/DirectoryStreamFactoryTest.java | 4 +--- 3 files changed, 5 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java b/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java index 709e3418..550017b4 100644 --- a/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java +++ b/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java @@ -5,6 +5,7 @@ import org.cryptomator.cryptofs.CryptoPathMapper; import org.cryptomator.cryptofs.CryptoPathMapper.CiphertextDirectory; import org.cryptomator.cryptofs.DirectoryIdBackup; +import org.cryptomator.cryptofs.common.Constants; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -26,16 +27,14 @@ public class DirectoryStreamFactory { private final CryptoPathMapper cryptoPathMapper; private final DirectoryStreamComponent.Builder directoryStreamComponentBuilder; // sharing reusable builder via synchronized - private final DirectoryIdBackup dirIdBackup; private final Map streams = new HashMap<>(); private volatile boolean closed = false; @Inject - public DirectoryStreamFactory(CryptoPathMapper cryptoPathMapper, DirectoryStreamComponent.Builder directoryStreamComponentBuilder, DirectoryIdBackup dirIdBackup) { + public DirectoryStreamFactory(CryptoPathMapper cryptoPathMapper, DirectoryStreamComponent.Builder directoryStreamComponentBuilder) { this.cryptoPathMapper = cryptoPathMapper; this.directoryStreamComponentBuilder = directoryStreamComponentBuilder; - this.dirIdBackup = dirIdBackup; } public synchronized CryptoDirectoryStream newDirectoryStream(CryptoPath cleartextDir, Filter filter) throws IOException { @@ -43,12 +42,6 @@ public synchronized CryptoDirectoryStream newDirectoryStream(CryptoPath cleartex throw new ClosedFileSystemException(); } CiphertextDirectory ciphertextDir = cryptoPathMapper.getCiphertextDir(cleartextDir); - //TODO: should we really do this implicit backup? - try{ - dirIdBackup.execute(ciphertextDir); - } catch (IOException e) { - LOG.info("Cannot create dirId backup of {}. Probably read only."); - } DirectoryStream ciphertextDirStream = Files.newDirectoryStream(ciphertextDir.path); CryptoDirectoryStream cleartextDirStream = directoryStreamComponentBuilder // .dirId(ciphertextDir.dirId) // diff --git a/src/test/java/org/cryptomator/cryptofs/dir/CryptoDirectoryStreamTest.java b/src/test/java/org/cryptomator/cryptofs/dir/CryptoDirectoryStreamTest.java index c7c2f0d6..30474546 100644 --- a/src/test/java/org/cryptomator/cryptofs/dir/CryptoDirectoryStreamTest.java +++ b/src/test/java/org/cryptomator/cryptofs/dir/CryptoDirectoryStreamTest.java @@ -11,7 +11,6 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.ArgumentMatcher; import org.mockito.Mockito; import java.io.IOException; @@ -32,7 +31,7 @@ public class CryptoDirectoryStreamTest { private static final Consumer DO_NOTHING_ON_CLOSE = ignored -> { }; private static final Filter ACCEPT_ALL = ignored -> true; - + private NodeProcessor nodeProcessor; private DirectoryStream dirStream; @@ -70,7 +69,7 @@ public void testDirListing() throws IOException { Mockito.doAnswer(invocation -> { return Stream.empty(); }).when(nodeProcessor).process(Mockito.argThat(node -> node.fullCiphertextFileName.equals("invalidCiphertext"))); - + try (CryptoDirectoryStream stream = new CryptoDirectoryStream("foo", dirStream, cleartextPath, ACCEPT_ALL, DO_NOTHING_ON_CLOSE, nodeProcessor)) { Iterator iter = stream.iterator(); Assertions.assertTrue(iter.hasNext()); diff --git a/src/test/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactoryTest.java b/src/test/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactoryTest.java index f0d2429e..565f4947 100644 --- a/src/test/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactoryTest.java +++ b/src/test/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactoryTest.java @@ -3,7 +3,6 @@ import org.cryptomator.cryptofs.CryptoPath; import org.cryptomator.cryptofs.CryptoPathMapper; import org.cryptomator.cryptofs.CryptoPathMapper.CiphertextDirectory; -import org.cryptomator.cryptofs.DirectoryIdBackup; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -31,9 +30,8 @@ public class DirectoryStreamFactoryTest { private final CryptoPathMapper cryptoPathMapper = mock(CryptoPathMapper.class); private final DirectoryStreamComponent directoryStreamComp = mock(DirectoryStreamComponent.class); private final DirectoryStreamComponent.Builder directoryStreamBuilder = mock(DirectoryStreamComponent.Builder.class); - private final DirectoryIdBackup directoryIdBackup = mock(DirectoryIdBackup.class); - private final DirectoryStreamFactory inTest = new DirectoryStreamFactory(cryptoPathMapper, directoryStreamBuilder, directoryIdBackup); + private final DirectoryStreamFactory inTest = new DirectoryStreamFactory(cryptoPathMapper, directoryStreamBuilder); @SuppressWarnings("unchecked") From 6e074dff6b96d71ad9fa3a1d1dc47dfd93b284ec Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Fri, 4 Feb 2022 17:51:44 +0100 Subject: [PATCH 06/14] remove mockito-junit runner --- pom.xml | 6 ------ .../org/cryptomator/cryptofs/DirectoryIdBackupTest.java | 9 +++------ 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/pom.xml b/pom.xml index e1ce0643..92f9eb0d 100644 --- a/pom.xml +++ b/pom.xml @@ -100,12 +100,6 @@ ${mockito.version} test - - org.mockito - mockito-junit-jupiter - ${mockito.version} - test - org.hamcrest hamcrest diff --git a/src/test/java/org/cryptomator/cryptofs/DirectoryIdBackupTest.java b/src/test/java/org/cryptomator/cryptofs/DirectoryIdBackupTest.java index 51bd12a7..cf5441c2 100644 --- a/src/test/java/org/cryptomator/cryptofs/DirectoryIdBackupTest.java +++ b/src/test/java/org/cryptomator/cryptofs/DirectoryIdBackupTest.java @@ -6,12 +6,9 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.io.TempDir; -import org.mockito.Mock; import org.mockito.MockedStatic; import org.mockito.Mockito; -import org.mockito.junit.jupiter.MockitoExtension; import java.io.IOException; import java.nio.ByteBuffer; @@ -19,7 +16,6 @@ import java.nio.file.Files; import java.nio.file.Path; -@ExtendWith(MockitoExtension.class) public class DirectoryIdBackupTest { @TempDir @@ -27,9 +23,7 @@ public class DirectoryIdBackupTest { private String dirId = "12345678"; private CryptoPathMapper.CiphertextDirectory cipherDirObject; - @Mock private EncryptingWritableByteChannel encChannel; - @Mock private Cryptor cryptor; private DirectoryIdBackup dirIdBackup; @@ -38,6 +32,9 @@ public class DirectoryIdBackupTest { @BeforeEach public void init() { cipherDirObject = new CryptoPathMapper.CiphertextDirectory(dirId, contentPath); + cryptor = Mockito.mock(Cryptor.class); + encChannel = Mockito.mock(EncryptingWritableByteChannel.class); + dirIdBackup = new DirectoryIdBackup(cryptor); } From fe9722eb3689fac63631c579c70d53789f165ee5 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Sat, 5 Feb 2022 00:06:52 +0100 Subject: [PATCH 07/14] Remove clearetext filter for dir.c9r --- .../java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java index 35052047..d0396a64 100644 --- a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java +++ b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java @@ -316,9 +316,8 @@ void createDirectory(CryptoPath cleartextDir, FileAttribute... attrs) throws } } - //TODO: where should the filter decorator be placed? Here or DirectoryStreamFactory? DirectoryStream newDirectoryStream(CryptoPath cleartextDir, Filter filter) throws IOException { - return directoryStreamFactory.newDirectoryStream(cleartextDir, entry -> !entry.getFileName().equals(Constants.DIR_ID_FILE) && filter.accept(entry)); + return directoryStreamFactory.newDirectoryStream(cleartextDir, filter); } FileChannel newFileChannel(CryptoPath cleartextPath, Set optionsSet, FileAttribute... attrs) throws IOException { From aa8ced11b2ad8b0f096567ec19ad0a418f8d796d Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Sat, 5 Feb 2022 00:07:33 +0100 Subject: [PATCH 08/14] implement ciphertext dir filter and add unit test --- .../cryptofs/common/Constants.java | 1 + .../cryptofs/dir/DirectoryStreamFactory.java | 15 ++++++---- .../dir/DirectoryStreamFactoryTest.java | 30 +++++++++++++++++-- 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptofs/common/Constants.java b/src/main/java/org/cryptomator/cryptofs/common/Constants.java index c36b95ca..c5ec40c1 100644 --- a/src/main/java/org/cryptomator/cryptofs/common/Constants.java +++ b/src/main/java/org/cryptomator/cryptofs/common/Constants.java @@ -29,6 +29,7 @@ private Constants() { public static final int DEFAULT_SHORTENING_THRESHOLD = 220; public static final int MAX_SYMLINK_LENGTH = 32767; // max path length on NTFS and FAT32: 32k-1 public static final int MAX_DIR_FILE_LENGTH = 36; // UUIDv4: hex-encoded 16 byte int + 4 hyphens = 36 ASCII chars + public static final int MIN_CIPHER_NAME_LENGTH = 26; //rounded up base64url encoded (16 bytes IV + 0 bytes empty string) + file suffix = 26 ASCII chars public static final String SEPARATOR = "/"; public static final String RECOVERY_DIR_NAME = "CRYPTOMATOR_RECOVERY"; diff --git a/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java b/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java index 550017b4..df6ebb8c 100644 --- a/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java +++ b/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java @@ -4,10 +4,7 @@ import org.cryptomator.cryptofs.CryptoPath; import org.cryptomator.cryptofs.CryptoPathMapper; import org.cryptomator.cryptofs.CryptoPathMapper.CiphertextDirectory; -import org.cryptomator.cryptofs.DirectoryIdBackup; import org.cryptomator.cryptofs.common.Constants; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import javax.inject.Inject; import java.io.IOException; @@ -23,8 +20,6 @@ @CryptoFileSystemScoped public class DirectoryStreamFactory { - private static final Logger LOG = LoggerFactory.getLogger(DirectoryStreamFactory.class); - private final CryptoPathMapper cryptoPathMapper; private final DirectoryStreamComponent.Builder directoryStreamComponentBuilder; // sharing reusable builder via synchronized private final Map streams = new HashMap<>(); @@ -42,7 +37,8 @@ public synchronized CryptoDirectoryStream newDirectoryStream(CryptoPath cleartex throw new ClosedFileSystemException(); } CiphertextDirectory ciphertextDir = cryptoPathMapper.getCiphertextDir(cleartextDir); - DirectoryStream ciphertextDirStream = Files.newDirectoryStream(ciphertextDir.path); + //TODO: use HealthCheck with warning and suggest fix to create one + DirectoryStream ciphertextDirStream = Files.newDirectoryStream(ciphertextDir.path, this::cryptofsEncryptedDataFilter); CryptoDirectoryStream cleartextDirStream = directoryStreamComponentBuilder // .dirId(ciphertextDir.dirId) // .ciphertextDirectoryStream(ciphertextDirStream) // @@ -55,6 +51,13 @@ public synchronized CryptoDirectoryStream newDirectoryStream(CryptoPath cleartex return cleartextDirStream; } + //visible for testing + boolean cryptofsEncryptedDataFilter(Path path) { + var tmp = path.getFileName().toString(); + return tmp.length() >= Constants.MIN_CIPHER_NAME_LENGTH // + && (tmp.endsWith(Constants.CRYPTOMATOR_FILE_SUFFIX) || tmp.endsWith(Constants.DEFLATED_FILE_SUFFIX)); + } + public synchronized void close() throws IOException { closed = true; IOException exception = new IOException("Close failed"); diff --git a/src/test/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactoryTest.java b/src/test/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactoryTest.java index 565f4947..d9da8b91 100644 --- a/src/test/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactoryTest.java +++ b/src/test/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactoryTest.java @@ -5,7 +5,11 @@ import org.cryptomator.cryptofs.CryptoPathMapper.CiphertextDirectory; 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.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.mockito.Mockito; import java.io.IOException; @@ -15,6 +19,7 @@ import java.nio.file.FileSystem; import java.nio.file.Path; import java.nio.file.spi.FileSystemProvider; +import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.mockito.ArgumentMatchers.any; @@ -95,12 +100,33 @@ public void testCloseClosesAllNonClosedDirectoryStreams() throws IOException { public void testNewDirectoryStreamAfterClosedThrowsClosedFileSystemException() throws IOException { CryptoPath path = mock(CryptoPath.class); Filter filter = mock(Filter.class); - + inTest.close(); - + Assertions.assertThrows(ClosedFileSystemException.class, () -> { inTest.newDirectoryStream(path, filter); }); } + @DisplayName("CiphertextDirStream only contains files with names at least 26 chars long and ending with .c9r or .c9s") + @ParameterizedTest() + @MethodSource("provideFilterExamples") + public void testCiphertextDirStreamFilter(String fileName, boolean expected) { + Path p = Mockito.mock(Path.class); + Mockito.when(p.getFileName()).thenReturn(p); + Mockito.when(p.toString()).thenReturn(fileName); + + boolean actual = inTest.cryptofsEncryptedDataFilter(p); + + Assertions.assertEquals(expected, actual); + } + + private static Stream provideFilterExamples() { + return Stream.of( // + Arguments.of("foo25____25chars_____.c9r", false), // + Arguments.of("bar25____25chars_____.c9s", false), // + Arguments.of("foo26____26chars______.c9r", true), // + Arguments.of("bar26____26chars______.c9s", true)); + } + } From c6839d36faefd701bfa191d8dae76318b706bf2c Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Sat, 5 Feb 2022 00:48:09 +0100 Subject: [PATCH 09/14] fix code smells --- .../java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java | 1 - .../java/org/cryptomator/cryptofs/DirectoryIdBackup.java | 4 +--- .../cryptomator/cryptofs/dir/DirectoryStreamFactory.java | 6 +++--- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java index d0396a64..91f57a44 100644 --- a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java +++ b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java @@ -15,7 +15,6 @@ import org.cryptomator.cryptofs.attr.AttributeViewType; import org.cryptomator.cryptofs.common.ArrayUtils; import org.cryptomator.cryptofs.common.CiphertextFileType; -import org.cryptomator.cryptofs.common.Constants; import org.cryptomator.cryptofs.common.DeletingFileVisitor; import org.cryptomator.cryptofs.common.FinallyUtil; import org.cryptomator.cryptofs.dir.CiphertextDirectoryDeleter; diff --git a/src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java b/src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java index 63c4e868..8e64eba3 100644 --- a/src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java +++ b/src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java @@ -11,8 +11,6 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.StandardOpenOption; -import java.util.function.BiFunction; -import java.util.function.Function; /** * Single purpose class to backup the directory id of an encrypted directory when it is created. @@ -38,7 +36,7 @@ public DirectoryIdBackup(Cryptor cryptor) { */ public void execute(CryptoPathMapper.CiphertextDirectory ciphertextDirectory) throws IOException { try (var channel = Files.newByteChannel(ciphertextDirectory.path.resolve(Constants.DIR_ID_FILE), StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE); // - var encryptingChannel = wrapEncryptionAround(channel,cryptor)) { + var encryptingChannel = wrapEncryptionAround(channel, cryptor)) { encryptingChannel.write(ByteBuffer.wrap(ciphertextDirectory.dirId.getBytes(StandardCharsets.UTF_8))); } } diff --git a/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java b/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java index df6ebb8c..30c9853b 100644 --- a/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java +++ b/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java @@ -22,7 +22,7 @@ public class DirectoryStreamFactory { private final CryptoPathMapper cryptoPathMapper; private final DirectoryStreamComponent.Builder directoryStreamComponentBuilder; // sharing reusable builder via synchronized - private final Map streams = new HashMap<>(); + private final Map> streams = new HashMap<>(); private volatile boolean closed = false; @@ -61,9 +61,9 @@ boolean cryptofsEncryptedDataFilter(Path path) { public synchronized void close() throws IOException { closed = true; IOException exception = new IOException("Close failed"); - Iterator> iter = streams.entrySet().iterator(); + Iterator>> iter = streams.entrySet().iterator(); while (iter.hasNext()) { - Map.Entry entry = iter.next(); + Map.Entry> entry = iter.next(); iter.remove(); try { entry.getKey().close(); From 0338ea053378c7cd2cf4f77891231513b29bb16a Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 8 Feb 2022 18:05:09 +0100 Subject: [PATCH 10/14] apply suggestions from code review Co-authored by: Sebastian Stenzel --- .../java/org/cryptomator/cryptofs/common/Constants.java | 2 +- .../cryptomator/cryptofs/dir/DirectoryStreamFactory.java | 9 ++++----- .../cryptofs/dir/DirectoryStreamFactoryTest.java | 4 ++-- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptofs/common/Constants.java b/src/main/java/org/cryptomator/cryptofs/common/Constants.java index c5ec40c1..b56dbfee 100644 --- a/src/main/java/org/cryptomator/cryptofs/common/Constants.java +++ b/src/main/java/org/cryptomator/cryptofs/common/Constants.java @@ -33,5 +33,5 @@ private Constants() { public static final String SEPARATOR = "/"; public static final String RECOVERY_DIR_NAME = "CRYPTOMATOR_RECOVERY"; - public static final String DIR_ID_FILE = "id.c9r"; + public static final String DIR_ID_FILE = "dirid.c9r"; } diff --git a/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java b/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java index 30c9853b..0b614c88 100644 --- a/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java +++ b/src/main/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactory.java @@ -14,7 +14,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.HashMap; -import java.util.Iterator; import java.util.Map; @CryptoFileSystemScoped @@ -38,7 +37,7 @@ public synchronized CryptoDirectoryStream newDirectoryStream(CryptoPath cleartex } CiphertextDirectory ciphertextDir = cryptoPathMapper.getCiphertextDir(cleartextDir); //TODO: use HealthCheck with warning and suggest fix to create one - DirectoryStream ciphertextDirStream = Files.newDirectoryStream(ciphertextDir.path, this::cryptofsEncryptedDataFilter); + DirectoryStream ciphertextDirStream = Files.newDirectoryStream(ciphertextDir.path, this::matchesEncryptedContentPattern); CryptoDirectoryStream cleartextDirStream = directoryStreamComponentBuilder // .dirId(ciphertextDir.dirId) // .ciphertextDirectoryStream(ciphertextDirStream) // @@ -52,7 +51,7 @@ public synchronized CryptoDirectoryStream newDirectoryStream(CryptoPath cleartex } //visible for testing - boolean cryptofsEncryptedDataFilter(Path path) { + boolean matchesEncryptedContentPattern(Path path) { var tmp = path.getFileName().toString(); return tmp.length() >= Constants.MIN_CIPHER_NAME_LENGTH // && (tmp.endsWith(Constants.CRYPTOMATOR_FILE_SUFFIX) || tmp.endsWith(Constants.DEFLATED_FILE_SUFFIX)); @@ -61,9 +60,9 @@ boolean cryptofsEncryptedDataFilter(Path path) { public synchronized void close() throws IOException { closed = true; IOException exception = new IOException("Close failed"); - Iterator>> iter = streams.entrySet().iterator(); + var iter = streams.entrySet().iterator(); while (iter.hasNext()) { - Map.Entry> entry = iter.next(); + var entry = iter.next(); iter.remove(); try { entry.getKey().close(); diff --git a/src/test/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactoryTest.java b/src/test/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactoryTest.java index d9da8b91..981a885d 100644 --- a/src/test/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactoryTest.java +++ b/src/test/java/org/cryptomator/cryptofs/dir/DirectoryStreamFactoryTest.java @@ -109,14 +109,14 @@ public void testNewDirectoryStreamAfterClosedThrowsClosedFileSystemException() t } @DisplayName("CiphertextDirStream only contains files with names at least 26 chars long and ending with .c9r or .c9s") - @ParameterizedTest() + @ParameterizedTest @MethodSource("provideFilterExamples") public void testCiphertextDirStreamFilter(String fileName, boolean expected) { Path p = Mockito.mock(Path.class); Mockito.when(p.getFileName()).thenReturn(p); Mockito.when(p.toString()).thenReturn(fileName); - boolean actual = inTest.cryptofsEncryptedDataFilter(p); + boolean actual = inTest.matchesEncryptedContentPattern(p); Assertions.assertEquals(expected, actual); } From 65f0625182e05c29fcf64025c2f3147e23d3e8dd Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 15 Mar 2022 12:52:09 +0100 Subject: [PATCH 11/14] add/remove comment --- src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java | 1 - src/main/java/org/cryptomator/cryptofs/DirectoryIdLoader.java | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java b/src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java index 8e64eba3..9ebe78bb 100644 --- a/src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java +++ b/src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java @@ -18,7 +18,6 @@ @CryptoFileSystemScoped public class DirectoryIdBackup { - //visible and existing for testing private Cryptor cryptor; @Inject diff --git a/src/main/java/org/cryptomator/cryptofs/DirectoryIdLoader.java b/src/main/java/org/cryptomator/cryptofs/DirectoryIdLoader.java index 627ee94b..6c520158 100644 --- a/src/main/java/org/cryptomator/cryptofs/DirectoryIdLoader.java +++ b/src/main/java/org/cryptomator/cryptofs/DirectoryIdLoader.java @@ -8,6 +8,7 @@ import java.nio.channels.Channels; import java.nio.channels.FileChannel; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.StandardOpenOption; @@ -24,6 +25,7 @@ public DirectoryIdLoader() { @Override public String load(Path dirFilePath) throws IOException { + //TODO: replace by Files.readString(StandardCharsets.UTF_8) try (FileChannel ch = FileChannel.open(dirFilePath, StandardOpenOption.READ); InputStream in = Channels.newInputStream(ch)) { long size = ch.size(); From 1f72a65ce639e8fe1fbd6e428bd6fb81268c0a1c Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 16 Mar 2022 16:04:48 +0100 Subject: [PATCH 12/14] Apply suggestions from code review Co-authored-by: Sebastian Stenzel --- .../cryptofs/DirectoryIdLoader.java | 2 -- ...ptyCiphertextDirectoryIntegrationTest.java | 22 ++++++++++++++++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptofs/DirectoryIdLoader.java b/src/main/java/org/cryptomator/cryptofs/DirectoryIdLoader.java index 6c520158..627ee94b 100644 --- a/src/main/java/org/cryptomator/cryptofs/DirectoryIdLoader.java +++ b/src/main/java/org/cryptomator/cryptofs/DirectoryIdLoader.java @@ -8,7 +8,6 @@ import java.nio.channels.Channels; import java.nio.channels.FileChannel; import java.nio.charset.StandardCharsets; -import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.StandardOpenOption; @@ -25,7 +24,6 @@ public DirectoryIdLoader() { @Override public String load(Path dirFilePath) throws IOException { - //TODO: replace by Files.readString(StandardCharsets.UTF_8) try (FileChannel ch = FileChannel.open(dirFilePath, StandardOpenOption.READ); InputStream in = Channels.newInputStream(ch)) { long size = ch.size(); diff --git a/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java b/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java index 9fa1e308..d4bee8fa 100644 --- a/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java +++ b/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java @@ -16,6 +16,7 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import org.mockito.Mockito; @@ -28,6 +29,7 @@ import java.nio.file.FileSystem; import java.nio.file.Files; import java.nio.file.Path; +import java.util.function.Predicate; import java.util.stream.Stream; import static java.nio.file.StandardOpenOption.CREATE_NEW; @@ -159,21 +161,35 @@ private Path firstEmptyCiphertextDirectory() throws IOException { try (Stream allFilesInVaultDir = Files.walk(pathToVault)) { return allFilesInVaultDir // .filter(Files::isDirectory) // - .filter(this::isEmptyDirectory) // + .filter(this::isEmptyCryptoFsDirectory) // .filter(this::isEncryptedDirectory) // .findFirst() // .get(); } } - private boolean isEmptyDirectory(Path path) { + private boolean isEmptyCryptoFsDirectory(Path path) { + Predicate isIgnoredFile = p -> Constants.DIR_ID_FILE.equals(p.getFileName().toString()); try (Stream files = Files.list(path)) { - return files.filter(p -> p.getFileName().toString().equals(Constants.DIR_ID_FILE)).count() == 0; + return files.noneMatch(isIgnoredFile.negate()); } catch (IOException e) { throw new UncheckedIOException(e); } } + @Test + @DisplayName("Tests internal cryptofs directory emptiness definition") + public void testCryptoFsDirEmptiness() throws IOException { + var emptiness = pathToVault.getParent().resolve("emptiness"); + var ignoredFile = emptiness.resolve(Constants.DIR_ID_FILE); + Files.createDirectory(emptiness); + Files.createFile(ignoredFile); + + boolean result = isEmptyCryptoFsDirectory(emptiness); + + Assertions.assertTrue(result, "Directory containing only dir id file is not considered empty"); + } + private boolean isEncryptedDirectory(Path pathInVault) { Path relativePath = pathToVault.relativize(pathInVault); String relativePathAsString = relativePath.toString().replace(File.separatorChar, '/'); From 2c0b6d6d1db43c696b4c6bd33f24468a575c41e0 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 16 Mar 2022 18:30:48 +0100 Subject: [PATCH 13/14] Improving integration test --- .../DeleteNonEmptyCiphertextDirectoryIntegrationTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java b/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java index d4bee8fa..a3a7c4e9 100644 --- a/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java +++ b/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java @@ -171,7 +171,7 @@ private Path firstEmptyCiphertextDirectory() throws IOException { private boolean isEmptyCryptoFsDirectory(Path path) { Predicate isIgnoredFile = p -> Constants.DIR_ID_FILE.equals(p.getFileName().toString()); try (Stream files = Files.list(path)) { - return files.noneMatch(isIgnoredFile.negate()); + return files.filter(isIgnoredFile.negate()).findFirst().isEmpty(); } catch (IOException e) { throw new UncheckedIOException(e); } @@ -187,7 +187,7 @@ public void testCryptoFsDirEmptiness() throws IOException { boolean result = isEmptyCryptoFsDirectory(emptiness); - Assertions.assertTrue(result, "Directory containing only dir id file is not considered empty"); + Assertions.assertTrue(result, "Ciphertext directory containing only dirId-file should be accepted as an empty dir"); } private boolean isEncryptedDirectory(Path pathInVault) { From 310c038363fe2b244bb8f991a46dc1c7d2b85a7f Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Thu, 17 Mar 2022 12:21:44 +0100 Subject: [PATCH 14/14] Partially revert 2c0b6d6d1db43c696b4c6bd33f24468a575c41e0 --- .../DeleteNonEmptyCiphertextDirectoryIntegrationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java b/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java index a3a7c4e9..3c04a6fa 100644 --- a/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java +++ b/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java @@ -171,7 +171,7 @@ private Path firstEmptyCiphertextDirectory() throws IOException { private boolean isEmptyCryptoFsDirectory(Path path) { Predicate isIgnoredFile = p -> Constants.DIR_ID_FILE.equals(p.getFileName().toString()); try (Stream files = Files.list(path)) { - return files.filter(isIgnoredFile.negate()).findFirst().isEmpty(); + return files.noneMatch(isIgnoredFile.negate()); } catch (IOException e) { throw new UncheckedIOException(e); }