Used got to handle requests for image-size (#8892)

refs #8589, refs #8868 - swap `request` with `got` in `getImageSizeFromUrl` util - less handling for request cases e.g. timeouts, follow redirects
TryGhost · Aug 31, 2017 · 30bee11 · 30bee11
1 parent bf47397
commit 30bee11
Show file tree

Hide file tree

Showing 5 changed files with 383 additions and 258 deletions.
diff --git a/core/server/utils/image-size-from-url.js b/core/server/utils/image-size-from-url.js
@@ -14,108 +14,90 @@
 // we add the protocol to the incomplete one and use urlFor() to get the absolute URL.
 // If the request fails or image-size is not able to read the file, we reject with error.
 
-var sizeOf       = require('image-size'),
-    url          = require('url'),
-    Promise      = require('bluebird'),
-    http         = require('http'),
-    https        = require('https'),
-    config       = require('../config'),
-    utils        = require('../utils'),
-    errors       = require('../errors'),
-    dimensions,
-    request,
-    requestHandler;
+var sizeOf = require('image-size'),
+    url = require('url'),
+    Promise = require('bluebird'),
+    got = require('got'),
+    config = require('../config'),
+    utils = require('../utils'),
+    errors = require('../errors'),
+    dimensions;
 
 /**
  * @description read image dimensions from URL
  * @param {String} imagePath
  * @returns {Promise<Object>} imageObject or error
  */
 module.exports.getImageSizeFromUrl = function getImageSizeFromUrl(imagePath) {
-    return new Promise(function imageSizeRequest(resolve, reject) {
-        var imageObject = {},
-            options,
-            timeout = config.get('times:getImageSizeTimeoutInMS') || 10000;
+    var imageObject = {},
+        requestOptions,
+        timeout = config.get('times:getImageSizeTimeoutInMS') || 10000;
 
-        imageObject.url = imagePath;
+    imageObject.url = imagePath;
 
-        // check if we got an url without any protocol
-        if (imagePath.indexOf('http') === -1) {
-            // our gravatar urls start with '//' in that case add 'http:'
-            if (imagePath.indexOf('//') === 0) {
-                // it's a gravatar url
-                imagePath = 'http:' + imagePath;
-            } else {
-                // get absolute url for image
-                imagePath = utils.url.urlFor('image', {image: imagePath}, true);
-            }
+    // check if we got an url without any protocol
+    if (imagePath.indexOf('http') === -1) {
+        // our gravatar urls start with '//' in that case add 'http:'
+        if (imagePath.indexOf('//') === 0) {
+            // it's a gravatar url
+            imagePath = 'http:' + imagePath;
+        } else {
+            // get absolute url for image
+            imagePath = utils.url.urlFor('image', {image: imagePath}, true);
         }
+    }
 
-        options = url.parse(imagePath);
+    imagePath = url.parse(imagePath);
+    requestOptions = {
+        headers: {
+            'User-Agent': 'Mozilla/5.0'
+        },
+        timeout: timeout,
+        encoding: null
+    };
 
-        requestHandler = imagePath.indexOf('https') === 0 ? https : http;
-        options.headers = {'User-Agent': 'Mozilla/5.0'};
+    return got(
+        imagePath,
+        requestOptions
+    ).then(function (response) {
+        try {
+            // response.body contains the Buffer. Using the Buffer rather than an URL
+            // requires to use sizeOf synchronously. See https://github.com/image-size/image-size#asynchronous
+            dimensions = sizeOf(response.body);
 
-        request = requestHandler.get(options, function (res) {
-            var chunks = [];
+            imageObject.width = dimensions.width;
+            imageObject.height = dimensions.height;
 
-            res.on('data', function (chunk) {
-                chunks.push(chunk);
-            });
-
-            res.on('end', function () {
-                if (res.statusCode === 200) {
-                    try {
-                        dimensions = sizeOf(Buffer.concat(chunks));
-
-                        imageObject.width = dimensions.width;
-                        imageObject.height = dimensions.height;
-
-                        return resolve(imageObject);
-                    } catch (err) {
-                        return reject(new errors.InternalServerError({
-                            code: 'IMAGE_SIZE',
-                            err: err,
-                            context: imagePath
-                        }));
-                    }
-                } else if (res.statusCode === 404) {
-                    return reject(new errors.NotFoundError({
-                        message: 'Image not found.',
-                        code: 'IMAGE_SIZE',
-                        statusCode: res.statusCode,
-                        context: imagePath
-                    }));
-                } else {
-                    return reject(new errors.InternalServerError({
-                        message: 'Unknown Request error.',
-                        code: 'IMAGE_SIZE',
-                        statusCode: res.statusCode,
-                        context: imagePath
-                    }));
-                }
-            });
-        }).on('socket', function (socket) {
-            if (timeout) {
-                socket.setTimeout(timeout);
-
-                /**
-                 * https://nodejs.org/api/http.html
-                 * "...if a callback is assigned to the Server's 'timeout' event, timeouts must be handled explicitly"
-                 *
-                 * socket.destroy will jump to the error listener
-                 */
-                socket.on('timeout', function () {
-                    request.abort();
-                    socket.destroy(new Error('Request timed out.'));
-                });
-            }
-        }).on('error', function (err) {
-            return reject(new errors.InternalServerError({
+            return Promise.resolve(imageObject);
+        } catch (err) {
+            return Promise.reject(new errors.InternalServerError({
                 code: 'IMAGE_SIZE',
                 err: err,
-                context: imagePath
+                context: imagePath.href
+            }));
+        }
+    }).catch(function (err) {
+        if (err.statusCode === 404) {
+            return Promise.reject(new errors.NotFoundError({
+                message: 'Image not found.',
+                code: 'IMAGE_SIZE',
+                statusCode: err.statusCode,
+                context: err.url || imagePath.href || imagePath
+            }));
+        } else if (err.code === 'ETIMEDOUT') {
+            return Promise.reject(new errors.InternalServerError({
+                message: 'Request timed out.',
+                code: 'IMAGE_SIZE',
+                statusCode: err.statusCode,
+                context: err.url || imagePath.href || imagePath
             }));
-        });
+        } else {
+            return Promise.reject(new errors.InternalServerError({
+                message: 'Unknown Request error.',
+                code: 'IMAGE_SIZE',
+                statusCode: err.statusCode,
+                context: err.url || imagePath.href || imagePath
+            }));
+        }
     });
 };