From 230a1026471ca6bf94e830840a8938a8360af6ed Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Tue, 23 Jan 2018 15:07:42 -0800
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/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>
---
 lib/fs.js | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/lib/fs.js b/lib/fs.js
index 6a99996e73cea5..b2043d45f40afc 100644
--- a/lib/fs.js
+++ b/lib/fs.js
@@ -1110,20 +1110,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;
   };
 }
@@ -1155,13 +1151,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;
   };
 }