From 383a3f8b708c3093418e4ef341865c02585c012c Mon Sep 17 00:00:00 2001
From: Tetsuharu Ohzeki <tetsuharu.ohzeki@gmail.com>
Date: Thu, 20 Apr 2023 15:28:18 +0900
Subject: [PATCH] fs: add support for mode flag to specify the copy behavior

`fs.copyFile()` supports copy-on-write operation
if the underlying platform supports it by passing a mode flag.
This behavior was added in
https://github.com/nodejs/node/commit/a16d88d9e9a6581e463082549823189aba25ca76.

This patch adds `mode` flag to `fs.cp()`, `fs.cpSync()`,
and `fsPromises.cp()` to allow to change their behaviors
to copy files.

This test case is based on the test case that was introduced
when we add `fs.constants.COPYFILE_FICLONE`.
https://github.com/nodejs/node/commit/a16d88d9e9a6581e463082549823189aba25ca76.

This test strategy is:

- If the platform supports copy-on-write operation,
  check whether the destination is expected
- Otherwise, the operation will fail
  and check whether the failure error information is expected.

Fixes: https://github.com/nodejs/node/issues/47080
PR-URL: https://github.com/nodejs/node/pull/47084
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
---
 doc/api/fs.md                 |  22 ++++++-
 lib/internal/fs/cp/cp-sync.js |   2 +-
 lib/internal/fs/cp/cp.js      |   2 +-
 lib/internal/fs/utils.js      |   1 +
 test/parallel/test-fs-cp.mjs  | 104 ++++++++++++++++++++++++++++++++++
 5 files changed, 128 insertions(+), 3 deletions(-)

