Skip to content

Commit

Permalink
chimera: fix loop creation on directory move
Browse files Browse the repository at this point in the history
Motivation:
On directory move we must check that destination is not a subdirectory
on the source directory.

Modification:
Update JdbcFs#rename to check for a potential loop creation before move.
Added unit test to cover the use case.

Result:
no more loops on directory move.

Fixes: #7559
Ticket: #10608
Acked-by: Dmitry Litvintsev
Acked-by: Paul Millar
Target: master, 10.0, 9.2, 9.1, 9.0, 8.2
Require-book: no
Require-notes: yes
(cherry picked from commit 72da8e9)
Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
  • Loading branch information
kofemann committed May 6, 2024
1 parent 15f32f5 commit 6256911
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import static java.util.stream.Collectors.toList;
import static org.dcache.chimera.FileSystemProvider.SetXattrMode;
import static org.dcache.chimera.FileSystemProvider.StatCacheOption.NO_STAT;
import static org.dcache.chimera.FileSystemProvider.StatCacheOption.STAT;

import com.google.common.base.Strings;
Expand Down Expand Up @@ -50,6 +51,7 @@
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.OptionalLong;
import java.util.ServiceLoader;
import java.util.Set;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -474,7 +476,13 @@ FsInode mkdir(FsInode parent, String name, int owner, int group, int mode) {
* @param inode
* @return true if moved, false if source did not exist
*/
boolean rename(FsInode inode, FsInode srcDir, String source, FsInode destDir, String dest) {
boolean rename(FsInode inode, FsInode srcDir, String source, FsInode destDir, String dest) throws ChimeraFsException {

// If source is a directory check that we don't move into its own subdirectory
if (inode.isDirectory()) {
checkLoop(inode, destDir);
}

String moveLink = "UPDATE t_dirs SET iparent=?, iname=? WHERE iparent=? AND iname=? AND ichild=?";
int n = _jdbc.update(moveLink,
ps -> {
Expand Down Expand Up @@ -577,6 +585,30 @@ protected String inode2path(long elementId, long root) {
}
}

protected void checkLoop(FsInode inode, FsInode dst) throws InvalidArgumentChimeraException {
if (inode.ino() == dst.ino()) {
throw new InvalidArgumentChimeraException("Cannot move directory into itself.");
}

long destInumber = dst.ino();
while (true) {
OptionalLong parent = _jdbc.query("SELECT iparent FROM t_dirs WHERE ichild=?",
rs -> rs.next() ? OptionalLong.of(rs.getLong("iparent")) : OptionalLong.empty(),
destInumber);

if (parent.isEmpty()) {
// we have reached the root of the three
break;
}

long parentIno = parent.getAsLong();
if (parentIno == inode.ino()) {
throw new InvalidArgumentChimeraException("Cannot move directory into its own subdirectory.");
}
destInumber = parentIno;
}
}

FsInode createInodeInParent(FsInode parent, String name, String id, int owner, int group,
int mode,
int type, int nlink, long size) {
Expand Down
20 changes: 19 additions & 1 deletion modules/chimera/src/test/java/org/dcache/chimera/JdbcFsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -990,7 +990,8 @@ public void testMoveIntoDir() throws Exception {

@Test(expected = FileNotFoundChimeraFsException.class)
public void testMoveNotExists() throws Exception {
_fs.rename(_rootInode, _rootInode, "foo", _rootInode, "bar");
FsInode dummy = new FsInode(_fs, 31415);
_fs.rename(dummy, _rootInode, "foo", _rootInode, "bar");
}

@Test(expected = DirNotEmptyChimeraFsException.class)
Expand Down Expand Up @@ -2046,6 +2047,23 @@ public void testRemoveXattrAfterSet() throws Exception {
_fs.stat(inode).getGeneration(), greaterThan(s0.getGeneration()));
}

@Test(expected = InvalidArgumentChimeraException.class)
public void testMvDirectoryIntoItself() throws Exception {

FsInode dir = _fs.mkdir("/dir");
_fs.rename(dir, _rootInode, "dir", dir, "subdir");
}

@Test(expected = InvalidArgumentChimeraException.class)
public void testMvDirectoryOwnSubtree() throws Exception {

FsInode dir = _fs.mkdir("/dir");
_fs.mkdir("/dir/subdir1");
FsInode dir2 = _fs.mkdir("/dir/subdir1/subdir2");

_fs.rename(dir, _rootInode, "dir", dir2, "subdir3");
}

private long getDirEntryCount(FsInode dir) throws IOException {
try (var s = _fs.newDirectoryStream(dir)) {
return s.stream().count();
Expand Down

0 comments on commit 6256911

Please sign in to comment.