Node: fs.copyFile creates empty file if source and destination are the same

[spikeburton]

• Version: 12.18.3
• Platform: MacOS (Darwin Kernel Version 19.6.0)
• Subsystem: fs

What steps will reproduce the bug?

1. Create a new file: echo "Hello World" >> test.txt
2. Run the following snippet:
   js const fs = require('fs'); fs.copyFileSync('test.txt', 'test.txt');
3. Observe output: cat test.txt

How often does it reproduce? Is there a required condition?

The behavior here appears to be consistent.

What is the expected behavior?

I expect to either receive an error that this action should not be performed, or that the file ends up in the same state as it was initially.

What do you see instead?

The file has been overwritten and is now empty. All data has been lost.

Additional information

This bug could potentially cause data loss in certain situations.

[iandrc]

Hm... Interesting. Though the behavior seems to be kinda predictable. However, I got your point and think this could be done a bit better.

Btw, you named the issue fs.copyFile() but you actually used fs.copyFileSync() in your example. Is the behavior the same when using the async version?

@addaleax @richardlau what do you think about this one? Can we change the behavior of this one or it may impair something?

P.S. Please, assign this one to me as well. I'll dig into it.

[spikeburton]

@iandrc Apologies for the lack of clarity and consistency there, the behavior seems to be identical from my testing between fs.copyFile and fs.copyFileSync

[iandrc]

@spikeburton np, just asking.
I'll try to find a root cause of this and reproduce the issue on node 14.x.

UPD. It has the same behavior on node 14.x. I guess we can improve the function by adding the some checks on the src and dest params.

[richardlau]

@iandrc I think the copyfile behaviour is mostly in libuv. If I remember correctly the macOS implementation changed at some point (https://github.com/libuv/libuv/pull/2233) in libuv (also see follow up https://github.com/libuv/libuv/pull/2578).

At the very least it sounds like we should have a test for this (either in Node.js and/or libuv).

[iandrc]

Hey @richardlau

I'm not that familiar with C and/or libuv to change something there. Plus, the https://github.com/libuv/libuv/pull/2578 still was not merged. I would not expect that to change anytime soon, tbh.

By "test" you mean some checks within a function, right?

[spikeburton]

@richardlau FWIW I just tested and am also able to reproduce on CoreOS Linux

[iandrc]

@spikeburton I believe that is reproducible on Windows systems as well :)

Still, I would like to work on this stuff if you don't mind

[spikeburton]

@iandrc Please, by all means :)

[iandrc]

@cjihrig do we need any additional logic on the node side or your commit should fix this?

[cjihrig]

If https://github.com/libuv/libuv/pull/2947 lands, there shouldn't be any required changes on the Node side other than updating libuv.

[iandrc]

Ok, I got it. Thanks

[juanarbol]

Closing. The PR https://github.com/libuv/libuv/pull/2947 is landed, and libuv is was updated (https://github.com/nodejs/node/commit/62443686d9cf0915186d696ba48a0ae1f4926625) on Node, this issue will be solved on a next release.