feat(web-server): Use isbinaryfile for binary file detection

mramato · mramato · commit f938a8ef6a90 · 2016-01-22T00:11:54.000-05:00
Delete `lib/binary-extensions.json` and change the preprocessor to use the `isbinaryfile` module instead. Factor nextPreprocessor function creation into a factory function for code clarity. Update preprocessor specs to use binary data when mocking binary files. Closes #1070
diff --git a/lib/binary-extensions.json b/lib/binary-extensions.json
diff --git a/lib/preprocessor.js b/lib/preprocessor.js
@@ -1,8 +1,7 @@
-var path = require('path')
 var fs = require('graceful-fs')
 var crypto = require('crypto')
 var mm = require('minimatch')
-var extensions = require('./binary-extensions.json').extensions
+var isBinaryFile = require('isbinaryfile')
 
 var log = require('./logger').create('preprocess')
 
@@ -12,10 +11,30 @@ var sha1 = function (data) {
   return hash.digest('hex')
 }
 
-var isBinary = Object.create(null)
-extensions.forEach(function (extension) {
-  isBinary['.' + extension] = true
-})
+var createNextProcessor = function (preprocessors, file, done) {
+  return function nextPreprocessor (error, content) {
+    // normalize B-C
+    if (arguments.length === 1 && typeof error === 'string') {
+      content = error
+      error = null
+    }
+
+    if (error) {
+      file.content = null
+      file.contentPath = null
+      return done(error)
+    }
+
+    if (!preprocessors.length) {
+      file.contentPath = null
+      file.content = content
+      file.sha = sha1(content)
+      return done()
+    }
+
+    preprocessors.shift()(content, file, nextPreprocessor)
+  }
+}
 
 var createPreprocessor = function (config, basePath, injector) {
   var alreadyDisplayedWarnings = {}
@@ -51,64 +70,49 @@ var createPreprocessor = function (config, basePath, injector) {
 
   return function preprocess (file, done) {
     patterns = Object.keys(config)
-    var thisFileIsBinary = isBinary[path.extname(file.originalPath).toLowerCase()]
-    var preprocessors = []
-
-    var nextPreprocessor = function (error, content) {
-      // normalize B-C
-      if (arguments.length === 1 && typeof error === 'string') {
-        content = error
-        error = null
-      }
-
-      if (error) {
-        file.content = null
-        file.contentPath = null
-        return done(error)
-      }
 
-      if (!preprocessors.length) {
-        file.contentPath = null
-        file.content = content
-        file.sha = sha1(content)
-        return done()
+    return fs.readFile(file.originalPath, function (err, buffer) {
+      if (err) {
+        throw err
       }
 
-      preprocessors.shift()(content, file, nextPreprocessor)
-    }
-
-    for (var i = 0; i < patterns.length; i++) {
-      if (mm(file.originalPath, patterns[i], {dot: true})) {
-        if (thisFileIsBinary) {
-          log.warn('Ignoring preprocessing (%s) %s because it is a binary file.',
-            config[patterns[i]].join(', '), file.originalPath)
-        } else {
-          config[patterns[i]].forEach(function (name) {
-            var p = instances[name]
-            if (p == null) {
-              p = instantiatePreprocessor(name)
-            }
+      isBinaryFile(buffer, buffer.length, function (err, thisFileIsBinary) {
+        if (err) {
+          throw err
+        }
 
-            if (p == null) {
-              if (!alreadyDisplayedWarnings[name]) {
-                alreadyDisplayedWarnings[name] = true
-                log.warn('Failed to instantiate preprocessor %s', name)
-              }
-              return
+        var preprocessors = []
+        var nextPreprocessor = createNextProcessor(preprocessors, file, done)
+
+        for (var i = 0; i < patterns.length; i++) {
+          if (mm(file.originalPath, patterns[i], {dot: true})) {
+            if (thisFileIsBinary) {
+              log.warn('Ignoring preprocessing (%s) %s because it is a binary file.',
+                config[patterns[i]].join(', '), file.originalPath)
+            } else {
+              config[patterns[i]].forEach(function (name) {
+                var p = instances[name]
+                if (p == null) {
+                  p = instantiatePreprocessor(name)
+                }
+
+                if (p == null) {
+                  if (!alreadyDisplayedWarnings[name]) {
+                    alreadyDisplayedWarnings[name] = true
+                    log.warn('Failed to instantiate preprocessor %s', name)
+                  }
+                  return
+                }
+
+                instances[name] = p
+                preprocessors.push(p)
+              })
             }
-
-            instances[name] = p
-            preprocessors.push(p)
-          })
+          }
         }
-      }
-    }
 
-    return fs.readFile(file.originalPath, function (err, buffer) {
-      if (err) {
-        throw err
-      }
-      nextPreprocessor(null, thisFileIsBinary ? buffer : buffer.toString())
+        nextPreprocessor(null, thisFileIsBinary ? buffer : buffer.toString())
+      })
     })
   }
 }
diff --git a/package.json b/package.json
@@ -265,6 +265,7 @@
     "glob": "^6.0.3",
     "graceful-fs": "^4.1.2",
     "http-proxy": "^1.11.1",
+    "isbinaryfile": "^3.0.0",
     "lodash": "^3.8.0",
     "log4js": "^0.6.28",
     "mime": "^1.3.4",
diff --git a/test/unit/preprocessor.spec.js b/test/unit/preprocessor.spec.js
@@ -6,14 +6,17 @@ describe('preprocessor', () => {
   var m
   var mockFs
 
+  // mimic first few bytes of a pdf file
+  var binarydata = new Buffer([0x25, 0x50, 0x44, 0x66, 0x46, 0x00])
+
   beforeEach(() => {
     mockFs = mocks.fs.create({
       some: {
         'a.js': mocks.fs.file(0, 'content'),
         'b.js': mocks.fs.file(0, 'content'),
         'a.txt': mocks.fs.file(0, 'some-text'),
-        'photo.png': mocks.fs.file(0, 'binary'),
-        'CAM_PHOTO.JPG': mocks.fs.file(0, 'binary'),
+        'photo.png': mocks.fs.file(0, binarydata),
+        'CAM_PHOTO.JPG': mocks.fs.file(0, binarydata),
         '.dir': {
           'a.js': mocks.fs.file(0, 'content')
         }
