fs_extra icon indicating copy to clipboard operation
fs_extra copied to clipboard

Data loss when moving directory on Windows

Open rgwood opened this issue 2 years ago • 4 comments

Hi, we use fs_extra in Nushell; thanks for writing it!

We recently had a scary bug report, and it looks like the root cause is fs_extra::dir::move_dir(). If you use move_dir() to change the case of a directory name on Windows (ex: "Test" -> "test"), the directory is deleted and fs_extra reports success.

For example:

// This deletes the directory named Test and does not panic!
fs_extra::dir::move_dir("Test", "test", &options).unwrap();

Steps to Reproduce

I've put together a minimal repro: https://github.com/rgwood/fs-extra-repro

Version Info

fs-extra v1.2.0 Windows 11 (can't reproduce on Linux) rustc v1.63.0

rgwood avatar Sep 19 '22 03:09 rgwood

Hi @rgwood! Thank you for the detail report. Sound awful. I will fix it on this week.

webdesus avatar Sep 20 '22 09:09 webdesus

I tried to come up with a fast crutch, but I see you already fixed this moment in your app. In the near future, I will try to fix this the same way. :smiley:

webdesus avatar Sep 23 '22 21:09 webdesus

This also happens when source equals target when moving or copying. Test case:

#[test]
fn it_move_work_same_path() {
    let test_dir = Path::new(TEST_FOLDER).join("it_move_work_same_path");
    let path_to = test_dir.join("dir1");
    let dir1 = (test_dir.join("dir1"), path_to.clone());
    let dir2 = (test_dir.join("dir2"), path_to.join("dir2"));
    let sub = (dir1.0.join("sub"), dir1.1.join("sub"));
    let file1 = (test_dir.join("file1.txt"), path_to.join("file1.txt"));
    let file2 = (test_dir.join("file2.txt"), path_to.join("file2.txt"));
    let file3 = (dir1.0.join("file3.txt"), dir1.1.join("file3.txt"));
    let file4 = (sub.0.join("file4.txt"), sub.1.join("file4.txt"));
    let file5 = (dir2.0.join("file5.txt"), dir2.1.join("file5.txt"));

    match dir::create_all(&path_to, true) {
        Ok(_) => {}
        Err(_) => {}
    };
    dir::create_all(&dir1.0, true).unwrap();
    dir::create_all(&dir2.0, true).unwrap();
    dir::create_all(&sub.0, true).unwrap();

    assert!(dir1.0.exists());
    assert!(dir1.1.exists());
    assert!(dir2.0.exists());
    assert!(!dir2.1.exists());
    assert!(sub.0.exists());
    assert!(sub.1.exists());

    file::write_all(&file1.0, "content1").unwrap();
    file::write_all(&file2.0, "content2").unwrap();
    file::write_all(&file3.0, "content3").unwrap();
    file::write_all(&file4.0, "content4").unwrap();
    file::write_all(&file5.0, "content5").unwrap();

    assert!(file1.0.exists());
    assert!(file2.0.exists());
    assert!(file3.0.exists());
    assert!(file4.0.exists());
    assert!(file5.0.exists());
    assert!(!file1.1.exists());
    assert!(!file2.1.exists());
    assert!(file3.1.exists());
    assert!(file4.1.exists());
    assert!(!file5.1.exists());

    let options = dir::CopyOptions::new();
    let result = dir::move_dir(dir1.0, path_to, &options).unwrap();

    assert_eq!(16, result);
    assert!(dir1.1.exists()); // <-- panics
    assert!(file3.1.exists()); // <-- panics
    assert!(file4.1.exists()); // <-- panics
}

pczarn avatar Sep 27 '22 14:09 pczarn

For comparison, the behavior of cp and mv:

$ LANG=C mv out/ .
mv: 'out/' and './out' are the same file
$ LANG=C mv out/ out/
mv: cannot move 'out/' to a subdirectory of itself, 'out/out'
$ LANG=C cp -r out/ .
cp: 'out/' and './out' are the same file

pczarn avatar Sep 27 '22 16:09 pczarn