LongFileNameProvider.DeflatedFileName::persist can corrupt vault structure

[infeo]

CryptoFS supports storing a resource name in an additional file, if the string would exceed an underlying filesystem limit. Instead of the encrypted & encoded filename the target resource is "named" with a placeholder derived from the long name (see LongFileNameProvidder class).

During creation of a resource, if the filename is "long", the a persist method on that filename is called: https://github.com/cryptomator/cryptofs/blob/9fce43013cac264b229ff9f5146b7ef2e46509c1/src/main/java/org/cryptomator/cryptofs/LongFileNameProvider.java#L104-L111

The problem about the method is, that any appearing, checked IOException is wrapped in an UncheckedIOException, effectivley hiding the behaviour. This method is solely called by CiphertextFilePath::persistLongFileName. Functions relying on this persistence do catch UncheckedIOException, which in case of a throw leaves the vault structure in an unclean state.

As an example, if a directory is created and the persistence calls fails, the already created files or folder are not removed again, because only IOExceptions are catched:

    void createDirectory(CryptoPath cleartextDir, FileAttribute<?>... attrs) throws IOException {
        /*
           some preparation before
        */
        Files.createDirectory(ciphertextPath.getRawPath());
        try (FileChannel channel = FileChannel.open(ciphertextDirFile, EnumSet.of(StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE), attrs)) {
            channel.write(UTF_8.encode(ciphertextDir.dirId));
        }
        // 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);
            ciphertextPath.persistLongFileName();    //this throws UncheckedIoException
        } catch (IOException e) {
            // make sure there is no orphan dir file:
            Files.delete(ciphertextDirFile);
            cryptoPathMapper.invalidatePathMapping(cleartextDir);
            dirIdProvider.delete(ciphertextDirFile);
            throw e;
        }
    }

I looked through the usages of CiphertextFilePath::persistLongFileName and it is only used in contexts, where an IOException can be thrown. Hence i suggest, that the function is refactored.

[overheadhunter]

So your suggestion is to make LongFileNameProvider.persist() and CiphertextFilePath.persistLongFileName() throw an IOException?

[infeo]

Yes