Skip to content

Commit

Permalink
fs: cleanup fd lchown and lchownSync
Browse files Browse the repository at this point in the history
lchown and lchownSync were opening file descriptors without
closing them. Looks like it has been that way for 7 years.
Does anyone actually use these functions?

PR-URL: #18329
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
jasnell committed Jan 26, 2018
1 parent fa33f02 commit 082f952
Showing 1 changed file with 19 additions and 11 deletions.
30 changes: 19 additions & 11 deletions lib/fs.js
Expand Up @@ -1330,20 +1330,16 @@ if (constants.O_SYMLINK !== undefined) {
};

fs.lchmodSync = function(path, mode) {
var fd = fs.openSync(path, constants.O_WRONLY | constants.O_SYMLINK);
const fd = fs.openSync(path, constants.O_WRONLY | constants.O_SYMLINK);

// Prefer to return the chmod error, if one occurs,
// but still try to close, and report closing errors if they occur.
var ret;
let ret;
try {
ret = fs.fchmodSync(fd, mode);
} catch (err) {
try {
fs.closeSync(fd);
} catch (ignore) {}
throw err;
} finally {
fs.closeSync(fd);
}
fs.closeSync(fd);
return ret;
};
}
Expand Down Expand Up @@ -1381,13 +1377,25 @@ if (constants.O_SYMLINK !== undefined) {
callback(err);
return;
}
fs.fchown(fd, uid, gid, callback);
// Prefer to return the chown error, if one occurs,
// but still try to close, and report closing errors if they occur.
fs.fchown(fd, uid, gid, function(err) {
fs.close(fd, function(err2) {
callback(err || err2);
});
});
});
};

fs.lchownSync = function(path, uid, gid) {
var fd = fs.openSync(path, constants.O_WRONLY | constants.O_SYMLINK);
return fs.fchownSync(fd, uid, gid);
const fd = fs.openSync(path, constants.O_WRONLY | constants.O_SYMLINK);
let ret;
try {
ret = fs.fchownSync(fd, uid, gid);
} finally {
fs.closeSync(fd);
}
return ret;
};
}

Expand Down

0 comments on commit 082f952

Please sign in to comment.