From 1b8fceea3e873938a832206527a1f69fcd546178 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Tue, 26 Mar 2019 23:32:20 -0700 Subject: [PATCH 1/3] test: add known_issues test for fs.copyFile() On macOS, fs.copyFile() may not respect file permissions. There is a PR for libuv that should fix this, but until it lands and we can either float a patch or upgrade libuv, have a known_issues test. Ref: https://github.com/nodejs/node/issues/26936 Ref: https://github.com/libuv/libuv/pull/2233 --- test/known_issues/known_issues.status | 6 +++++ .../test-fs-copyfile-respect-permissions.js | 25 +++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 test/known_issues/test-fs-copyfile-respect-permissions.js diff --git a/test/known_issues/known_issues.status b/test/known_issues/known_issues.status index 3463f0a0ecfc6a..d7e0b546d43ff8 100644 --- a/test/known_issues/known_issues.status +++ b/test/known_issues/known_issues.status @@ -7,18 +7,24 @@ prefix known_issues [true] # This section applies to all platforms [$system==win32] +test-fs-copyfile-respect-permissions: SKIP [$system==linux] test-vm-timeout-escape-promise: PASS,FLAKY +test-fs-copyfile-respect-permissions: SKIP [$system==macos] [$system==solaris] +test-fs-copyfile-respect-permissions: SKIP [$system==freebsd] +test-fs-copyfile-respect-permissions: SKIP [$system==aix] +test-fs-copyfile-respect-permissions: SKIP [$arch==arm] # https://github.com/nodejs/node/issues/24120 test-vm-timeout-escape-nexttick: PASS,FLAKY +test-fs-copyfile-respect-permissions: SKIP diff --git a/test/known_issues/test-fs-copyfile-respect-permissions.js b/test/known_issues/test-fs-copyfile-respect-permissions.js new file mode 100644 index 00000000000000..42feb6cf5e59cb --- /dev/null +++ b/test/known_issues/test-fs-copyfile-respect-permissions.js @@ -0,0 +1,25 @@ +'use strict'; + +// Test that fs.copyFile() respects file permissions. +// Ref: https://github.com/nodejs/node/issues/26936 + +require('../common'); + +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); + +const source = path.join(tmpdir.path, 'source'); +const dest = path.join(tmpdir.path, 'dest'); + +fs.writeFileSync(source, 'source'); +fs.writeFileSync(dest, 'dest'); +fs.chmodSync(dest, '444'); + +fs.copyFile(source, dest, (err) => { + assert.strictEqual(err.code, 'EACCESS'); + assert.strictEqual(fs.readFileSync(dest, 'utf8'), 'dest'); +}); From c84a7320346f896b5a64616177d8d508330432a6 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 29 Mar 2019 10:36:42 -0700 Subject: [PATCH 2/3] fixup! test: add known_issues test for fs.copyFile() --- .../known_issues/test-fs-copyfile-respect-permissions.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/known_issues/test-fs-copyfile-respect-permissions.js b/test/known_issues/test-fs-copyfile-respect-permissions.js index 42feb6cf5e59cb..26b6ce03c8be16 100644 --- a/test/known_issues/test-fs-copyfile-respect-permissions.js +++ b/test/known_issues/test-fs-copyfile-respect-permissions.js @@ -3,7 +3,7 @@ // Test that fs.copyFile() respects file permissions. // Ref: https://github.com/nodejs/node/issues/26936 -require('../common'); +const common = require('../common'); const tmpdir = require('../common/tmpdir'); tmpdir.refresh(); @@ -19,7 +19,10 @@ fs.writeFileSync(source, 'source'); fs.writeFileSync(dest, 'dest'); fs.chmodSync(dest, '444'); -fs.copyFile(source, dest, (err) => { +assert.throws(() => { fs.copyFileSync(source, dest); }, + { code: 'EACCESS' }); + +fs.copyFile(source, dest, common.mustCall((err) => { assert.strictEqual(err.code, 'EACCESS'); assert.strictEqual(fs.readFileSync(dest, 'utf8'), 'dest'); -}); +})); From d15753a09333043db9af34ea3f119ce62eaa94d1 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 29 Mar 2019 11:22:56 -0700 Subject: [PATCH 3/3] fixup! fixup! test: add known_issues test for fs.copyFile() --- .../test-fs-copyfile-respect-permissions.js | 50 +++++++++++++------ 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/test/known_issues/test-fs-copyfile-respect-permissions.js b/test/known_issues/test-fs-copyfile-respect-permissions.js index 26b6ce03c8be16..0ebc5fbfc1dbed 100644 --- a/test/known_issues/test-fs-copyfile-respect-permissions.js +++ b/test/known_issues/test-fs-copyfile-respect-permissions.js @@ -12,17 +12,39 @@ const assert = require('assert'); const fs = require('fs'); const path = require('path'); -const source = path.join(tmpdir.path, 'source'); -const dest = path.join(tmpdir.path, 'dest'); - -fs.writeFileSync(source, 'source'); -fs.writeFileSync(dest, 'dest'); -fs.chmodSync(dest, '444'); - -assert.throws(() => { fs.copyFileSync(source, dest); }, - { code: 'EACCESS' }); - -fs.copyFile(source, dest, common.mustCall((err) => { - assert.strictEqual(err.code, 'EACCESS'); - assert.strictEqual(fs.readFileSync(dest, 'utf8'), 'dest'); -})); +let n = 0; + +function beforeEach() { + n++; + const source = path.join(tmpdir.path, `source${n}`); + const dest = path.join(tmpdir.path, `dest${n}`); + fs.writeFileSync(source, 'source'); + fs.writeFileSync(dest, 'dest'); + fs.chmodSync(dest, '444'); + + const check = (err) => { + assert.strictEqual(err.code, 'EACCESS'); + assert.strictEqual(fs.readFileSync(dest, 'utf8'), 'dest'); + }; + + return { source, dest, check }; +} + +// Test synchronous API. +{ + const { source, dest, check } = beforeEach(); + assert.throws(() => { fs.copyFileSync(source, dest); }, check); +} + +// Test promises API. +{ + const { source, dest, check } = beforeEach(); + assert.throws(async () => { await fs.promises.copyFile(source, dest); }, + check); +} + +// Test callback API. +{ + const { source, dest, check } = beforeEach(); + fs.copyFile(source, dest, common.mustCall(check)); +}