diff --git a/doc/api/fs.md b/doc/api/fs.md
index c3b4fc54734728..8daaf8ee9685ca 100644
--- a/doc/api/fs.md
+++ b/doc/api/fs.md
@@ -962,6 +962,10 @@ try {
 <!-- YAML
 added: v16.7.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/47084
+    description: Accept an additional `mode` option to specify
+                 the copy behavior as the `mode` argument of `fs.copyFile()`.
   - version: v17.6.0
     pr-url: https://github.com/nodejs/node/pull/41819
     description: Accepts an additional `verbatimSymlinks` option to specify
@@ -986,6 +990,8 @@ changes:
     operation will ignore errors if you set this to false and the destination
     exists. Use the `errorOnExist` option to change this behavior.
     **Default:** `true`.
+  * `mode` {integer} modifiers for copy operation. **Default:** `0`.
+    See `mode` flag of [`fsPromises.copyFile()`][].
   * `preserveTimestamps` {boolean} When `true` timestamps from `src` will
     be preserved. **Default:** `false`.
   * `recursive` {boolean} copy directories recursively **Default:** `false`
@@ -2269,6 +2275,10 @@ copyFile('source.txt', 'destination.txt', constants.COPYFILE_EXCL, callback);
 <!-- YAML
 added: v16.7.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/47084
+    description: Accept an additional `mode` option to specify
+                 the copy behavior as the `mode` argument of `fs.copyFile()`.
   - version: v18.0.0
     pr-url: https://github.com/nodejs/node/pull/41678
     description: Passing an invalid callback to the `callback` argument
@@ -2298,6 +2308,8 @@ changes:
     operation will ignore errors if you set this to false and the destination
     exists. Use the `errorOnExist` option to change this behavior.
     **Default:** `true`.
+  * `mode` {integer} modifiers for copy operation. **Default:** `0`.
+    See `mode` flag of [`fs.copyFile()`][].
   * `preserveTimestamps` {boolean} When `true` timestamps from `src` will
     be preserved. **Default:** `false`.
   * `recursive` {boolean} copy directories recursively **Default:** `false`
@@ -5120,7 +5132,11 @@ copyFileSync('source.txt', 'destination.txt', constants.COPYFILE_EXCL);
 <!-- YAML
 added: v16.7.0
 changes:
-  - version: v17.6.0
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/47084
+    description: Accept an additional `mode` option to specify
+                 the copy behavior as the `mode` argument of `fs.copyFile()`.
+  - version: v17.6.0  
     pr-url: https://github.com/nodejs/node/pull/41819
     description: Accepts an additional `verbatimSymlinks` option to specify
                  whether to perform path resolution for symlinks.
@@ -5143,6 +5159,8 @@ changes:
     operation will ignore errors if you set this to false and the destination
     exists. Use the `errorOnExist` option to change this behavior.
     **Default:** `true`.
+  * `mode` {integer} modifiers for copy operation. **Default:** `0`.
+    See `mode` flag of [`fs.copyFileSync()`][].
   * `preserveTimestamps` {boolean} When `true` timestamps from `src` will
     be preserved. **Default:** `false`.
   * `recursive` {boolean} copy directories recursively **Default:** `false`
@@ -7907,6 +7925,7 @@ the file contents.
 [`fs.chmod()`]: #fschmodpath-mode-callback
 [`fs.chown()`]: #fschownpath-uid-gid-callback
 [`fs.copyFile()`]: #fscopyfilesrc-dest-mode-callback
+[`fs.copyFileSync()`]: #fscopyfilesyncsrc-dest-mode
 [`fs.createReadStream()`]: #fscreatereadstreampath-options
 [`fs.createWriteStream()`]: #fscreatewritestreampath-options
 [`fs.exists()`]: #fsexistspath-callback
@@ -7940,6 +7959,7 @@ the file contents.
 [`fs.writeFile()`]: #fswritefilefile-data-options-callback
 [`fs.writev()`]: #fswritevfd-buffers-position-callback
 [`fsPromises.access()`]: #fspromisesaccesspath-mode
+[`fsPromises.copyFile()`]: #fspromisescopyfilesrc-dest-mode
 [`fsPromises.open()`]: #fspromisesopenpath-flags-mode
 [`fsPromises.opendir()`]: #fspromisesopendirpath-options
 [`fsPromises.rm()`]: #fspromisesrmpath-options
diff --git a/lib/internal/fs/cp/cp-sync.js b/lib/internal/fs/cp/cp-sync.js
index cfd54a4ff0e23f..348d45adcd7c4e 100644
--- a/lib/internal/fs/cp/cp-sync.js
+++ b/lib/internal/fs/cp/cp-sync.js
@@ -226,7 +226,7 @@ function mayCopyFile(srcStat, src, dest, opts) {
 }
 
 function copyFile(srcStat, src, dest, opts) {
-  copyFileSync(src, dest);
+  copyFileSync(src, dest, opts.mode);
   if (opts.preserveTimestamps) handleTimestamps(srcStat.mode, src, dest);
   return setDestMode(dest, srcStat.mode);
 }
diff --git a/lib/internal/fs/cp/cp.js b/lib/internal/fs/cp/cp.js
index 97378051cc45c9..622e145c383671 100644
--- a/lib/internal/fs/cp/cp.js
+++ b/lib/internal/fs/cp/cp.js
@@ -257,7 +257,7 @@ async function mayCopyFile(srcStat, src, dest, opts) {
 }
 
 async function _copyFile(srcStat, src, dest, opts) {
-  await copyFile(src, dest);
+  await copyFile(src, dest, opts.mode);
   if (opts.preserveTimestamps) {
     return handleTimestampsAndMode(srcStat.mode, src, dest);
   }
diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js
index 6bf588263f11b1..7b88ae5574f024 100644
--- a/lib/internal/fs/utils.js
+++ b/lib/internal/fs/utils.js
@@ -774,6 +774,7 @@ const validateCpOptions = hideStackFrames((options) => {
   validateBoolean(options.preserveTimestamps, 'options.preserveTimestamps');
   validateBoolean(options.recursive, 'options.recursive');
   validateBoolean(options.verbatimSymlinks, 'options.verbatimSymlinks');
+  options.mode = getValidMode(options.mode, 'copyFile');
   if (options.dereference === true && options.verbatimSymlinks === true) {
     throw new ERR_INCOMPATIBLE_OPTION_PAIR('dereference', 'verbatimSymlinks');
   }
diff --git a/test/parallel/test-fs-cp.mjs b/test/parallel/test-fs-cp.mjs
index c4d6c4b737e371..8aedad334d2b24 100644
--- a/test/parallel/test-fs-cp.mjs
+++ b/test/parallel/test-fs-cp.mjs
@@ -38,6 +38,30 @@ function nextdir() {
   assertDirEquivalent(src, dest);
 }
 
+// It copies a nested folder structure with mode flags.
+// This test is based on fs.promises.copyFile() with `COPYFILE_FICLONE_FORCE`.
+(() => {
+  const src = './test/fixtures/copy/kitchen-sink';
+  const dest = nextdir();
+  try {
+    cpSync(src, dest, mustNotMutateObjectDeep({
+      recursive: true,
+      mode: fs.constants.COPYFILE_FICLONE_FORCE,
+    }));
+  } catch (err) {
+    // If the platform does not support `COPYFILE_FICLONE_FORCE` operation,
+    // it should enter this path.
+    assert.strictEqual(err.syscall, 'copyfile');
+    assert(err.code === 'ENOTSUP' || err.code === 'ENOTTY' ||
+      err.code === 'ENOSYS' || err.code === 'EXDEV');
+    return;
+  }
+
+  // If the platform support `COPYFILE_FICLONE_FORCE` operation,
+  // it should reach to here.
+  assertDirEquivalent(src, dest);
+})();
+
 // It does not throw errors when directory is copied over and force is false.
 {
   const src = nextdir();
@@ -107,6 +131,14 @@ function nextdir() {
     });
 }
 
+// It rejects if options.mode is invalid.
+{
+  assert.throws(
+    () => cpSync('a', 'b', { mode: -1 }),
+    { code: 'ERR_OUT_OF_RANGE' }
+  );
+}
+
 
 // It throws an error when both dereference and verbatimSymlinks are enabled.
 {
@@ -425,6 +457,31 @@ if (!isWindows) {
   }));
 }
 
+// It copies a nested folder structure with mode flags.
+// This test is based on fs.promises.copyFile() with `COPYFILE_FICLONE_FORCE`.
+{
+  const src = './test/fixtures/copy/kitchen-sink';
+  const dest = nextdir();
+  cp(src, dest, mustNotMutateObjectDeep({
+    recursive: true,
+    mode: fs.constants.COPYFILE_FICLONE_FORCE,
+  }), mustCall((err) => {
+    if (!err) {
+      // If the platform support `COPYFILE_FICLONE_FORCE` operation,
+      // it should reach to here.
+      assert.strictEqual(err, null);
+      assertDirEquivalent(src, dest);
+      return;
+    }
+
+    // If the platform does not support `COPYFILE_FICLONE_FORCE` operation,
+    // it should enter this path.
+    assert.strictEqual(err.syscall, 'copyfile');
+    assert(err.code === 'ENOTSUP' || err.code === 'ENOTTY' ||
+      err.code === 'ENOSYS' || err.code === 'EXDEV');
+  }));
+}
+
 // It does not throw errors when directory is copied over and force is false.
 {
   const src = nextdir();
@@ -799,6 +856,14 @@ if (!isWindows) {
   );
 }
 
+// It throws if options is not object.
+{
+  assert.throws(
+    () => cp('a', 'b', { mode: -1 }, () => {}),
+    { code: 'ERR_OUT_OF_RANGE' }
+  );
+}
+
 // Promises implementation of copy.
 
 // It copies a nested folder structure with files and folders.
@@ -810,6 +875,35 @@ if (!isWindows) {
   assertDirEquivalent(src, dest);
 }
 
+// It copies a nested folder structure with mode flags.
+// This test is based on fs.promises.copyFile() with `COPYFILE_FICLONE_FORCE`.
+{
+  const src = './test/fixtures/copy/kitchen-sink';
+  const dest = nextdir();
+  let p = null;
+  let successFiClone = false;
+  try {
+    p = await fs.promises.cp(src, dest, mustNotMutateObjectDeep({
+      recursive: true,
+      mode: fs.constants.COPYFILE_FICLONE_FORCE,
+    }));
+    successFiClone = true;
+  } catch (err) {
+    // If the platform does not support `COPYFILE_FICLONE_FORCE` operation,
+    // it should enter this path.
+    assert.strictEqual(err.syscall, 'copyfile');
+    assert(err.code === 'ENOTSUP' || err.code === 'ENOTTY' ||
+      err.code === 'ENOSYS' || err.code === 'EXDEV');
+  }
+
+  if (successFiClone) {
+    // If the platform support `COPYFILE_FICLONE_FORCE` operation,
+    // it should reach to here.
+    assert.strictEqual(p, undefined);
+    assertDirEquivalent(src, dest);
+  }
+}
+
 // It accepts file URL as src and dest.
 {
   const src = './test/fixtures/copy/kitchen-sink';
@@ -847,6 +941,16 @@ if (!isWindows) {
   );
 }
 
+// It rejects if options.mode is invalid.
+{
+  await assert.rejects(
+    fs.promises.cp('a', 'b', {
+      mode: -1,
+    }),
+    { code: 'ERR_OUT_OF_RANGE' }
+  );
+}
+
 function assertDirEquivalent(dir1, dir2) {
   const dir1Entries = [];
   collectEntries(dir1, dir1Entries);