Use random temp folder for remote zip extraction (#210)

* Use random temp folder for remote zip extraction

- generating random folder inside `/tmp/tldr/`
- removing temp folders after cache has been updated

Author: vladimyr

lib/cache.js

@@ -1,13 +1,18 @@
 'use strict';
 
 const fs = require('fs-extra');
+const os = require('os');
 const path = require('path');
 const config = require('./config');
 const remote = require('./remote');
 const platform = require('./platform');
 const index = require('./index');
+const utils = require('./utils');
 
 const CACHE_FOLDER = path.join(config.get().cache, 'cache');
+const TEMP_FOLDER = path.join(os.tmpdir(), 'tldr');
+
+exports.CACHE_FOLDER = CACHE_FOLDER;
 
 exports.lastUpdated = () => {
   return fs.stat(CACHE_FOLDER);
@@ -33,15 +38,32 @@ exports.clear = () => {
 };
 
 exports.update = () => {
+  // Temporary folder path: /tmp/tldr/{randomName}
+  let tempFolder = path.join(TEMP_FOLDER, utils.uniqueId());
+
   // Downloading fresh copy
-  return fs.ensureDir(CACHE_FOLDER)
+  return Promise.all([
+    // Create new temporary folder
+    fs.ensureDir(tempFolder),
+    fs.ensureDir(CACHE_FOLDER)
+  ])
     .then(() => {
-      return remote.download();
+      // Download and extract cache data to temporary folder
+      return remote.download(tempFolder);
     })
-    .then((tempFolder) => {
+    .then(() => {
+      // Copy data to cache folder
       return fs.copy(tempFolder, CACHE_FOLDER);
     })
     .then(() => {
-      return index.rebuildPagesIndex();
+      return Promise.all([
+        // Remove temporary folder
+        fs.remove(tempFolder),
+        index.rebuildPagesIndex()
+      ]);
+    })
+    // eslint-disable-next-line no-unused-vars
+    .then(([_, shortIndex]) => {
+      return shortIndex;
     });
 };

lib/remote.js

@@ -1,46 +1,39 @@
 'use strict';
 
-const path = require('path');
-const os = require('os');
-const fs = require('fs-extra');
+const unzip = require('unzip2');
 const config  = require('./config');
 
-// Downloads the zip file from github and extracts it to /tmp/tldr
-exports.download = () => {
-  let request = require('request');
-  let unzip = require('unzip2');
+let request = require('request');
+
+// Downloads the zip file from github and extracts it to folder
+exports.download = (path) => {
   let url = config.get().repository;
-  let target = path.join(os.tmpdir(), 'tldr');
-
-  // Empty the tmp dir
-  return fs.emptyDir(target)
-    .then(() => {
-      // Creating the extractor
-      let extractor = unzip.Extract({ path: target });
-
-      // Setting the proxy if set by config
-      if (config.get().proxy) {
-        request = request.defaults({ proxy: config.proxy });
-      }
-
-      // Creating the request and passing the extractor
-      let req = request.get({
-        url: url,
-        headers: { 'User-Agent' : 'tldr-node-client' }
-      });
-
-      req.pipe(extractor);
-
-      return new Promise((resolve, reject) => {
-        req.on('error', (err) => {
-          reject(err);
-        });
-        extractor.on('error', () => {
-          reject(new Error('Cannot update from ' + url));
-        });
-        extractor.on('close', () => {
-          resolve(target);
-        });
-      });
+
+  // Creating the extractor
+  let extractor = unzip.Extract({ path });
+
+  // Setting the proxy if set by config
+  if (config.get().proxy) {
+    request = request.defaults({ proxy: config.proxy });
+  }
+
+  // Creating the request and passing the extractor
+  let req = request.get({
+    url: url,
+    headers: { 'User-Agent' : 'tldr-node-client' }
+  });
+
+  req.pipe(extractor);
+
+  return new Promise((resolve, reject) => {
+    req.on('error', (err) => {
+      reject(err);
+    });
+    extractor.on('error', () => {
+      reject(new Error('Cannot update from ' + url));
+    });
+    extractor.on('close', () => {
+      resolve();
     });
+  });
 };

lib/utils.js

@@ -1,6 +1,7 @@
 const fs = require('fs-extra');
 const path = require('path');
 const glob = require('glob');
+const crypto = require('crypto');
 const flatten = require('lodash/flatten');
 
 module.exports = {
@@ -52,5 +53,11 @@ module.exports = {
         }
       });
     });
+  },
+
+  // eslint-disable-next-line no-magic-numbers
+  uniqueId(length = 32) {
+    const size = Math.ceil(length / 2);
+    return crypto.randomBytes(size).toString('hex').slice(0, length);
   }
 };

test/cache.spec.js

@@ -5,6 +5,7 @@ const should = require('should');
 const sinon = require('sinon');
 const fs = require('fs-extra');
 const index = require('../lib/index');
+const remote = require('../lib/remote');
 const platform = require('../lib/platform');
 
 
@@ -23,6 +24,50 @@ describe('Cache', () => {
       });
   });
 
+  describe('update()', () => {
+    beforeEach(() => {
+      sinon.spy(fs, 'ensureDir');
+      sinon.spy(fs, 'remove');
+      sinon.stub(fs, 'copy').resolves();
+      sinon.stub(remote, 'download').resolves();
+      sinon.stub(index, 'rebuildPagesIndex').resolves();
+    });
+
+    it('should use randomly created temp folder', () => {
+      const count = 16;
+      return Promise.all(Array.from({ length: count }).map(() => {
+        return cache.update();
+      })).then(() => {
+        let calls = fs.ensureDir.getCalls().filter((call) => {
+          return !call.calledWith(cache.CACHE_FOLDER);
+        });
+        calls.should.have.length(count);
+        let tempFolders = calls.map((call) => {
+          return call.args[0];
+        });
+        tempFolders.should.have.length(new Set(tempFolders).size);
+      });
+    });
+
+    it('should remove temp folder after cache gets updated', () => {
+      return cache.update().then(() => {
+        let createFolder = fs.ensureDir.getCalls().find((call) => {
+          return !call.calledWith(cache.CACHE_FOLDER);
+        });
+        let removeFolder = fs.remove.getCall(0);
+        removeFolder.args[0].should.be.equal(createFolder.args[0]);
+      });
+    });
+
+    afterEach(() => {
+      fs.ensureDir.restore();
+      fs.remove.restore();
+      fs.copy.restore();
+      remote.download.restore();
+      index.rebuildPagesIndex.restore();
+    });
+  });
+
   describe('getPage()', () => {
     beforeEach(() => {
       sinon.stub(index, 'getShortIndex').returns({