Don't check extensions before the first non compressed extension (#493)

Currently, we extract all the extensions from an image path and report
back that the image is invalid if *any* of the extensions is not a
valida extension, however this can cause trouble with images including
information between dots that are not strictly extensions.

For example:

```
elementaryos-0.3.2-stable-i386.20151209.iso
```

Etcher will consider `20151209` to be an invalid extension and therefore
will prevent such image from being selected.

As a way to allow these corner cases but will make use of our enforced
check controls, the validation routine will only consider extensions
starting from the first non compressed extension.

This PR also includes logic to show a nice GUI dialog saying that the
image is invalid in order to provide a much better experience than just
silently failing.

Fixes: https://github.com/resin-io/etcher/issues/492
Signed-off-by: Juan Cruz Viotti <jviottidc@gmail.com>

lib/gui/app.js

@@ -108,6 +108,14 @@ app.controller('AppController', function(
   this.tooltipModal = TooltipModalService;
   this.success = true;
 
+  this.handleError = function(error) {
+    OSDialogService.showError(error);
+
+    // Also throw it so it gets displayed in DevTools
+    // and its reported by TrackJS.
+    throw error;
+  };
+
   if (UpdateNotifierService.shouldCheckForUpdates()) {
     AnalyticsService.logEvent('Checking for updates');
 
@@ -147,7 +155,7 @@ app.controller('AppController', function(
     OSWindowProgressService.set(state.progress);
   });
 
-  DriveScannerService.start(2000).on('error', OSDialogService.showError).on('scan', function(drives) {
+  DriveScannerService.start(2000).on('error', self.handleError).on('scan', function(drives) {
 
     // Cover the case where you select a drive, but then eject it.
     if (self.selection.hasDrive() && !_.find(drives, self.selection.isCurrentDrive)) {
@@ -189,6 +197,7 @@ app.controller('AppController', function(
 
   this.selectImage = function(image) {
     if (!SupportedFormatsModel.isSupportedImage(image.path)) {
+      OSDialogService.showError('Invalid image', `${image.path} is not a supported image type.`);
       AnalyticsService.logEvent('Invalid image', image);
       return;
     }
@@ -297,7 +306,7 @@ app.controller('AppController', function(
       }
 
       self.writer.resetState();
-      OSDialogService.showError(error);
+      self.handleError(error);
     })
     .finally(OSWindowProgressService.clear);
   };

lib/gui/models/supported-formats.js

@@ -22,6 +22,7 @@
 
 const angular = require('angular');
 const _ = require('lodash');
+const path = require('path');
 const imageStream = require('etcher-image-stream');
 const MODULE_NAME = 'Etcher.Models.SupportedFormats';
 const SupportedFormats = angular.module(MODULE_NAME, []);
@@ -30,23 +31,22 @@ SupportedFormats.service('SupportedFormatsModel', function() {
   let self = this;
 
   /**
-   * @summary Check if a file type is a compressed format
+   * @summary Build an extension list getter from a type
    * @function
    * @private
    *
-   * @param {Object} fileType - file type
-   * @returns {Boolean} whether the file type is a compressed format
+   * @param {String} type - file type
+   * @returns {Function} extension list getter
    *
    * @example
-   * if (isCompressedFileType({
-   *   extension: 'zip',
-   *   type: 'compressed'
-   * })) {
-   *   console.log('This is a compressed file type');
-   * }
+   * const extensions = getExtensionsFromTypeGetter('archive')();
    */
-  const isCompressedFileType = function(fileType) {
-    return fileType.type === 'compressed';
+  const getExtensionsFromTypeGetter = function(type) {
+    return function() {
+      return _.map(_.filter(imageStream.supportedFileTypes, function(fileType) {
+        return fileType.type === type;
+      }), 'extension');
+    };
   };
 
   /**
@@ -60,9 +60,7 @@ SupportedFormats.service('SupportedFormatsModel', function() {
    *   console.log('We support the ' + extension + ' compressed file format');
    * });
    */
-  this.getCompressedExtensions = function() {
-    return _.map(_.filter(imageStream.supportedFileTypes, isCompressedFileType), 'extension');
-  };
+  this.getCompressedExtensions = getExtensionsFromTypeGetter('compressed');
 
   /**
    * @summary Get non compressed extensions
@@ -75,9 +73,20 @@ SupportedFormats.service('SupportedFormatsModel', function() {
    *   console.log('We support the ' + extension + ' file format');
    * });
    */
-  this.getNonCompressedExtensions = function() {
-    return _.map(_.reject(imageStream.supportedFileTypes, isCompressedFileType), 'extension');
-  };
+  this.getNonCompressedExtensions = getExtensionsFromTypeGetter('image');
+
+  /**
+   * @summary Get archive extensions
+   * @function
+   * @public
+   *
+   * @returns {String[]} archive extensions
+   *
+   * SupportedFormatsModel.getArchiveExtensions().forEach(function(extension) {
+   *   console.log('We support the ' + extension + ' file format');
+   * });
+   */
+  this.getArchiveExtensions = getExtensionsFromTypeGetter('archive');
 
   /**
    * @summary Get all supported extensions
@@ -107,25 +116,20 @@ SupportedFormats.service('SupportedFormatsModel', function() {
    * }
    */
   this.isSupportedImage = function(image) {
+    const extension = path.extname(image).slice(1);
 
-    // We roll our own extension detection system instead of
-    // using `path.extname()` since that function will only
-    // return the last extension, while we're interested
-    // to check every possible extension of an image path,
-    // like `.tar.gz`.
+    if (_.some([
+      _.includes(self.getNonCompressedExtensions(), extension),
+      _.includes(self.getArchiveExtensions(), extension)
+    ])) {
+      return true;
+    }
 
-    const firstDotIndex = _.get(/(\.)[\w\.]+$/.exec(image), 'index');
-
-    // An image without an extension is not considered a valid image
-    if (!firstDotIndex) {
+    if (!_.includes(self.getCompressedExtensions(), extension)) {
       return false;
     }
 
-    const extensions = image.slice(firstDotIndex + 1).split('.');
-
-    return _.every(extensions, function(extension) {
-      return _.includes(self.getAllExtensions(), extension);
-    });
+    return self.isSupportedImage(path.basename(image, `.${extension}`));
   };
 
 });

lib/gui/os/dialog/services/dialog.js

@@ -81,25 +81,33 @@ module.exports = function($q, SupportedFormatsModel) {
    * @function
    * @public
    *
-   * @param {Error} error - error
+   * @param {(Error|String)} error - error
+   * @param {String} [description] - error description
    *
    * @example
    * OSDialogService.showError(new Error('Foo Bar'));
+   *
+   * @example
+   * OSDialogService.showError(new Error('Foo Bar'), 'A custom description');
+   *
+   * @example
+   * OSDialogService.showError('Foo Bar', 'An error happened!');
    */
-  this.showError = function(error) {
+  this.showError = function(error, description) {
     error = error || {};
 
     // Try to get as most information as possible about the error
     // rather than falling back to generic messages right away.
-    const title = error.message || error.code || 'An error ocurred';
-    const message = error.stack || JSON.stringify(error) || '';
+    const title = _.attempt(function() {
+      if (_.isString(error)) {
+        return error;
+      }
 
+      return error.message || error.code || 'An error ocurred';
+    });
+
+    const message = description || error.stack || JSON.stringify(error) || '';
     electron.remote.dialog.showErrorBox(title, message);
-
-    // Also throw it so it gets displayed in DevTools
-    // and its reported by TrackJS.
-    throw error;
-
   };
 
 };

package.json

@@ -60,7 +60,7 @@
     "chalk": "^1.1.3",
     "drivelist": "^3.2.0",
     "electron-is-running-in-asar": "^1.0.0",
-    "etcher-image-stream": "^2.0.0",
+    "etcher-image-stream": "^2.1.0",
     "etcher-image-write": "^5.0.1",
     "flexboxgrid": "^6.3.0",
     "is-elevated": "^1.0.0",

tests/gui/models/supported-formats.spec.js

@@ -23,7 +23,7 @@ describe('Browser: SupportedFormats', function() {
 
       it('should return the supported compressed extensions', function() {
         const extensions = SupportedFormatsModel.getCompressedExtensions();
-        m.chai.expect(extensions).to.deep.equal([ 'zip', 'gz', 'bz2', 'xz' ]);
+        m.chai.expect(extensions).to.deep.equal([ 'gz', 'bz2', 'xz' ]);
       });
 
     });
@@ -37,12 +37,22 @@ describe('Browser: SupportedFormats', function() {
 
     });
 
+    describe('.getArchiveExtensions()', function() {
+
+      it('should return the supported archive extensions', function() {
+        const extensions = SupportedFormatsModel.getArchiveExtensions();
+        m.chai.expect(extensions).to.deep.equal([ 'zip' ]);
+      });
+
+    });
+
     describe('.getAllExtensions()', function() {
 
-      it('should return the union of .getCompressedExtensions and .getNonCompressedExtensions', function() {
+      it('should return the union of all compressed, uncompressed, and archive extensions', function() {
+        const archiveExtensions = SupportedFormatsModel.getArchiveExtensions();
         const compressedExtensions = SupportedFormatsModel.getCompressedExtensions();
         const nonCompressedExtensions = SupportedFormatsModel.getNonCompressedExtensions();
-        const expected = _.union(compressedExtensions, nonCompressedExtensions);
+        const expected = _.union(archiveExtensions, compressedExtensions, nonCompressedExtensions);
         const extensions = SupportedFormatsModel.getAllExtensions();
         m.chai.expect(extensions).to.deep.equal(expected);
       });
@@ -62,15 +72,29 @@ describe('Browser: SupportedFormats', function() {
       });
 
       it('should return true if the extension is included in .getAllExtensions()', function() {
-        const supportedExtensions = SupportedFormatsModel.getAllExtensions();
-        const imagePath = '/path/to/foo.' + _.first(supportedExtensions);
+        const nonCompressedExtension = _.first(SupportedFormatsModel.getNonCompressedExtensions());
+        const imagePath = '/path/to/foo.' + nonCompressedExtension;
+        const isSupported = SupportedFormatsModel.isSupportedImage(imagePath);
+        m.chai.expect(isSupported).to.be.true;
+      });
+
+      it('should not consider an extension before a non compressed extension', function() {
+        const nonCompressedExtension = _.first(SupportedFormatsModel.getNonCompressedExtensions());
+        const imagePath = '/path/to/foo.1234.' + nonCompressedExtension;
         const isSupported = SupportedFormatsModel.isSupportedImage(imagePath);
         m.chai.expect(isSupported).to.be.true;
       });
 
       it('should return true if the extension is supported and the file name includes dots', function() {
-        const supportedExtensions = SupportedFormatsModel.getAllExtensions();
-        const imagePath = '/path/to/foo.1.2.3-bar.' + _.first(supportedExtensions);
+        const nonCompressedExtension = _.first(SupportedFormatsModel.getNonCompressedExtensions());
+        const imagePath = '/path/to/foo.1.2.3-bar.' + nonCompressedExtension;
+        const isSupported = SupportedFormatsModel.isSupportedImage(imagePath);
+        m.chai.expect(isSupported).to.be.true;
+      });
+
+      it('should return true if the extension is only a supported archive extension', function() {
+        const archiveExtension = _.first(SupportedFormatsModel.getArchiveExtensions());
+        const imagePath = '/path/to/foo.' + archiveExtension;
         const isSupported = SupportedFormatsModel.isSupportedImage(imagePath);
         m.chai.expect(isSupported).to.be.true;
       });