Skip to content

Commit

Permalink
feat: ignore ENOENT errors during chown
Browse files Browse the repository at this point in the history
Some files might be deleted during chownr. This commit ignore ENOENT
errors to tolerate such cases to mimic 'chown -R'.

Fixes npm/cli#496
  • Loading branch information
raymondfeng committed Feb 4, 2020
1 parent deaa058 commit 5bbda8c
Show file tree
Hide file tree
Showing 3 changed files with 323 additions and 10 deletions.
56 changes: 46 additions & 10 deletions chownr.js
Expand Up @@ -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 => {
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
}
}

Expand All @@ -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
Expand All @@ -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))
Expand All @@ -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())
Expand All @@ -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
}

Expand Down
134 changes: 134 additions & 0 deletions 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()
})
})
}

0 comments on commit 5bbda8c

Please sign in to comment.