Add housekeeping to keep track of the Content-Type of the image befor…

…e each step. Fixes processing of .ico using GraphicsMagick, and improves performance of other GraphicsMagic operations that only support autodetection of the source image type by streaming it to disc first.
papandreou · Sep 17, 2015 · d19100e · d19100e
1 parent 5b9657e
commit d19100e
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 5 deletions.
diff --git a/lib/getFilterInfosAndTargetContentTypeFromQueryString.js b/lib/getFilterInfosAndTargetContentTypeFromQueryString.js
@@ -2,6 +2,7 @@
 var Stream = require('stream'),
     _ = require('underscore'),
     gm = require('gm'),
+    mime = require('mime'),
     exifReader = require('exif-reader'),
     icc = require('icc'),
     sharp,
@@ -25,6 +26,15 @@ Object.keys(gm.prototype).forEach(function (propertyName) {
     }
 });
 
+function getMockFileNameForContentType(contentType) {
+    if (contentType) {
+        if (contentType === 'image/vnd.microsoft.icon') {
+            return '.ico';
+        }
+        return mime.extensions[contentType];
+    }
+}
+
 // For compatibility with the sharp format switchers (minus webp, which graphicsmagick doesn't support).
 // Consider adding more from this list: gm convert -list format
 ['jpeg', 'png'].forEach(function (formatName) {
@@ -60,7 +70,7 @@ module.exports = function getFilterInfosAndTargetContentTypeFromQueryString(quer
         operationNames = [],
         usedQueryStringFragments = [],
         leftOverQueryStringFragments = [],
-        targetContentType,
+        targetContentType = options.sourceContentType,
         root = options.root || options.rootPath;
 
     function checkSharpOrGmOperation(operation) {
@@ -115,7 +125,7 @@ module.exports = function getFilterInfosAndTargetContentTypeFromQueryString(quer
                                 spawned = true;
                                 var seenData = false,
                                     hasEnded = false,
-                                    gmInstance = gm(readStream);
+                                    gmInstance = gm(readStream, getMockFileNameForContentType(gmOperationsForThisInstance[0].sourceContentType));
                                 if (options.maxInputPixels) {
                                     gmInstance.limit('pixels', options.maxInputPixels);
                                 }
@@ -188,7 +198,6 @@ module.exports = function getFilterInfosAndTargetContentTypeFromQueryString(quer
             if (filters[operationName]) {
                 flushOperations();
                 filterInfo = filters[operationName](operationArgs, {
-                    inputContentType: targetContentType,
                     numPreceedingFilters: filterInfos.length
                 });
                 if (filterInfo) {
@@ -274,6 +283,7 @@ module.exports = function getFilterInfosAndTargetContentTypeFromQueryString(quer
                             currentEngineName = candidateEngineNames[0];
                         }
                     }
+                    var sourceContentType = targetContentType;
                     if (operationName === 'setFormat' && operationArgs.length > 0) {
                         var targetFormat = operationArgs[0].toLowerCase();
                         if (targetFormat === 'jpg') {
@@ -283,7 +293,7 @@ module.exports = function getFilterInfosAndTargetContentTypeFromQueryString(quer
                     } else if (operationName === 'jpeg' || operationName === 'png' || operationName === 'webp') {
                         targetContentType = 'image/' + operationName;
                     }
-                    operations.push({name: operationName, args: operationArgs, usedQueryStringFragment: keyValuePair});
+                    operations.push({sourceContentType: sourceContentType, name: operationName, args: operationArgs, usedQueryStringFragment: keyValuePair});
                     usedQueryStringFragments.push(keyValuePair);
                 }
             } else {

diff --git a/lib/processImage.js b/lib/processImage.js
@@ -72,7 +72,8 @@ module.exports = function (options) {
 
                 if (contentType && contentType.indexOf('image/') === 0) {
                     var filterInfosAndTargetFormat = getFilterInfosAndTargetContentTypeFromQueryString(queryString, _.defaults({
-                            sourceFilePath: options.root && Path.resolve(options.root, req.url.substr(1))
+                            sourceFilePath: options.root && Path.resolve(options.root, req.url.substr(1)),
+                            sourceContentType: contentType
                         }, options)),
                         targetContentType = filterInfosAndTargetFormat.targetContentType;
                     if (filterInfosAndTargetFormat.filterInfos.length === 0) {

diff --git a/test/processImage.js b/test/processImage.js
@@ -1,5 +1,6 @@
 /*global describe, it, beforeEach, __dirname*/
 var express = require('express'),
+    fs = require('fs'),
     pathModule = require('path'),
     unexpected = require('unexpected'),
     processImage = require('../lib/processImage'),
@@ -262,6 +263,43 @@ describe('express-processimage', function () {
         });
     });
 
+    // Undetectable by gm -- the source format must be explicitly specified
+    it('should convert an icon to png via GraphicsMagick', function () {
+        return expect('GET /favicon.ico?gm&setFormat=png', 'to yield response', {
+            headers: {
+                'Content-Type': 'image/png'
+            },
+            body: expect.it('to have metadata satisfying', {
+                format: 'PNG',
+                size: {
+                    width: 16,
+                    height: 16
+                }
+            })
+        });
+    });
+
+    it('should convert an icon served as image/vnd.microsoft.icon to png via GraphicsMagick', function () {
+        return expect(express().use(processImage(config)).get('/favicon.ico', function (req, res, next) {
+            res.setHeader('Content-Type', 'image/vnd.microsoft.icon');
+            fs.createReadStream(pathModule.resolve(__dirname, '..', 'testdata', 'favicon.ico')).pipe(res);
+        }), 'to yield exchange', {
+            request: 'GET /favicon.ico?gm&setFormat=png',
+            response: {
+                headers: {
+                    'Content-Type': 'image/png'
+                },
+                body: expect.it('to have metadata satisfying', {
+                    format: 'PNG',
+                    size: {
+                        width: 16,
+                        height: 16
+                    }
+                })
+            }
+        });
+    });
+
     describe.skipIf(!sharp, 'when sharp is available', function () {
         it('should allow retrieving the image metadata as JSON', function () {
             return expect('GET /turtle.jpg?metadata', 'to yield response', {