Skip to content

Commit

Permalink
unix: handle src, dest same in uv_fs_copyfile()
Browse files Browse the repository at this point in the history
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: #2298
Refs: nodejs/node#34624
PR-URL: #2947
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jameson Nash <vtjnash@gmail.com>
  • Loading branch information
cjihrig committed Aug 9, 2020
1 parent b2cec84 commit 278cfa0
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 11 deletions.
32 changes: 21 additions & 11 deletions src/unix/fs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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 &&
Expand Down
5 changes: 5 additions & 0 deletions test/test-fs-copyfile.c
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down

0 comments on commit 278cfa0

Please sign in to comment.