cryptomator / cryptofs

Java Filesystem Provider with integrated encryption
GNU Affero General Public License v3.0
94 stars 35 forks source link

DirectoryNotEmptyException when moving directory with REPLACE_EXISTING set #176

Closed JaniruTEC closed 1 year ago

JaniruTEC commented 1 year ago

According to the JavaDocs of Files#move moving an empty directory to an existing empty directory should work if REPLACE_EXISTING is set:

[...] This method may be invoked to move an empty directory. In some implementations a directory has entries for special files or links that are created when the directory is created. In such implementations a directory is considered empty when only the special entries exist. When invoked to move a directory that is not empty then the directory is moved if it does not require moving the entries in the directory. For example, renaming a directory on the same FileStore will usually not require moving the entries in the directory. When moving a directory requires that its entries be moved then this method fails (by throwing an IOException). [...]

[...] REPLACE_EXISTING If the target file exists, then the target file is replaced if it is not a non-empty directory. If the target file exists and is a symbolic link, then the symbolic link itself, not the target of the link, is replaced. [...]

The following fails with a DirectoryNotEmptyException :

var source = fs.getPath("/sourceDir");
var target = fs.getPath("/targetDir");
Files.createDirectory(source);
Files.createDirectory(target);

Assertions.assertDoesNotThrow(() -> Files.move(source, target, REPLACE_EXISTING));

Stacktrace:

java.nio.file.DirectoryNotEmptyException: /targetDir
    at org.cryptomator.cryptofs/org.cryptomator.cryptofs.CryptoFileSystemImpl.moveDirectory(CryptoFileSystemImpl.java:628)
    at org.cryptomator.cryptofs/org.cryptomator.cryptofs.CryptoFileSystemImpl.move(CryptoFileSystemImpl.java:575)
    at org.cryptomator.cryptofs/org.cryptomator.cryptofs.MoveOperation.move(MoveOperation.java:37)
    at org.cryptomator.cryptofs/org.cryptomator.cryptofs.CryptoFileSystemProvider.move(CryptoFileSystemProvider.java:255)
    at java.base/java.nio.file.Files.move(Files.java:1432)
    [...]

The exception is thrown here:

Path oldCiphertextDir = cryptoPathMapper.getCiphertextDir(cleartextTarget).path;
boolean oldCiphertextDirExists = true;
try (DirectoryStream<Path> ds = Files.newDirectoryStream(oldCiphertextDir)) {
    if (ds.iterator().hasNext()) {
        throw new DirectoryNotEmptyException(cleartextTarget.toString()); //628
    }
} catch (NoSuchFileException e) {
    oldCiphertextDirExists = false;
}

The offending file is /vault/d/.../.../dirid.c9r, presumably the dirid file of the target directory.

Likely resolution: The dirid file of the target directory should not count towards the empty-check.

overheadhunter commented 1 year ago

The dirid file of the target directory should not count towards the empty-check.

Agreed. The additional copy of the dir id file in d/2/30/ was added recently. I guess we forgot to adjust this logic.