From 991ccb1d3577028c05f3b7ac3d12a9f9e9c9b497 Mon Sep 17 00:00:00 2001 From: kirrg001 Date: Thu, 14 Dec 2017 22:32:34 +0100 Subject: [PATCH] Moved make-absolute-urls to url service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs #9178 - this util uses the url services (!) - moving this file into lib would not make sense right now - that would mean a module requires first ../lib/url, which then requires ../services/url - the url service definitely need a clean up 😃 --- .../apps/amp/lib/helpers/amp_content.js | 3 +- core/server/services/rss/generate-feed.js | 4 +- core/server/services/url/utils.js | 64 ++++++++++++++++++- core/server/utils/make-absolute-urls.js | 61 ------------------ .../url/utils_spec.js} | 54 ++++++++++++++-- .../unit/utils/make-absolute-urls_spec.js | 47 -------------- 6 files changed, 113 insertions(+), 120 deletions(-) delete mode 100644 core/server/utils/make-absolute-urls.js rename core/test/unit/{utils/url_spec.js => services/url/utils_spec.js} (92%) delete mode 100644 core/test/unit/utils/make-absolute-urls_spec.js diff --git a/core/server/apps/amp/lib/helpers/amp_content.js b/core/server/apps/amp/lib/helpers/amp_content.js index ece438ffc7..0ff0b85513 100644 --- a/core/server/apps/amp/lib/helpers/amp_content.js +++ b/core/server/apps/amp/lib/helpers/amp_content.js @@ -15,7 +15,6 @@ var Promise = require('bluebird'), logging = proxy.logging, i18n = proxy.i18n, errors = proxy.errors, - makeAbsoluteUrl = require('../../../../utils/make-absolute-urls'), urlService = require('../../../../services/url'), amperizeCache = {}, allowedAMPTags = [], @@ -126,7 +125,7 @@ function getAmperizeHTML(html, post) { amperize = amperize || new Amperize(); // make relative URLs abolute - html = makeAbsoluteUrl(html, urlService.utils.urlFor('home', true), post.url).html(); + html = urlService.utils.makeAbsoluteUrls(html, urlService.utils.urlFor('home', true), post.url).html(); if (!amperizeCache[post.id] || moment(new Date(amperizeCache[post.id].updated_at)).diff(new Date(post.updated_at)) < 0) { return new Promise(function (resolve) { diff --git a/core/server/services/rss/generate-feed.js b/core/server/services/rss/generate-feed.js index 7005308840..dc11782545 100644 --- a/core/server/services/rss/generate-feed.js +++ b/core/server/services/rss/generate-feed.js @@ -2,8 +2,6 @@ var downsize = require('downsize'), RSS = require('rss'), urlService = require('../../services/url'), filters = require('../../filters'), - processUrls = require('../../utils/make-absolute-urls'), - generateFeed, generateItem, generateTags; @@ -23,7 +21,7 @@ generateTags = function generateTags(data) { generateItem = function generateItem(post, siteUrl, secure) { var itemUrl = urlService.utils.urlFor('post', {post: post, secure: secure}, true), - htmlContent = processUrls(post.html, siteUrl, itemUrl), + htmlContent = urlService.utils.makeAbsoluteUrls(post.html, siteUrl, itemUrl), item = { title: post.title, // @TODO: DRY this up with data/meta/index & other excerpt code diff --git a/core/server/services/url/utils.js b/core/server/services/url/utils.js index 8cf7202642..c075b234a8 100644 --- a/core/server/services/url/utils.js +++ b/core/server/services/url/utils.js @@ -1,9 +1,11 @@ +'use strict'; + // Contains all path information to be used throughout the codebase. // Assumes that config.url is set, and is valid - -var moment = require('moment-timezone'), +const moment = require('moment-timezone'), _ = require('lodash'), url = require('url'), + cheerio = require('cheerio'), config = require('../../config'), settingsCache = require('../settings/cache'), // @TODO: unify this with the path in server/app.js @@ -365,6 +367,64 @@ function redirectToAdmin(status, res, adminPath) { return res.redirect(redirectUrl); } +/** + * Make absolute URLs + * @param {string} html + * @param {string} siteUrl (blog URL) + * @param {string} itemUrl (URL of current context) + * @returns {object} htmlContent + * @description Takes html, blog url and item url and converts relative url into + * absolute urls. Returns an object. The html string can be accessed by calling `html()` on + * the variable that takes the result of this function + */ +function makeAbsoluteUrls(html, siteUrl, itemUrl) { + var htmlContent = cheerio.load(html, {decodeEntities: false}); + + // convert relative resource urls to absolute + ['href', 'src'].forEach(function forEach(attributeName) { + htmlContent('[' + attributeName + ']').each(function each(ix, el) { + var baseUrl, + attributeValue, + parsed; + + el = htmlContent(el); + + attributeValue = el.attr(attributeName); + + // if URL is absolute move on to the next element + try { + parsed = url.parse(attributeValue); + + if (parsed.protocol) { + return; + } + + // Do not convert protocol relative URLs + if (attributeValue.lastIndexOf('//', 0) === 0) { + return; + } + } catch (e) { + return; + } + + // CASE: don't convert internal links + if (attributeValue[0] === '#') { + return; + } + // compose an absolute URL + + // if the relative URL begins with a '/' use the blog URL (including sub-directory) + // as the base URL, otherwise use the post's URL. + baseUrl = attributeValue[0] === '/' ? siteUrl : itemUrl; + attributeValue = urlJoin(baseUrl, attributeValue); + el.attr(attributeName, attributeValue); + }); + }); + + return htmlContent; +} + +module.exports.makeAbsoluteUrls = makeAbsoluteUrls; module.exports.getProtectedSlugs = getProtectedSlugs; module.exports.getSubdir = getSubdir; module.exports.urlJoin = urlJoin; diff --git a/core/server/utils/make-absolute-urls.js b/core/server/utils/make-absolute-urls.js deleted file mode 100644 index c661b97661..0000000000 --- a/core/server/utils/make-absolute-urls.js +++ /dev/null @@ -1,61 +0,0 @@ -var cheerio = require('cheerio'), - url = require('url'), - urlService = require('../services/url'); - -/** - * Make absolute URLs - * @param {string} html - * @param {string} siteUrl (blog URL) - * @param {string} itemUrl (URL of current context) - * @returns {object} htmlContent - * @description Takes html, blog url and item url and converts relative url into - * absolute urls. Returns an object. The html string can be accessed by calling `html()` on - * the variable that takes the result of this function - */ -function makeAbsoluteUrls(html, siteUrl, itemUrl) { - var htmlContent = cheerio.load(html, {decodeEntities: false}); - // convert relative resource urls to absolute - ['href', 'src'].forEach(function forEach(attributeName) { - htmlContent('[' + attributeName + ']').each(function each(ix, el) { - var baseUrl, - attributeValue, - parsed; - - el = htmlContent(el); - - attributeValue = el.attr(attributeName); - - // if URL is absolute move on to the next element - try { - parsed = url.parse(attributeValue); - - if (parsed.protocol) { - return; - } - - // Do not convert protocol relative URLs - if (attributeValue.lastIndexOf('//', 0) === 0) { - return; - } - } catch (e) { - return; - } - - // CASE: don't convert internal links - if (attributeValue[0] === '#') { - return; - } - // compose an absolute URL - - // if the relative URL begins with a '/' use the blog URL (including sub-directory) - // as the base URL, otherwise use the post's URL. - baseUrl = attributeValue[0] === '/' ? siteUrl : itemUrl; - attributeValue = urlService.utils.urlJoin(baseUrl, attributeValue); - el.attr(attributeName, attributeValue); - }); - }); - - return htmlContent; -} - -module.exports = makeAbsoluteUrls; diff --git a/core/test/unit/utils/url_spec.js b/core/test/unit/services/url/utils_spec.js similarity index 92% rename from core/test/unit/utils/url_spec.js rename to core/test/unit/services/url/utils_spec.js index 1712b3c141..c048fad9c6 100644 --- a/core/test/unit/utils/url_spec.js +++ b/core/test/unit/services/url/utils_spec.js @@ -3,11 +3,11 @@ var should = require('should'), sinon = require('sinon'), _ = require('lodash'), moment = require('moment-timezone'), - urlService = require('../../../server/services/url'), - constants = require('../../../server/lib/constants'), - settingsCache = require('../../../server/services/settings/cache'), - configUtils = require('../../utils/configUtils'), - testUtils = require('../../utils'), + urlService = require('../../../../server/services/url'), + constants = require('../../../../server/lib/constants'), + settingsCache = require('../../../../server/services/settings/cache'), + configUtils = require('../../../utils/configUtils'), + testUtils = require('../../../utils'), config = configUtils.config, sandbox = sinon.sandbox.create(); @@ -721,4 +721,48 @@ describe('Url', function () { urlService.utils.redirectToAdmin(302, res, '#/my/awesome/path'); }); }); + + describe('make absolute urls ', function () { + var siteUrl = 'http://my-ghost-blog.com', + itemUrl = 'my-awesome-post'; + + beforeEach(function () { + configUtils.set({url: 'http://my-ghost-blog.com'}); + }); + + afterEach(function () { + configUtils.restore(); + }); + + it('[success] does not convert absolute URLs', function () { + var html = '', + result = urlService.utils.makeAbsoluteUrls(html, siteUrl, itemUrl).html(); + + result.should.match(//); + }); + it('[failure] does not convert protocol relative `//` URLs', function () { + var html = '', + result = urlService.utils.makeAbsoluteUrls(html, siteUrl, itemUrl).html(); + + result.should.match(//); + }); + it('[failure] does not convert internal links starting with "#"', function () { + var html = '', + result = urlService.utils.makeAbsoluteUrls(html, siteUrl, itemUrl).html(); + + result.should.match(//); + }); + it('[success] converts a relative URL', function () { + var html = '', + result = urlService.utils.makeAbsoluteUrls(html, siteUrl, itemUrl).html(); + + result.should.match(//); + }); + it('[success] converts a relative URL including subdirectories', function () { + var html = '', + result = urlService.utils.makeAbsoluteUrls(html, 'http://my-ghost-blog.com/blog', itemUrl).html(); + + result.should.match(//); + }); + }); }); diff --git a/core/test/unit/utils/make-absolute-urls_spec.js b/core/test/unit/utils/make-absolute-urls_spec.js deleted file mode 100644 index e2df0364f9..0000000000 --- a/core/test/unit/utils/make-absolute-urls_spec.js +++ /dev/null @@ -1,47 +0,0 @@ -var should = require('should'), // jshint ignore:line - makeAbsoluteUrls = require('../../../server/utils/make-absolute-urls'), - configUtils = require('../../utils/configUtils'); - -describe('Make absolute URLs ', function () { - var siteUrl = 'http://my-ghost-blog.com', - itemUrl = 'my-awesome-post'; - - beforeEach(function () { - configUtils.set({url: 'http://my-ghost-blog.com'}); - }); - - afterEach(function () { - configUtils.restore(); - }); - - it('[success] does not convert absolute URLs', function () { - var html = '', - result = makeAbsoluteUrls(html, siteUrl, itemUrl).html(); - - result.should.match(//); - }); - it('[failure] does not convert protocol relative `//` URLs', function () { - var html = '', - result = makeAbsoluteUrls(html, siteUrl, itemUrl).html(); - - result.should.match(//); - }); - it('[failure] does not convert internal links starting with "#"', function () { - var html = '', - result = makeAbsoluteUrls(html, siteUrl, itemUrl).html(); - - result.should.match(//); - }); - it('[success] converts a relative URL', function () { - var html = '', - result = makeAbsoluteUrls(html, siteUrl, itemUrl).html(); - - result.should.match(//); - }); - it('[success] converts a relative URL including subdirectories', function () { - var html = '', - result = makeAbsoluteUrls(html, 'http://my-ghost-blog.com/blog', itemUrl).html(); - - result.should.match(//); - }); -});