copy can delete file content

[TanninOne]

• Operating System: Windows
• Node.js version: 8.2.1
• fs-extra version: 4.0.3

I realize that fs-extra 5.0.0 uses fs.copy if supported but I believe this bug to be still present and relevant when using node.js versions older than 8.5.
Since the latest stable electron version is still using node.js 8.2.1 I think this might affect a considerable number of applications.

Ok, so there was a patch to prevent copying a file onto itself, but that patch is broken or rather: it doesn't consider all possible cases, mainly: case insensitive file systems.

if (src === dest) return cb(new Error('Source and destination must not be the same.'))

will not catch cases like copy('c:\\dummy.txt', 'C:\\dUmMy.txt') on a case insensitive drive.
But this is probably not the only possible situation where that check fails, you can easily construct a case using junction points/directory symlinks where two paths are different according to a string comparison but are actually the same file.

What happens in this case is that fs-extra creates the destination file empty, overwriting the existing file and then fails because the source file no longer exists for reading.
And to add insult to injury it leaves a file handle open so the empty file can't be removed until the application is closed.

[RyanZim]

Ouch, that's an ugly bug! @manidlou can you look into this? Any PR work should be against v6-dev branch.

[manidlou]

Sure thing!

[manidlou]

@TanninOne will you please try your case with v6.0.0? We wanna see if your case is resolved in v6.0.0.

[TanninOne]

It fixes this case but looking at the code, if I'm honest, I don't think this was solved cleanly.
case-insensitivity depends on both operating system and filesystem. A fat32 partition mounted on linux is still case insensitive, whereas NTFS is actually case sensitive but the win32 api usually isn't (but this can apparently be changed in the registry).

Also, this change doesn't fix the problem with links/junction points.
Let's say you create directory "a" with file x.txt in it and junction point "b" that links to "a".
If you now copy('a/x.txt', 'b/x.txt') the file is deleted and you get an error message.

I'm not sure what the best way to fix this would be. You could try to stat source and destination files and refuse to copy if they have the same ino id.
Or maybe you can acquire an exclusive lock on the source file that would disallow it from being modified for the duration of the copy. This way if the destination file is the same file you should get a "file in use" error message or something.

[manidlou]

@TanninOne I confirm that! Though one of those edge cases! I am working on it.

[RyanZim]

Fixed by #582; will be released in v7

[TanninOne]

I'm so sorry to have to return to this but I feel I have to warn you about this bug in node.js:
nodejs/node#12115
In short: On windows node.js will produce the same ino for different files. The bug is fixed but who knows how long it will take until stuff like electron has been updated? This is not a theoretical issue either, my own application stumbles over it occasionally.
Tbh at this point I'm not sure what to do about it in regards to the copy function.

[RyanZim]

IMO, there's nothing we can do about it when Node.js itself has a bug. This is a problem with Node.js, not us; it's unfortunate that fs-extra users are affected by Node's bug.