From 5bbda8cba1c955dbb0ff4da573254e207f8775ca Mon Sep 17 00:00:00 2001
From: Raymond Feng
Date: Tue, 28 Jan 2020 15:57:11 -0800
Subject: [PATCH] feat: ignore ENOENT errors during chown
Some files might be deleted during chownr. This commit ignore ENOENT
errors to tolerate such cases to mimic 'chown -R'.
Fixes https://github.com/npm/cli/issues/496
---
chownr.js | 56 +++++++++++++---
test/concurrent-sync.js | 134 +++++++++++++++++++++++++++++++++++++
test/concurrent.js | 143 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 323 insertions(+), 10 deletions(-)
create mode 100644 test/concurrent-sync.js
create mode 100644 test/concurrent.js
diff --git a/chownr.js b/chownr.js
index 9f04393..1f84a33 100644
--- a/chownr.js
+++ b/chownr.js
@@ -11,6 +11,24 @@ const needEISDIRHandled = fs.lchown &&
!process.version.match(/v1[1-9]+\./) &&
!process.version.match(/v10\.[6-9]/)
+const lchownSync = (path, uid, gid) => {
+ try {
+ return fs[LCHOWNSYNC](path, uid, gid)
+ } catch (er) {
+ if (er.code !== 'ENOENT')
+ throw er
+ }
+}
+
+const chownSync = (path, uid, gid) => {
+ try {
+ return fs.chownSync(path, uid, gid)
+ } catch (er) {
+ if (er.code !== 'ENOENT')
+ throw er
+ }
+}
+
/* istanbul ignore next */
const handleEISDIR =
needEISDIRHandled ? (path, uid, gid, cb) => er => {
@@ -28,14 +46,14 @@ const handleEISDIR =
const handleEISDirSync =
needEISDIRHandled ? (path, uid, gid) => {
try {
- return fs[LCHOWNSYNC](path, uid, gid)
+ return lchownSync(path, uid, gid)
} catch (er) {
if (er.code !== 'EISDIR')
throw er
- fs.chownSync(path, uid, gid)
+ chownSync(path, uid, gid)
}
}
- : (path, uid, gid) => fs[LCHOWNSYNC](path, uid, gid)
+ : (path, uid, gid) => lchownSync(path, uid, gid)
// fs.readdir could only accept an options object as of node v6
const nodeVersion = process.version
@@ -45,9 +63,19 @@ let readdirSync = (path, options) => fs.readdirSync(path, options)
if (/^v4\./.test(nodeVersion))
readdir = (path, options, cb) => fs.readdir(path, cb)
+const chown = (cpath, uid, gid, cb) => {
+ fs[LCHOWN](cpath, uid, gid, handleEISDIR(cpath, uid, gid, er => {
+ // Skip ENOENT error
+ if (er && er.code === 'ENOENT') return cb()
+ cb(er)
+ }))
+}
+
const chownrKid = (p, child, uid, gid, cb) => {
if (typeof child === 'string')
return fs.lstat(path.resolve(p, child), (er, stats) => {
+ // Skip ENOENT error
+ if (er && er.code === 'ENOENT') return cb()
if (er)
return cb(er)
stats.name = child
@@ -59,11 +87,11 @@ const chownrKid = (p, child, uid, gid, cb) => {
if (er)
return cb(er)
const cpath = path.resolve(p, child.name)
- fs[LCHOWN](cpath, uid, gid, handleEISDIR(cpath, uid, gid, cb))
+ chown(cpath, uid, gid, cb)
})
} else {
const cpath = path.resolve(p, child.name)
- fs[LCHOWN](cpath, uid, gid, handleEISDIR(cpath, uid, gid, cb))
+ chown(cpath, uid, gid, cb)
}
}
@@ -74,8 +102,10 @@ const chownr = (p, uid, gid, cb) => {
// or doesn't exist. give up.
if (er && er.code !== 'ENOTDIR' && er.code !== 'ENOTSUP')
return cb(er)
+ if (er && er.code === 'ENOENT')
+ return cb()
if (er || !children.length)
- return fs[LCHOWN](p, uid, gid, handleEISDIR(p, uid, gid, cb))
+ return chown(p, uid, gid, cb)
let len = children.length
let errState = null
@@ -85,7 +115,7 @@ const chownr = (p, uid, gid, cb) => {
if (er)
return cb(errState = er)
if (-- len === 0)
- return fs[LCHOWN](p, uid, gid, handleEISDIR(p, uid, gid, cb))
+ return chown(p, uid, gid, cb)
}
children.forEach(child => chownrKid(p, child, uid, gid, then))
@@ -94,9 +124,14 @@ const chownr = (p, uid, gid, cb) => {
const chownrKidSync = (p, child, uid, gid) => {
if (typeof child === 'string') {
- const stats = fs.lstatSync(path.resolve(p, child))
- stats.name = child
- child = stats
+ try {
+ const stats = fs.lstatSync(path.resolve(p, child))
+ stats.name = child
+ child = stats
+ } catch (er) {
+ if (er.code === 'ENOENT') return
+ throw er;
+ }
}
if (child.isDirectory())
@@ -112,6 +147,7 @@ const chownrSync = (p, uid, gid) => {
} catch (er) {
if (er && er.code === 'ENOTDIR' && er.code !== 'ENOTSUP')
return handleEISDirSync(p, uid, gid)
+ if (er && er.code === 'ENOENT') return
throw er
}
diff --git a/test/concurrent-sync.js b/test/concurrent-sync.js
new file mode 100644
index 0000000..16d63eb
--- /dev/null
+++ b/test/concurrent-sync.js
@@ -0,0 +1,134 @@
+if (!process.getuid || !process.getgid) {
+ throw new Error("Tests require getuid/getgid support")
+}
+
+var curUid = +process.getuid()
+, curGid = +process.getgid()
+, test = require("tap").test
+, mkdirp = require("mkdirp")
+, rimraf = require("rimraf")
+, fs = require("fs")
+, path = require("path")
+
+// sniff the 'id' command for other groups that i can legally assign to
+var exec = require("child_process").exec
+, groups
+, dirs = []
+, files = []
+
+// Monkey-patch fs.readdirSync to remove f1 before the callback happens
+const readdirSync = fs.readdirSync
+
+var chownr = require("../")
+
+exec("id", function (code, output) {
+ if (code) throw new Error("failed to run 'id' command")
+ groups = output.trim().split("=")[3].split(",").map(function (s) {
+ return parseInt(s, 10)
+ }).filter(function (g) {
+ return g !== curGid
+ })
+
+ // console.error([curUid, groups[0]], "uid, gid")
+
+ rimraf("/tmp/chownr", function (er) {
+ if (er) throw er
+ var cnt = 5
+ for (var i = 0; i < 5; i ++) {
+ mkdirp(getDir(), then)
+ }
+ function then (er, made) {
+ if (er) throw er
+ var f1 = path.join(made, "f1");
+ files.push(f1)
+ fs.writeFileSync(f1, "file-1");
+ var f2 = path.join(made, "f2");
+ files.push(f2)
+ fs.writeFileSync(f2, "file-2");
+ if (-- cnt === 0) {
+ runTest()
+ }
+ }
+ })
+})
+
+function getDir () {
+ var dir = "/tmp/chownr"
+
+ dir += "/" + Math.floor(Math.random() * Math.pow(16,4)).toString(16)
+ dirs.push(dir)
+ dir += "/" + Math.floor(Math.random() * Math.pow(16,4)).toString(16)
+ dirs.push(dir)
+ dir += "/" + Math.floor(Math.random() * Math.pow(16,4)).toString(16)
+ dirs.push(dir)
+ return dir
+}
+
+function runTest () {
+ test("patch fs.readdirSync", function (t) {
+ // Monkey-patch fs.readdirSync to remove f1 before returns
+ // This simulates the case where some files are deleted when chownr.sync
+ // is in progress
+ fs.readdirSync = function () {
+ const args = [].slice.call(arguments)
+ const dir = args[0]
+ const children = readdirSync.apply(fs, args);
+ try {
+ fs.unlinkSync(path.join(dir, 'f1'))
+ } catch (er) {
+ if (er.code !== 'ENOENT') throw er
+ }
+ return children
+ }
+ t.end()
+ })
+
+ test("should complete successfully", function (t) {
+ // console.error("calling chownr", curUid, groups[0], typeof curUid, typeof groups[0])
+ chownr.sync("/tmp/chownr", curUid, groups[0])
+ t.end()
+ })
+
+ test("restore fs.readdirSync", function (t) {
+ // Restore fs.readdirSync
+ fs.readdirSync = readdirSync
+ t.end()
+ })
+
+ dirs.forEach(function (dir) {
+ test("verify "+dir, function (t) {
+ fs.stat(dir, function (er, st) {
+ if (er) {
+ t.ifError(er)
+ return t.end()
+ }
+ t.equal(st.uid, curUid, "uid should be " + curUid)
+ t.equal(st.gid, groups[0], "gid should be "+groups[0])
+ t.end()
+ })
+ })
+ })
+
+ files.forEach(function (f) {
+ test("verify "+f, function (t) {
+ fs.stat(f, function (er, st) {
+ if (er) {
+ if (er.code !== 'ENOENT')
+ t.ifError(er)
+ return t.end()
+ }
+ t.equal(st.uid, curUid, "uid should be " + curUid)
+ t.equal(st.gid, groups[0], "gid should be "+groups[0])
+ t.end()
+ })
+ })
+ })
+
+ test("cleanup", function (t) {
+ rimraf("/tmp/chownr/", function (er) {
+ t.ifError(er)
+ t.end()
+ })
+ })
+}
+
diff --git a/test/concurrent.js b/test/concurrent.js
new file mode 100644
index 0000000..935caa7
--- /dev/null
+++ b/test/concurrent.js
@@ -0,0 +1,143 @@
+if (!process.getuid || !process.getgid) {
+ throw new Error("Tests require getuid/getgid support")
+}
+
+var curUid = +process.getuid()
+, curGid = +process.getgid()
+, test = require("tap").test
+, mkdirp = require("mkdirp")
+, rimraf = require("rimraf")
+, fs = require("fs")
+, path = require("path")
+
+// sniff the 'id' command for other groups that i can legally assign to
+var exec = require("child_process").exec
+, groups
+, dirs = []
+, files = []
+
+// Monkey-patch fs.readdir to remove f1 before the callback happens
+const readdir = fs.readdir
+
+var chownr = require("../")
+
+exec("id", function (code, output) {
+ if (code) throw new Error("failed to run 'id' command")
+ groups = output.trim().split("=")[3].split(",").map(function (s) {
+ return parseInt(s, 10)
+ }).filter(function (g) {
+ return g !== curGid
+ })
+
+ // console.error([curUid, groups[0]], "uid, gid")
+
+ rimraf("/tmp/chownr", function (er) {
+ if (er) throw er
+ var cnt = 5
+ for (var i = 0; i < 5; i ++) {
+ mkdirp(getDir(), then)
+ }
+ function then (er, made) {
+ if (er) throw er
+ var f1 = path.join(made, "f1");
+ files.push(f1)
+ fs.writeFile(f1, "file-1", er => {
+ if (er) throw er
+ var f2 = path.join(made, "f2");
+ files.push(f2)
+ fs.writeFile(f2, "file-2", er => {
+ if (er) throw er
+ if (-- cnt === 0) {
+ runTest()
+ }
+ })
+ })
+ }
+ })
+})
+
+function getDir () {
+ var dir = "/tmp/chownr"
+
+ dir += "/" + Math.floor(Math.random() * Math.pow(16,4)).toString(16)
+ dirs.push(dir)
+ dir += "/" + Math.floor(Math.random() * Math.pow(16,4)).toString(16)
+ dirs.push(dir)
+ dir += "/" + Math.floor(Math.random() * Math.pow(16,4)).toString(16)
+ dirs.push(dir)
+ return dir
+}
+
+function runTest () {
+ test("patch fs.readdir", function (t) {
+ // Monkey-patch fs.readdir to remove f1 before the callback happens
+ // This simulates the case where some files are deleted when chownr
+ // is in progress asynchronously
+ fs.readdir = function () {
+ const args = [].slice.call(arguments)
+ const cb = args.pop()
+ const dir = args[0]
+ args.push((er, children) => {
+ if (er) return cb(er)
+ fs.unlink(path.join(dir, 'f1'), er => {
+ if (er && er.code === 'ENOENT') return cb(null, children)
+ cb(er, children)
+ });
+ });
+ readdir.apply(fs, args);
+ }
+ t.end()
+ })
+
+ test("should complete successfully", function (t) {
+ // console.error("calling chownr", curUid, groups[0], typeof curUid, typeof groups[0])
+ chownr("/tmp/chownr", curUid, groups[0], function (er) {
+ t.ifError(er)
+ t.end()
+ })
+ })
+
+
+ test("restore fs.readdir", function (t) {
+ // Restore fs.readdir
+ fs.readdir = readdir
+ t.end()
+ })
+
+ dirs.forEach(function (dir) {
+ test("verify "+dir, function (t) {
+ fs.stat(dir, function (er, st) {
+ if (er) {
+ t.ifError(er)
+ return t.end()
+ }
+ t.equal(st.uid, curUid, "uid should be " + curUid)
+ t.equal(st.gid, groups[0], "gid should be "+groups[0])
+ t.end()
+ })
+ })
+ })
+
+ files.forEach(function (f) {
+ test("verify "+f, function (t) {
+ fs.stat(f, function (er, st) {
+ if (er) {
+ if (er.code !== 'ENOENT')
+ t.ifError(er)
+ return t.end()
+ }
+ t.equal(st.uid, curUid, "uid should be " + curUid)
+ t.equal(st.gid, groups[0], "gid should be "+groups[0])
+ t.end()
+ })
+ })
+ })
+
+ test("cleanup", function (t) {
+ rimraf("/tmp/chownr/", function (er) {
+ t.ifError(er)
+ t.end()
+ })
+ })
+}
+