Skip to content

Conversation

@GPlaczek
Copy link

@GPlaczek GPlaczek commented Mar 30, 2023

The problem

When user attempts to move a directory that contains symbolic links, it might result in an error without a proper cleanup.

How to reproduce

Sample code that produces the issue:

use fs_extra::dir::{CopyOptions, move_dir};

fn main() {
    println!("{:?}", move_dir("dir", "dir1", &CopyOptions {
        copy_inside: true,
        ..CopyOptions::new()}));

    println!("Hello, world!");
}

Sample input:

$ tree dir
dir
├── dir1
│   └── link1 -> ../file
├── file
└── link -> file

What happens

Running the above program against the given input leaves dir and dir1 directories in the following state:

$ tree dir1 dir
dir1
├── dir1
│   └── link1
└── file
dir
├── dir1
└── link -> file

What happens here is the file is copied before the link and the exists function returns false because it follows symbolic links (docs reference). After that, the function fails without cleaning up half-moved directory. Also, link1 after the copy holds the same data as file (which it points to) instead of being a symbolic link. I think this is not an expected behavior as well.

My fix

I fixed it by adding a follow field to file::CopyOptions (which defaults to true) and set it to false when invoked from move and copy functions from dir module. Setting this field prevents following symbolic links when moving or copying. Additionally, I fixed another small bug - setting overwrite to false in file::CopyOptions would still overwrite dangling symlinks, now checks are performed to prevent this.

My solution breaks backward compatibility, if you don't want to introduce such changes to your repository now, let me know and I can try to adjust it to be backwards compatible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant