Skip to content

Commit 230a102

Browse files
jasnellevanlucas
authored andcommitted
fs: cleanup fd lchown and lchownSync
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>
1 parent 6ae7bb1 commit 230a102

File tree

1 file changed

+19
-11
lines changed

1 file changed

+19
-11
lines changed

lib/fs.js

+19-11
Original file line numberDiff line numberDiff line change
@@ -1110,20 +1110,16 @@ if (constants.O_SYMLINK !== undefined) {
11101110
};
11111111

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

11151115
// Prefer to return the chmod error, if one occurs,
11161116
// but still try to close, and report closing errors if they occur.
1117-
var ret;
1117+
let ret;
11181118
try {
11191119
ret = fs.fchmodSync(fd, mode);
1120-
} catch (err) {
1121-
try {
1122-
fs.closeSync(fd);
1123-
} catch (ignore) {}
1124-
throw err;
1120+
} finally {
1121+
fs.closeSync(fd);
11251122
}
1126-
fs.closeSync(fd);
11271123
return ret;
11281124
};
11291125
}
@@ -1155,13 +1151,25 @@ if (constants.O_SYMLINK !== undefined) {
11551151
callback(err);
11561152
return;
11571153
}
1158-
fs.fchown(fd, uid, gid, callback);
1154+
// Prefer to return the chown error, if one occurs,
1155+
// but still try to close, and report closing errors if they occur.
1156+
fs.fchown(fd, uid, gid, function(err) {
1157+
fs.close(fd, function(err2) {
1158+
callback(err || err2);
1159+
});
1160+
});
11591161
});
11601162
};
11611163

11621164
fs.lchownSync = function(path, uid, gid) {
1163-
var fd = fs.openSync(path, constants.O_WRONLY | constants.O_SYMLINK);
1164-
return fs.fchownSync(fd, uid, gid);
1165+
const fd = fs.openSync(path, constants.O_WRONLY | constants.O_SYMLINK);
1166+
let ret;
1167+
try {
1168+
ret = fs.fchownSync(fd, uid, gid);
1169+
} finally {
1170+
fs.closeSync(fd);
1171+
}
1172+
return ret;
11651173
};
11661174
}
11671175

0 commit comments

Comments
 (0)