From 6a411049690757f627d362f94d08a47687368ad2 Mon Sep 17 00:00:00 2001 From: Aileen Nowak Date: Thu, 9 Nov 2017 17:11:54 +0700 Subject: [PATCH] Moved `isLocalFile` fn to storage utils (#8985) no issue - moved isLocalImage fn to storage utils used the RegExp of getLocalFileStoragePath to detect also relative image paths and added tests. - Added test for independent protocol request (skip, because not supported/implemented) --- core/server/adapters/storage/utils.js | 33 ++++-- core/server/utils/image-size.js | 17 +-- core/test/unit/adapters/storage/utils_spec.js | 106 ++++++++++++++++++ 3 files changed, 132 insertions(+), 24 deletions(-) diff --git a/core/server/adapters/storage/utils.js b/core/server/adapters/storage/utils.js index c65d39d5c6..cb4502b452 100644 --- a/core/server/adapters/storage/utils.js +++ b/core/server/adapters/storage/utils.js @@ -6,6 +6,7 @@ var globalUtils = require('../../utils'); /** * Sanitizes a given URL or path for an image to be readable by the local file storage + * as storage needs the path without `/content/images/` prefix * Always returns {string} url * @param {string} imagePath * @returns {string} imagePath @@ -13,15 +14,31 @@ var globalUtils = require('../../utils'); * for the local file storage. */ exports.getLocalFileStoragePath = function getLocalFileStoragePath(imagePath) { - if (imagePath.match(new RegExp('^' + globalUtils.url.urlJoin(globalUtils.url.urlFor('home', true), globalUtils.url.getSubdir(), '/', globalUtils.url.STATIC_IMAGE_URL_PREFIX)))) { - // Storage needs the path without `/content/images/` prefix - // The '/' in urlJoin is necessary to add the '/' to `content/images`, if no subdirectory is setup - return imagePath.replace(new RegExp('^' + globalUtils.url.urlJoin(globalUtils.url.urlFor('home', true), globalUtils.url.getSubdir(), '/', globalUtils.url.STATIC_IMAGE_URL_PREFIX)), ''); - } else if (imagePath.match(new RegExp('^' + globalUtils.url.urlJoin(globalUtils.url.getSubdir(), '/', globalUtils.url.STATIC_IMAGE_URL_PREFIX)))) { - // Storage needs the path without `/content/images/` prefix - // The '/' in urlJoin is necessary to add the '/' to `content/images`, if no subdirectory is setup - return imagePath.replace(new RegExp('^' + globalUtils.url.urlJoin(globalUtils.url.getSubdir(), '/', globalUtils.url.STATIC_IMAGE_URL_PREFIX)), ''); + // The '/' in urlJoin is necessary to add the '/' to `content/images`, if no subdirectory is setup + var urlRegExp = new RegExp('^' + globalUtils.url.urlJoin(globalUtils.url.urlFor('home', true), globalUtils.url.getSubdir(), '/', globalUtils.url.STATIC_IMAGE_URL_PREFIX)), + filePathRegExp = new RegExp('^' + globalUtils.url.urlJoin(globalUtils.url.getSubdir(), '/', globalUtils.url.STATIC_IMAGE_URL_PREFIX)); + + if (imagePath.match(urlRegExp)) { + return imagePath.replace(urlRegExp, ''); + } else if (imagePath.match(filePathRegExp)) { + return imagePath.replace(filePathRegExp, ''); } else { return imagePath; } }; + +/** + * @description compares the imagePath with a regex that reflects our local file storage + * @param {String} imagePath as URL or filepath + * @returns {Boolean} + */ + +exports.isLocalImage = function isLocalImage(imagePath) { + var localImagePath = this.getLocalFileStoragePath(imagePath); + + if (localImagePath !== imagePath) { + return true; + } else { + return false; + } +}; diff --git a/core/server/utils/image-size.js b/core/server/utils/image-size.js index 083465e5ac..e4a7aaf713 100644 --- a/core/server/utils/image-size.js +++ b/core/server/utils/image-size.js @@ -12,21 +12,6 @@ var debug = require('ghost-ignition').debug('utils:image-size'), getImageSizeFromUrl, getImageSizeFromFilePath; -/** - * @description compares the imagePath with a regex that reflects our local file storage - * @param {String} imagePath as URL or filepath - * @returns {Array} if match is true or null if not - */ -function isLocalImage(imagePath) { - imagePath = utils.url.urlFor('image', {image: imagePath}, true); - - if (imagePath) { - return imagePath.match(new RegExp('^' + utils.url.urlJoin(utils.url.urlFor('home', true), utils.url.getSubdir(), '/', utils.url.STATIC_IMAGE_URL_PREFIX))); - } else { - return false; - } -} - /** * @description processes the Buffer result of an image file * @param {Object} options @@ -93,7 +78,7 @@ getImageSizeFromUrl = function getImageSizeFromUrl(imagePath) { parsedUrl, timeout = config.get('times:getImageSizeTimeoutInMS') || 10000; - if (isLocalImage(imagePath)) { + if (storageUtils.isLocalImage(imagePath)) { // don't make a request for a locally stored image return getImageSizeFromFilePath(imagePath); } diff --git a/core/test/unit/adapters/storage/utils_spec.js b/core/test/unit/adapters/storage/utils_spec.js index 480fd72a9f..be88ccbc4f 100644 --- a/core/test/unit/adapters/storage/utils_spec.js +++ b/core/test/unit/adapters/storage/utils_spec.js @@ -34,6 +34,23 @@ describe('storage utils', function () { result.should.be.equal('/2017/07/ghost-logo.png'); }); + // Very unlikely that this is necessary, because Ghost will redirect the request beforehand. + // See https://github.com/TryGhost/Ghost/blob/master/core/server/middleware/url-redirects.js#L76 + // TODO: Change the code to make this test work + it.skip('should return local file storage path for https request, when blog setup as http', function () { + var url = 'https://myblog.com/content/images/2017/07/ghost-logo.png', + result; + + urlForStub = sandbox.stub(utils.url, 'urlFor'); + urlForStub.withArgs('home').returns('http://myblog.com/'); + urlGetSubdirStub = sandbox.stub(utils.url, 'getSubdir'); + urlGetSubdirStub.returns(''); + + result = storageUtils.getLocalFileStoragePath(url); + should.exist(result); + result.should.be.equal('/2017/07/ghost-logo.png'); + }); + it('should return local file storage path for absolute URL with subdirectory', function () { var url = 'http://myblog.com/blog/content/images/2017/07/ghost-logo.png', result; @@ -90,4 +107,93 @@ describe('storage utils', function () { result.should.be.equal('http://example-blog.com/ghost-logo.png'); }); }); + + describe('fn: isLocalImage', function () { + it('should return true when absolute URL and local file', function () { + var url = 'http://myblog.com/content/images/2017/07/ghost-logo.png', + result; + + urlForStub = sandbox.stub(utils.url, 'urlFor'); + urlForStub.withArgs('home').returns('http://myblog.com/'); + urlGetSubdirStub = sandbox.stub(utils.url, 'getSubdir'); + urlGetSubdirStub.returns(''); + + result = storageUtils.isLocalImage(url); + should.exist(result); + result.should.be.equal(true); + }); + + // Very unlikely that this is necessary, because Ghost will redirect the request beforehand. + // See https://github.com/TryGhost/Ghost/blob/master/core/server/middleware/url-redirects.js#L76 + // TODO: Change the code to make this test work + it.skip('should return local file storage path for https request, when blog setup as http', function () { + var url = 'https://myblog.com/content/images/2017/07/ghost-logo.png', + result; + + urlForStub = sandbox.stub(utils.url, 'urlFor'); + urlForStub.withArgs('home').returns('http://myblog.com/'); + urlGetSubdirStub = sandbox.stub(utils.url, 'getSubdir'); + urlGetSubdirStub.returns(''); + + result = storageUtils.isLocalImage(url); + should.exist(result); + result.should.be.equal(true); + }); + + it('should return true when absolute URL with subdirectory and local file', function () { + var url = 'http://myblog.com/blog/content/images/2017/07/ghost-logo.png', + result; + + urlForStub = sandbox.stub(utils.url, 'urlFor'); + urlForStub.withArgs('home').returns('http://myblog.com/'); + urlGetSubdirStub = sandbox.stub(utils.url, 'getSubdir'); + urlGetSubdirStub.returns('/blog'); + + result = storageUtils.isLocalImage(url); + should.exist(result); + result.should.be.equal(true); + }); + + it('should return true when relative URL and local file', function () { + var url = '/content/images/2017/07/ghost-logo.png', + result; + + urlForStub = sandbox.stub(utils.url, 'urlFor'); + urlForStub.withArgs('home').returns('http://myblog.com/'); + urlGetSubdirStub = sandbox.stub(utils.url, 'getSubdir'); + urlGetSubdirStub.returns(''); + + result = storageUtils.isLocalImage(url); + should.exist(result); + result.should.be.equal(true); + }); + + it('should return true when relative URL and local file', function () { + var url = '/blog/content/images/2017/07/ghost-logo.png', + result; + + urlForStub = sandbox.stub(utils.url, 'urlFor'); + urlForStub.withArgs('home').returns('http://myblog.com/'); + urlGetSubdirStub = sandbox.stub(utils.url, 'getSubdir'); + urlGetSubdirStub.returns('/blog'); + + result = storageUtils.isLocalImage(url); + should.exist(result); + result.should.be.equal(true); + }); + + it('should return false when no local file', function () { + var url = 'http://somewebsite.com/ghost-logo.png', + result; + + urlForStub = sandbox.stub(utils.url, 'urlFor'); + urlForStub.withArgs('home').returns('http://myblog.com/'); + urlGetSubdirStub = sandbox.stub(utils.url, 'getSubdir'); + urlGetSubdirStub.returns(''); + + result = storageUtils.isLocalImage(url); + should.exist(result); + result.should.be.equal(false); + }); + }); });