Merge pull request #764 from appirio-tech/refactor-file-validation

Test file extension and mime types and add tests

Author: nlitwin

app/directives/tc-file-input/tc-file-input.directive.js

@@ -4,9 +4,9 @@ import _ from 'lodash'
 (function() {
   'use strict'
 
-  angular.module('tcUIComponents').directive('tcFileInput', ['$timeout', tcFileInput])
+  angular.module('tcUIComponents').directive('tcFileInput', ['$timeout', 'Helpers', 'logger', tcFileInput])
 
-  function tcFileInput($timeout) {
+  function tcFileInput($timeout, Helpers, logger) {
     return {
       restrict: 'E',
       require: '^form',
@@ -26,14 +26,6 @@ import _ from 'lodash'
         scope.selectFile = selectFile
         var fileTypes = scope.fileType.split(',')
 
-        // Add extra checks for Windows zip file types
-        var hasZip = _.some(fileTypes, _.matches('zip'))
-
-        if (hasZip) {
-          fileTypes = angular.copy(fileTypes)
-          fileTypes.push('x-zip', 'x-zip-compressed')
-        }
-
         // fieldId is not set on element at this point, so grabbing with class .none
         // which exists on the element right away
         var fileInput = $(element[0]).find('.none')
@@ -49,10 +41,16 @@ import _ from 'lodash'
           }
 
           var fileSize = file.size
-          var isAllowedFileSize = fileSize < '524288000'
+          var fileExtension = file.name.slice(file.name.lastIndexOf('.') + 1)
 
-          var selectedFileType = file.type.slice(file.type.lastIndexOf('/') + 1)
-          var isAllowedFileFormat = _.some(fileTypes, _.matches(selectedFileType))
+          var isAllowedFileSize   = fileSize < '524288000'
+          var isAllowedFileExt    = _.some(fileTypes, _.matches(fileExtension))
+          var isAllowedMIMEType   = Helpers.isValidMIMEType(file.type, fileExtension)
+          var isAllowedFileFormat = isAllowedFileExt && isAllowedMIMEType
+
+          if (file && !isAllowedFileFormat) {
+            logger.error(`Invalid file. Allowed extensions: ${scope.fileType}. Recieved file extension ${fileExtension} with MIME type ${file.type} and file size ${fileSize}`)
+          }
 
           // Timeout needed for fixing IE bug ($apply already in progress)
           $timeout(function() {

app/directives/tc-file-input/tc-file-input.spec.js

@@ -161,6 +161,28 @@ describe('Topcoder File Input Directive', function() {
       })
     })
 
+    describe('with a file extension that is not in the list of fileTypes given to the directive', function() {
+      beforeEach(function() {
+        fileList[0].name = 'submission.zip.jpg'
+
+        $(fileInput).triggerHandler({
+          type: 'change',
+          target: { files: fileList }
+        })
+
+        $timeout.flush()
+      })
+
+      it('does not call setFileReference', function() {
+        expect(mockSetFileReference).not.calledOnce
+      })
+
+      it('has ng-touched and ng-invalid-required classes', function() {
+        expect($(fileInput).hasClass('ng-invalid-required')).to.be.true
+        expect($(fileInput).hasClass('ng-touched')).to.be.true
+      })
+    })
+
     describe('with a file that\'s greater than 500MB', function() {
       beforeEach(function() {
         fileList[0].size = 524288001

app/services/helpers.service.js

@@ -1,4 +1,5 @@
 import angular from 'angular'
+import _ from 'lodash'
 
 (function() {
   'use strict'
@@ -21,8 +22,8 @@ import angular from 'angular'
       redirectPostLogin: redirectPostLogin,
       getSocialUserData: getSocialUserData,
       setupLoginEventMetrics: setupLoginEventMetrics,
-      npad: npad
-
+      npad: npad,
+      isValidMIMEType: isValidMIMEType
     }
     return service
 
@@ -297,5 +298,50 @@ import angular from 'angular'
     function npad(toPad, n) {
       return $filter('npad')(toPad, n)
     }
+
+    function isValidMIMEType(mimeType, fileExt) {
+      var areStrings = _.isString(mimeType) && _.isString(fileExt)
+
+      if (!areStrings) {
+        return false
+      }
+
+      var mimeTypesByExtension = {
+        zip: [
+          'application/zip',
+          'application/x-zip',
+          'application/x-zip-compressed',
+          'application/octet-stream',
+          'application/x-compress',
+          'application/x-compressed',
+          'multipart/x-zip'
+        ],
+        jpeg: [
+          'image/jpeg',
+          'image/jpg',
+          'image/jpe_',
+          'image/pjpeg',
+          'image/vnd.swiftview-jpeg'
+        ],
+        jpg: [
+          'image/jpeg',
+          'image/jpg',
+          'image/jp_',
+          'application/jpg',
+          'application/x-jpg',
+          'image/pjpeg',
+          'image/pipeg',
+          'image/vnd.swiftview-jpeg',
+          'image/x-xbitmap'
+        ],
+        png: [
+          'image/png',
+          'application/png',
+          'application/x-png'
+        ]
+      }
+
+      return _.some(mimeTypesByExtension[fileExt], _.matches(mimeType))
+    }
   }
 })()

app/services/helpers.service.spec.js

@@ -390,4 +390,25 @@ describe('Helper Service', function() {
       expect($window._kmq[0][1]).to.exist.to.equal('mockuser')
     })
   })
+
+  describe('isValidMIMEType', function() {
+    it('should return false for for non string arguments', function() {
+      expect(Helpers.isValidMIMEType()).to.be.false
+      expect(Helpers.isValidMIMEType(1, 'jpg')).to.be.false
+      expect(Helpers.isValidMIMEType('application/zip', {})).to.be.false
+    })
+
+    it('should return false for an unsupported file extension', function() {
+      expect(Helpers.isValidMIMEType('application/zip', 'zip314')).to.be.false
+    })
+
+    it('should return false for an unsupported MIME type', function() {
+      expect(Helpers.isValidMIMEType('application/zip314', 'zip')).to.be.false
+    })
+
+    it('should return true for valid MIME types and file extensions', function() {
+      expect(Helpers.isValidMIMEType('application/zip', 'zip')).to.be.true
+      expect(Helpers.isValidMIMEType('image/png', 'png')).to.be.true
+    })
+  })
 })