Skip to content

Commit 577933a

Browse files
jasnellMylesBorins
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 504054c commit 577933a

File tree

1 file changed

+19
-11
lines changed

1 file changed

+19
-11
lines changed

lib/fs.js

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1090,20 +1090,16 @@ if (constants.O_SYMLINK !== undefined) {
10901090
};
10911091

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

10951095
// Prefer to return the chmod error, if one occurs,
10961096
// but still try to close, and report closing errors if they occur.
1097-
var ret;
1097+
let ret;
10981098
try {
10991099
ret = fs.fchmodSync(fd, mode);
1100-
} catch (err) {
1101-
try {
1102-
fs.closeSync(fd);
1103-
} catch (ignore) {}
1104-
throw err;
1100+
} finally {
1101+
fs.closeSync(fd);
11051102
}
1106-
fs.closeSync(fd);
11071103
return ret;
11081104
};
11091105
}
@@ -1135,13 +1131,25 @@ if (constants.O_SYMLINK !== undefined) {
11351131
callback(err);
11361132
return;
11371133
}
1138-
fs.fchown(fd, uid, gid, callback);
1134+
// Prefer to return the chown error, if one occurs,
1135+
// but still try to close, and report closing errors if they occur.
1136+
fs.fchown(fd, uid, gid, function(err) {
1137+
fs.close(fd, function(err2) {
1138+
callback(err || err2);
1139+
});
1140+
});
11391141
});
11401142
};
11411143

11421144
fs.lchownSync = function(path, uid, gid) {
1143-
var fd = fs.openSync(path, constants.O_WRONLY | constants.O_SYMLINK);
1144-
return fs.fchownSync(fd, uid, gid);
1145+
const fd = fs.openSync(path, constants.O_WRONLY | constants.O_SYMLINK);
1146+
let ret;
1147+
try {
1148+
ret = fs.fchownSync(fd, uid, gid);
1149+
} finally {
1150+
fs.closeSync(fd);
1151+
}
1152+
return ret;
11451153
};
11461154
}
11471155

0 commit comments

Comments
 (0)