From 278cfa018356c29b24b8be6a25dbefba84b406d3 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Tue, 4 Aug 2020 20:56:19 -0400 Subject: [PATCH] unix: handle src, dest same in uv_fs_copyfile() This commit handles the case where the source and destination are the same. This behavior was originally addressed in #2298, but the test added in that PR doesn't validate the file size after the operation. This commit also updates the test to check for that case. Refs: https://github.com/libuv/libuv/pull/2298 Refs: https://github.com/nodejs/node/issues/34624 PR-URL: https://github.com/libuv/libuv/pull/2947 Reviewed-By: Santiago Gimeno Reviewed-By: Richard Lau Reviewed-By: Anna Henningsen Reviewed-By: Jameson Nash --- src/unix/fs.c | 32 +++++++++++++++++++++----------- test/test-fs-copyfile.c | 5 +++++ 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/unix/fs.c b/src/unix/fs.c index a06f15b9..cbf078b7 100644 --- a/src/unix/fs.c +++ b/src/unix/fs.c @@ -1146,7 +1146,7 @@ static ssize_t uv__fs_copyfile(uv_fs_t* req) { goto out; } - dst_flags = O_WRONLY | O_CREAT | O_TRUNC; + dst_flags = O_WRONLY | O_CREAT; if (req->flags & UV_FS_COPYFILE_EXCL) dst_flags |= O_EXCL; @@ -1165,16 +1165,26 @@ static ssize_t uv__fs_copyfile(uv_fs_t* req) { goto out; } - /* Get the destination file's mode. */ - if (fstat(dstfd, &dst_statsbuf)) { - err = UV__ERR(errno); - goto out; - } + /* If the file is not being opened exclusively, verify that the source and + destination are not the same file. If they are the same, bail out early. */ + if ((req->flags & UV_FS_COPYFILE_EXCL) == 0) { + /* Get the destination file's mode. */ + if (fstat(dstfd, &dst_statsbuf)) { + err = UV__ERR(errno); + goto out; + } - /* Check if srcfd and dstfd refer to the same file */ - if (src_statsbuf.st_dev == dst_statsbuf.st_dev && - src_statsbuf.st_ino == dst_statsbuf.st_ino) { - goto out; + /* Check if srcfd and dstfd refer to the same file */ + if (src_statsbuf.st_dev == dst_statsbuf.st_dev && + src_statsbuf.st_ino == dst_statsbuf.st_ino) { + goto out; + } + + /* Truncate the file in case the destination already existed. */ + if (ftruncate(dstfd, 0) != 0) { + err = UV__ERR(errno); + goto out; + } } if (fchmod(dstfd, src_statsbuf.st_mode) == -1) { @@ -2046,7 +2056,7 @@ void uv_fs_req_cleanup(uv_fs_t* req) { /* Only necessary for asychronous requests, i.e., requests with a callback. * Synchronous ones don't copy their arguments and have req->path and - * req->new_path pointing to user-owned memory. UV_FS_MKDTEMP and + * req->new_path pointing to user-owned memory. UV_FS_MKDTEMP and * UV_FS_MKSTEMP are the exception to the rule, they always allocate memory. */ if (req->path != NULL && diff --git a/test/test-fs-copyfile.c b/test/test-fs-copyfile.c index 3335c881..e6f06e6e 100644 --- a/test/test-fs-copyfile.c +++ b/test/test-fs-copyfile.c @@ -125,6 +125,11 @@ TEST_IMPL(fs_copyfile) { r = uv_fs_copyfile(NULL, &req, src, src, 0, NULL); ASSERT(r == 0); uv_fs_req_cleanup(&req); + /* Verify that the src file did not get truncated. */ + r = uv_fs_stat(NULL, &req, src, NULL); + ASSERT_EQ(r, 0); + ASSERT_EQ(req.statbuf.st_size, 12); + uv_fs_req_cleanup(&req); unlink(src); /* Copies file synchronously. Creates new file. */