Install: avoid race conditions when creating directories (#1358)

install/dll-copy.js

@@ -4,6 +4,7 @@ const fs = require('fs');
 const path = require('path');
 
 const copyFileSync = require('fs-copy-file-sync');
+const libvips = require('../lib/libvips');
 const npmLog = require('npmlog');
 
 if (process.platform === 'win32') {
@@ -11,8 +12,8 @@ if (process.platform === 'win32') {
   const buildReleaseDir = path.join(buildDir, 'Release');
   npmLog.info('sharp', `Creating ${buildReleaseDir}`);
   try {
-    fs.mkdirSync(buildDir);
-    fs.mkdirSync(buildReleaseDir);
+    libvips.mkdirSync(buildDir);
+    libvips.mkdirSync(buildReleaseDir);
   } catch (err) {}
   const vendorLibDir = path.join(__dirname, '..', 'vendor', 'lib');
   npmLog.info('sharp', `Copying DLLs from ${vendorLibDir} to ${buildReleaseDir}`);

install/libvips.js

@@ -20,9 +20,7 @@ const distBaseUrl = process.env.SHARP_DIST_BASE_URL || `https://github.com/lovel
 
 const extractTarball = function (tarPath) {
   const vendorPath = path.join(__dirname, '..', 'vendor');
-  if (!fs.existsSync(vendorPath)) {
-    fs.mkdirSync(vendorPath);
-  }
+  libvips.mkdirSync(vendorPath);
   tar
     .extract({
       file: tarPath,

lib/libvips.js

@@ -15,15 +15,21 @@ const spawnSyncOptions = {
   shell: true
 };
 
+const mkdirSync = function (dirPath) {
+  try {
+    fs.mkdirSync(dirPath);
+  } catch (err) {
+    if (err.code !== 'EEXIST') {
+      throw err;
+    }
+  }
+};
+
 const cachePath = function () {
   const npmCachePath = env.npm_config_cache || (env.APPDATA ? path.join(env.APPDATA, 'npm-cache') : path.join(os.homedir(), '.npm'));
-  if (!fs.existsSync(npmCachePath)) {
-    fs.mkdirSync(npmCachePath);
-  }
+  mkdirSync(npmCachePath);
   const libvipsCachePath = path.join(npmCachePath, '_libvips');
-  if (!fs.existsSync(libvipsCachePath)) {
-    fs.mkdirSync(libvipsCachePath);
-  }
+  mkdirSync(libvipsCachePath);
   return libvipsCachePath;
 };
 
@@ -78,5 +84,6 @@ module.exports = {
   globalLibvipsVersion: globalLibvipsVersion,
   hasVendoredLibvips: hasVendoredLibvips,
   pkgConfigPath: pkgConfigPath,
-  useGlobalLibvips: useGlobalLibvips
+  useGlobalLibvips: useGlobalLibvips,
+  mkdirSync: mkdirSync
 };

package.json

@@ -103,6 +103,7 @@
     "exif-reader": "^1.0.2",
     "icc": "^1.0.0",
     "mocha": "^5.2.0",
+    "mock-fs": "^4.6.0",
     "nyc": "^12.0.2",
     "prebuild": "^7.6.2",
     "prebuild-ci": "^2.2.3",

test/unit/libvips.js

@@ -4,6 +4,7 @@ const assert = require('assert');
 const fs = require('fs');
 const semver = require('semver');
 const libvips = require('../../lib/libvips');
+const mockFS = require('mock-fs');
 
 const originalPlatform = process.platform;
 
@@ -74,4 +75,34 @@ describe('libvips binaries', function () {
       assert.strictEqual(true, fs.existsSync(cachePath));
     });
   });
+
+  describe('safe directory creation', function () {
+    before(function () {
+      mockFS({
+        exampleDirA: {
+          exampleDirB: {
+            exampleFile: 'Example test file'
+          }
+        }
+      });
+    });
+    after(function () { mockFS.restore(); });
+
+    it('mkdirSync creates a directory', function () {
+      const dirPath = 'createdDir';
+
+      libvips.mkdirSync(dirPath);
+      assert.strictEqual(true, fs.existsSync(dirPath));
+    });
+    it('mkdirSync does not throw error or overwrite an existing dir', function () {
+      const dirPath = 'exampleDirA';
+      const nestedDirPath = 'exampleDirA/exampleDirB';
+      assert.strictEqual(true, fs.existsSync(dirPath));
+
+      libvips.mkdirSync(dirPath);
+
+      assert.strictEqual(true, fs.existsSync(dirPath));
+      assert.strictEqual(true, fs.existsSync(nestedDirPath));
+    });
+  });
 });