Merge pull request #2102 from reavowed/fix-extract-name-clashes

Fix name clashes in package extraction

lib/util/extract.js

@@ -9,6 +9,7 @@ var junk = require('junk');
 var createError = require('./createError');
 var createWriteStream = require('fs-write-stream-atomic');
 var destroy = require('destroy');
+var tmp = require('tmp');
 
 // This forces the default chunk size to something small in an attempt
 // to avoid issue #314
@@ -164,15 +165,11 @@ function isSingleDir(dir) {
     });
 }
 
-function moveSingleDirContents(dir) {
-    var destDir = path.dirname(dir);
-
-    return Q.nfcall(fs.readdir, dir)
-    .then(function (files) {
-        var promises;
-
-        promises = files.map(function (file) {
-            var src = path.join(dir, file);
+function moveDirectory(srcDir, destDir) {
+    return Q.nfcall(fs.readdir, srcDir)
+    .then(function(files) {
+        var promises = files.map(function (file) {
+            var src = path.join(srcDir, file);
             var dst = path.join(destDir, file);
 
             return Q.nfcall(fs.rename, src, dst);
@@ -181,7 +178,7 @@ function moveSingleDirContents(dir) {
         return Q.all(promises);
     })
     .then(function () {
-        return Q.nfcall(fs.rmdir, dir);
+        return Q.nfcall(fs.rmdir, srcDir);
     });
 }
 
@@ -215,44 +212,53 @@ function extract(src, dst, opts) {
         return Q.reject(createError('File ' + src + ' is not a known archive', 'ENOTARCHIVE'));
     }
 
-    // Check archive file size
-    promise = Q.nfcall(fs.stat, src)
-    .then(function (stat) {
-        if (stat.size <= 8) {
-            throw createError('File ' + src + ' is an invalid archive', 'ENOTARCHIVE');
-        }
-
-        // Extract archive
-        return extractor(src, dst);
-    });
-
-    // TODO: There's an issue here if the src and dst are the same and
-    //       The zip name is the same as some of the zip file contents
-    //       Maybe create a temp directory inside dst, unzip it there,
-    //       unlink zip and then move contents
+    // Extract to a temporary directory in case of file name clashes
+    return Q.nfcall(tmp.dir, {
+        template: dst + '-XXXXXX',
+        mode: 0777 & ~process.umask()
+    }).then(function (tempDir) {
+        // nfcall may return multiple callback arguments as an array
+        return Array.isArray(tempDir) ? tempDir[0] : tempDir;
+    }).then(function (tempDir) {
+
+        // Check archive file size
+        promise = Q.nfcall(fs.stat, src)
+        .then(function (stat) {
+            if (stat.size <= 8) {
+                throw createError('File ' + src + ' is an invalid archive', 'ENOTARCHIVE');
+            }
 
-    // Remove archive
-    if (!opts.keepArchive) {
-        promise = promise
-        .then(function () {
-            return Q.nfcall(fs.unlink, src);
+            // Extract archive
+            return extractor(src, tempDir);
         });
-    }
 
-    // Move contents if a single directory was extracted
-    if (!opts.keepStructure) {
+        // Remove archive
+        if (!opts.keepArchive) {
+            promise = promise
+            .then(function () {
+                return Q.nfcall(fs.unlink, src);
+            });
+        }
+
+        // Move contents from the temporary directory
+        // If the contents are a single directory (and we're not preserving structure),
+        // move its contents directly instead.
         promise = promise
         .then(function () {
-            return isSingleDir(dst);
+            return isSingleDir(tempDir);
         })
         .then(function (singleDir) {
-            return singleDir ? moveSingleDirContents(singleDir) : null;
+            if (singleDir && !opts.keepStructure) {
+                return moveDirectory(singleDir, dst);
+            } else {
+                return moveDirectory(tempDir, dst);
+            }
         });
-    }
 
-    // Resolve promise to the dst dir
-    return promise.then(function () {
-        return dst;
+        // Resolve promise to the dst dir
+        return promise.then(function () {
+            return dst;
+        });
     });
 }
 

test/commands/install.js

@@ -3,6 +3,9 @@ var path = require('path');
 var helpers = require('../helpers');
 var nock = require('nock');
 var fs = require('../../lib/util/fs');
+var tar = require('tar-fs');
+var destroy = require('destroy');
+var Q = require('q');
 
 describe('bower install', function() {
 
@@ -447,4 +450,73 @@ describe('bower install', function() {
             expect(error.code).to.equal('ENOTDIR');
         });
     });
+
+    it('works if the package is a compressed single directory containing another directory with the same name', function() {
+        var mainPackageBaseName = path.basename(mainPackage.path);
+        var parentDir = path.dirname(mainPackage.path);
+
+        // Setup the main package with a directory with the same name
+        var mainPackageFiles = {};
+        mainPackageFiles[mainPackageBaseName + '/test.js'] = 'test';
+        mainPackage.prepare(mainPackageFiles);
+
+        // Create an archive containing the main package
+        var archiveDeferred = Q.defer();
+        var archivePath = path.join(parentDir, mainPackageBaseName + '.tar');
+        var stream = tar.pack(parentDir, { entries: [mainPackageBaseName] });
+        stream
+            .pipe(fs.createWriteStream(archivePath))
+            .on('finish', function(result) {
+                destroy(stream);
+                archiveDeferred.resolve(result);
+            });
+
+        //// Attempt to install the package from the archive
+        tempDir.prepare({
+            'bower.json': {
+                name: 'test'
+            }
+        });
+
+        return archiveDeferred.promise
+        .then(function() {
+            return helpers.run(install, [[archivePath]]);
+        })
+        .then(function() {
+            expect(tempDir.read(path.join('bower_components', 'package', mainPackageBaseName, 'test.js'))).to.contain('test');
+        });
+    });
+
+    it('works if the package is an archive containing a file with an identical name', function() {
+        var parentDir = path.dirname(mainPackage.path);
+
+        mainPackage.prepare({
+            'package.tar': 'test'
+        });
+
+        var archiveDeferred = Q.defer();
+        var archivePath = path.join(parentDir, 'package.tar');
+        var stream = tar.pack(mainPackage.path);
+        stream
+            .pipe(fs.createWriteStream(archivePath))
+            .on('finish', function(result) {
+                destroy(stream);
+                archiveDeferred.resolve(result);
+            });
+
+
+        tempDir.prepare({
+            'bower.json': {
+                name: 'test'
+            }
+        });
+
+        return archiveDeferred.promise
+        .then(function() {
+            return helpers.run(install, [[archivePath]]);
+        })
+        .then(function() {
+            expect(tempDir.read(path.join('bower_components', 'package', 'package.tar'))).to.contain('test');
+        });
+    });
 });