Skip to content

Commit 5bbda8c

Browse files
raymondfengraymondfeng
committed
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 npm/cli#496
1 parent deaa058 commit 5bbda8c

File tree

3 files changed

+323
-10
lines changed

3 files changed

+323
-10
lines changed

‎chownr.js‎

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,24 @@ const needEISDIRHandled = fs.lchown &&
1111
!process.version.match(/v1[1-9]+\./) &&
1212
!process.version.match(/v10\.[6-9]/)
1313

14+
const lchownSync = (path, uid, gid) => {
15+
try {
16+
return fs[LCHOWNSYNC](path, uid, gid)
17+
} catch (er) {
18+
if (er.code !== 'ENOENT')
19+
throw er
20+
}
21+
}
22+
23+
const chownSync = (path, uid, gid) => {
24+
try {
25+
return fs.chownSync(path, uid, gid)
26+
} catch (er) {
27+
if (er.code !== 'ENOENT')
28+
throw er
29+
}
30+
}
31+
1432
/* istanbul ignore next */
1533
const handleEISDIR =
1634
needEISDIRHandled ? (path, uid, gid, cb) => er => {
@@ -28,14 +46,14 @@ const handleEISDIR =
2846
const handleEISDirSync =
2947
needEISDIRHandled ? (path, uid, gid) => {
3048
try {
31-
return fs[LCHOWNSYNC](path, uid, gid)
49+
return lchownSync(path, uid, gid)
3250
} catch (er) {
3351
if (er.code !== 'EISDIR')
3452
throw er
35-
fs.chownSync(path, uid, gid)
53+
chownSync(path, uid, gid)
3654
}
3755
}
38-
: (path, uid, gid) => fs[LCHOWNSYNC](path, uid, gid)
56+
: (path, uid, gid) => lchownSync(path, uid, gid)
3957

4058
// fs.readdir could only accept an options object as of node v6
4159
const nodeVersion = process.version
@@ -45,9 +63,19 @@ let readdirSync = (path, options) => fs.readdirSync(path, options)
4563
if (/^v4\./.test(nodeVersion))
4664
readdir = (path, options, cb) => fs.readdir(path, cb)
4765

66+
const chown = (cpath, uid, gid, cb) => {
67+
fs[LCHOWN](cpath, uid, gid, handleEISDIR(cpath, uid, gid, er => {
68+
// Skip ENOENT error
69+
if (er && er.code === 'ENOENT') return cb()
70+
cb(er)
71+
}))
72+
}
73+
4874
const chownrKid = (p, child, uid, gid, cb) => {
4975
if (typeof child === 'string')
5076
return fs.lstat(path.resolve(p, child), (er, stats) => {
77+
// Skip ENOENT error
78+
if (er && er.code === 'ENOENT') return cb()
5179
if (er)
5280
return cb(er)
5381
stats.name = child
@@ -59,11 +87,11 @@ const chownrKid = (p, child, uid, gid, cb) => {
5987
if (er)
6088
return cb(er)
6189
const cpath = path.resolve(p, child.name)
62-
fs[LCHOWN](cpath, uid, gid, handleEISDIR(cpath, uid, gid, cb))
90+
chown(cpath, uid, gid, cb)
6391
})
6492
} else {
6593
const cpath = path.resolve(p, child.name)
66-
fs[LCHOWN](cpath, uid, gid, handleEISDIR(cpath, uid, gid, cb))
94+
chown(cpath, uid, gid, cb)
6795
}
6896
}
6997

@@ -74,8 +102,10 @@ const chownr = (p, uid, gid, cb) => {
74102
// or doesn't exist. give up.
75103
if (er && er.code !== 'ENOTDIR' && er.code !== 'ENOTSUP')
76104
return cb(er)
105+
if (er && er.code === 'ENOENT')
106+
return cb()
77107
if (er || !children.length)
78-
return fs[LCHOWN](p, uid, gid, handleEISDIR(p, uid, gid, cb))
108+
return chown(p, uid, gid, cb)
79109

80110
let len = children.length
81111
let errState = null
@@ -85,7 +115,7 @@ const chownr = (p, uid, gid, cb) => {
85115
if (er)
86116
return cb(errState = er)
87117
if (-- len === 0)
88-
return fs[LCHOWN](p, uid, gid, handleEISDIR(p, uid, gid, cb))
118+
return chown(p, uid, gid, cb)
89119
}
90120

91121
children.forEach(child => chownrKid(p, child, uid, gid, then))
@@ -94,9 +124,14 @@ const chownr = (p, uid, gid, cb) => {
94124

95125
const chownrKidSync = (p, child, uid, gid) => {
96126
if (typeof child === 'string') {
97-
const stats = fs.lstatSync(path.resolve(p, child))
98-
stats.name = child
99-
child = stats
127+
try {
128+
const stats = fs.lstatSync(path.resolve(p, child))
129+
stats.name = child
130+
child = stats
131+
} catch (er) {
132+
if (er.code === 'ENOENT') return
133+
throw er;
134+
}
100135
}
101136

102137
if (child.isDirectory())
@@ -112,6 +147,7 @@ const chownrSync = (p, uid, gid) => {
112147
} catch (er) {
113148
if (er && er.code === 'ENOTDIR' && er.code !== 'ENOTSUP')
114149
return handleEISDirSync(p, uid, gid)
150+
if (er && er.code === 'ENOENT') return
115151
throw er
116152
}
117153

‎test/concurrent-sync.js‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
if (!process.getuid || !process.getgid) {
2+
throw new Error("Tests require getuid/getgid support")
3+
}
4+
5+
var curUid = +process.getuid()
6+
, curGid = +process.getgid()
7+
, test = require("tap").test
8+
, mkdirp = require("mkdirp")
9+
, rimraf = require("rimraf")
10+
, fs = require("fs")
11+
, path = require("path")
12+
13+
// sniff the 'id' command for other groups that i can legally assign to
14+
var exec = require("child_process").exec
15+
, groups
16+
, dirs = []
17+
, files = []
18+
19+
// Monkey-patch fs.readdirSync to remove f1 before the callback happens
20+
const readdirSync = fs.readdirSync
21+
22+
var chownr = require("../")
23+
24+
exec("id", function (code, output) {
25+
if (code) throw new Error("failed to run 'id' command")
26+
groups = output.trim().split("=")[3].split(",").map(function (s) {
27+
return parseInt(s, 10)
28+
}).filter(function (g) {
29+
return g !== curGid
30+
})
31+
32+
// console.error([curUid, groups[0]], "uid, gid")
33+
34+
rimraf("/tmp/chownr", function (er) {
35+
if (er) throw er
36+
var cnt = 5
37+
for (var i = 0; i < 5; i ++) {
38+
mkdirp(getDir(), then)
39+
}
40+
function then (er, made) {
41+
if (er) throw er
42+
var f1 = path.join(made, "f1");
43+
files.push(f1)
44+
fs.writeFileSync(f1, "file-1");
45+
var f2 = path.join(made, "f2");
46+
files.push(f2)
47+
fs.writeFileSync(f2, "file-2");
48+
if (-- cnt === 0) {
49+
runTest()
50+
}
51+
}
52+
})
53+
})
54+
55+
function getDir () {
56+
var dir = "/tmp/chownr"
57+
58+
dir += "/" + Math.floor(Math.random() * Math.pow(16,4)).toString(16)
59+
dirs.push(dir)
60+
dir += "/" + Math.floor(Math.random() * Math.pow(16,4)).toString(16)
61+
dirs.push(dir)
62+
dir += "/" + Math.floor(Math.random() * Math.pow(16,4)).toString(16)
63+
dirs.push(dir)
64+
return dir
65+
}
66+
67+
function runTest () {
68+
test("patch fs.readdirSync", function (t) {
69+
// Monkey-patch fs.readdirSync to remove f1 before returns
70+
// This simulates the case where some files are deleted when chownr.sync
71+
// is in progress
72+
fs.readdirSync = function () {
73+
const args = [].slice.call(arguments)
74+
const dir = args[0]
75+
const children = readdirSync.apply(fs, args);
76+
try {
77+
fs.unlinkSync(path.join(dir, 'f1'))
78+
} catch (er) {
79+
if (er.code !== 'ENOENT') throw er
80+
}
81+
return children
82+
}
83+
t.end()
84+
})
85+
86+
test("should complete successfully", function (t) {
87+
// console.error("calling chownr", curUid, groups[0], typeof curUid, typeof groups[0])
88+
chownr.sync("/tmp/chownr", curUid, groups[0])
89+
t.end()
90+
})
91+
92+
test("restore fs.readdirSync", function (t) {
93+
// Restore fs.readdirSync
94+
fs.readdirSync = readdirSync
95+
t.end()
96+
})
97+
98+
dirs.forEach(function (dir) {
99+
test("verify "+dir, function (t) {
100+
fs.stat(dir, function (er, st) {
101+
if (er) {
102+
t.ifError(er)
103+
return t.end()
104+
}
105+
t.equal(st.uid, curUid, "uid should be " + curUid)
106+
t.equal(st.gid, groups[0], "gid should be "+groups[0])
107+
t.end()
108+
})
109+
})
110+
})
111+
112+
files.forEach(function (f) {
113+
test("verify "+f, function (t) {
114+
fs.stat(f, function (er, st) {
115+
if (er) {
116+
if (er.code !== 'ENOENT')
117+
t.ifError(er)
118+
return t.end()
119+
}
120+
t.equal(st.uid, curUid, "uid should be " + curUid)
121+
t.equal(st.gid, groups[0], "gid should be "+groups[0])
122+
t.end()
123+
})
124+
})
125+
})
126+
127+
test("cleanup", function (t) {
128+
rimraf("/tmp/chownr/", function (er) {
129+
t.ifError(er)
130+
t.end()
131+
})
132+
})
133+
}
134+

0 commit comments

Comments
 (0)