Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-04-08 02:52:39 -05:00
Code Issues Activity

Increased URL utility coverage to 100% (#9201)

Browse source 
refs #9192

To anyone seeing this go by - I'm about to start some fairly major refactoring work on the url utility. Before I do that, I wanted to make sure I had 100% coverage, and understanding of some of the weird cases.

The majority of the changes I've made are adding tests, but I was also able to clean up a little bit, remove a few lines or change them to make use of other tools.
This commit is contained in:
Hannah Wolfe 2017-11-02 20:35:58 +00:00 • committed by GitHub
parent 89e57ad6b2
commit 4ee522069c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 107 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

40
core/server/utils/url.js
View file

@ -1,5 +1,5 @@
// Contains all path information to be used throughout
// the codebase.
// Contains all path information to be used throughout the codebase.
// Assumes that config.url is set, and is valid
var moment = require('moment-timezone'),
_ = require('lodash'),
@ -41,16 +41,13 @@ function getBlogUrl(secure) {
* @return {string} URL a subdirectory if configured.
*/
function getSubdir() {
var localPath, subdir;
// Parse local path location
if (config.get('url')) {
localPath = url.parse(config.get('url')).path;
var localPath = url.parse(config.get('url')).path,
subdir;
// Remove trailing slash
if (localPath !== '/') {
localPath = localPath.replace(/\/$/, '');
}
// Remove trailing slash
if (localPath !== '/') {
localPath = localPath.replace(/\/$/, '');
}
subdir = localPath === '/' ? '' : localPath;
@ -274,32 +271,23 @@ function urlFor(context, data, absolute) {
urlPath = data.nav.url;
secure = data.nav.secure || secure;
baseUrl = getBlogUrl(secure);
hostname = baseUrl.split('//')[1] + getSubdir();
hostname = baseUrl.split('//')[1];
// If the hostname is present in the url
if (urlPath.indexOf(hostname) > -1
// do no not apply, if there is a subdomain, or a mailto link
&& !urlPath.split(hostname)[0].match(/\.|mailto:/)
// do not apply, if there is a port after the hostname
&& urlPath.split(hostname)[1].substring(0, 1) !== ':') {
// make link relative to account for possible
// mismatch in http/https etc, force absolute
// do not do so if link is a subdomain of blog url
// or if hostname is inside of the slug
// or if slug is a port
// make link relative to account for possible mismatch in http/https etc, force absolute
urlPath = urlPath.split(hostname)[1];
if (urlPath.substring(0, 1) !== '/') {
urlPath = '/' + urlPath;
}
urlPath = urlJoin('/', urlPath);
absolute = true;
}
}
} else if (context === 'home' && absolute) {
urlPath = getBlogUrl(secure);
// CASE: with or without protocol?
// @TODO: rename cors
if (data && data.cors) {
urlPath = urlPath.replace(/^.*?:\/\//g, '//');
}
// CASE: there are cases where urlFor('home') needs to be returned without trailing
// slash e. g. the `{{@blog.url}}` helper. See https://github.com/TryGhost/Ghost/issues/8569
if (data && data.trailingSlash === false) {
@ -332,7 +320,7 @@ function urlFor(context, data, absolute) {
}
} else if (_.isString(context) && _.indexOf(_.keys(knownPaths), context) !== -1) {
// trying to create a url for a named path
urlPath = knownPaths[context] || '/';
urlPath = knownPaths[context];
}
// This url already has a protocol so is likely an external url to be returned

103
core/test/unit/utils/url_spec.js
View file

@ -159,6 +159,18 @@ describe('Url', function () {
utils.url.urlFor(testContext, true).should.equal('http://my-ghost-blog.com/blog/rss/');
});
it('should handle weird cases by always returning /', function () {
utils.url.urlFor('').should.equal('/');
utils.url.urlFor('post', {}).should.equal('/');
utils.url.urlFor('post', {post: {}}).should.equal('/');
utils.url.urlFor(null).should.equal('/');
utils.url.urlFor(undefined).should.equal('/');
utils.url.urlFor({}).should.equal('/');
utils.url.urlFor({relativeUrl: ''}).should.equal('/');
utils.url.urlFor({relativeUrl: null}).should.equal('/');
utils.url.urlFor({relativeUrl: undefined}).should.equal('/');
});
it('should return url for a random path when asked for', function () {
var testContext = {relativeUrl: '/about/'};
@ -275,10 +287,13 @@ describe('Url', function () {
configUtils.set({url: 'http://my-ghost-blog.com'});
testData = {nav: {url: 'http://my-ghost-blog.com/'}};
utils.url.urlFor(testContext, testData).should.equal('http://my-ghost-blog.com/');
testData = {nav: {url: 'http://my-ghost-blog.com/short-and-sweet/'}};
utils.url.urlFor(testContext, testData).should.equal('http://my-ghost-blog.com/short-and-sweet/');
testData = {nav: {url: 'http://my-ghost-blog.com/short-and-sweet/'}, secure: true};
testData = {nav: {url: 'http://my-ghost-blog.com//short-and-sweet/'}, secure: true};
utils.url.urlFor(testContext, testData).should.equal('https://my-ghost-blog.com/short-and-sweet/');
testData = {nav: {url: 'http://my-ghost-blog.com:3000/'}};
@ -305,13 +320,27 @@ describe('Url', function () {
testData = {nav: {url: 'http://some-external-page.com/stuff-my-ghost-blog.com-around'}};
utils.url.urlFor(testContext, testData).should.equal('http://some-external-page.com/stuff-my-ghost-blog.com-around');
testData = {nav: {url: 'mailto:marshmallow@my-ghost-blog.com'}};
utils.url.urlFor(testContext, testData).should.equal('mailto:marshmallow@my-ghost-blog.com');
configUtils.set({url: 'http://my-ghost-blog.com/blog'});
testData = {nav: {url: 'http://my-ghost-blog.com/blog/'}};
utils.url.urlFor(testContext, testData).should.equal('http://my-ghost-blog.com/blog/');
testData = {nav: {url: 'http://my-ghost-blog.com/blog/short-and-sweet/'}};
utils.url.urlFor(testContext, testData).should.equal('http://my-ghost-blog.com/blog/short-and-sweet/');
configUtils.set({url: 'http://my-ghost-blog.com/'});
testData = {nav: {url: 'mailto:marshmallow@my-ghost-blog.com'}};
utils.url.urlFor(testContext, testData).should.equal('mailto:marshmallow@my-ghost-blog.com');
testData = {nav: {url: 'http://my-ghost-blog.com:3000/blog/'}};
utils.url.urlFor(testContext, testData).should.equal('http://my-ghost-blog.com:3000/blog/');
testData = {nav: {url: 'http://my-ghost-blog.com:3000/blog/short-and-sweet/'}};
utils.url.urlFor(testContext, testData).should.equal('http://my-ghost-blog.com:3000/blog/short-and-sweet/');
testData = {nav: {url: 'http://sub.my-ghost-blog.com/blog/'}};
utils.url.urlFor(testContext, testData).should.equal('http://sub.my-ghost-blog.com/blog/');
testData = {nav: {url: '//sub.my-ghost-blog.com/blog/'}};
utils.url.urlFor(testContext, testData).should.equal('//sub.my-ghost-blog.com/blog/');
});
it('sitemap: should return other known paths when requested', function () {
@ -434,6 +463,18 @@ describe('Url', function () {
utils.url.urlFor('api', true).should.eql('http://my-ghost-blog.com/blog/ghost/api/v0.1/');
});
it('api: relative path is correct', function () {
utils.url.urlFor('api').should.eql('/ghost/api/v0.1/');
});
it('api: relative path with subdir is correct', function () {
configUtils.set({
url: 'http://my-ghost-blog.com/blog'
});
utils.url.urlFor('api').should.eql('/blog/ghost/api/v0.1/');
});
it('api: should return http if config.url is http', function () {
configUtils.set({
url: 'http://my-ghost-blog.com'
@ -450,7 +491,7 @@ describe('Url', function () {
utils.url.urlFor('api', true).should.eql('https://my-ghost-blog.com/ghost/api/v0.1/');
});
it('[api url] blog url is http: cors should return no protocol', function () {
it('api: with cors, blog url is http: should return no protocol', function () {
configUtils.set({
url: 'http://my-ghost-blog.com'
});
@ -458,7 +499,7 @@ describe('Url', function () {
utils.url.urlFor('api', {cors: true}, true).should.eql('//my-ghost-blog.com/ghost/api/v0.1/');
});
it('[api url] admin url is http: cors should return no protocol', function () {
it('api: with cors, admin url is http: cors should return no protocol', function () {
configUtils.set({
url: 'http://my-ghost-blog.com',
admin: {
@ -469,7 +510,7 @@ describe('Url', function () {
utils.url.urlFor('api', {cors: true}, true).should.eql('//admin.ghost.example/ghost/api/v0.1/');
});
it('[api url] admin url is https: should return with protocol', function () {
it('api: with cors, admin url is https: should return with protocol', function () {
configUtils.set({
url: 'https://my-ghost-blog.com',
admin: {
@ -480,7 +521,7 @@ describe('Url', function () {
utils.url.urlFor('api', {cors: true}, true).should.eql('https://admin.ghost.example/ghost/api/v0.1/');
});
it('[api url] blog url is https: should return with protocol', function () {
it('api: with cors, blog url is https: should return with protocol', function () {
configUtils.set({
url: 'https://my-ghost-blog.com'
});
@ -543,7 +584,7 @@ describe('Url', function () {
localSettingsCache.active_timezone = 'America/Los_Angeles';
localSettingsCache.permalinks = '/:year/:id/:author/';
var testData = _.merge(testUtils.DataGenerator.Content.posts[2], {id: 3}, {author: {slug: 'joe-blog'}}),
var testData = _.merge({}, testUtils.DataGenerator.Content.posts[2], {id: 3}, {author: {slug: 'joe-blog'}}),
postLink = '/2015/3/joe-blog/';
testData.published_at = new Date('2016-01-01T00:00:00.000Z');
@ -554,13 +595,47 @@ describe('Url', function () {
localSettingsCache.active_timezone = 'Europe/Berlin';
localSettingsCache.permalinks = '/:year/:id/:author/';
var testData = _.merge(testUtils.DataGenerator.Content.posts[2], {id: 3}, {author: {slug: 'joe-blog'}}),
var testData = _.merge({}, testUtils.DataGenerator.Content.posts[2], {id: 3}, {author: {slug: 'joe-blog'}}),
postLink = '/2016/3/joe-blog/';
testData.published_at = new Date('2016-01-01T00:00:00.000Z');
utils.url.urlPathForPost(testData).should.equal(postLink);
});
it('permalink is /:primary_tag/:slug/ and there is a primary_tag', function () {
localSettingsCache.active_timezone = 'Europe/Berlin';
localSettingsCache.permalinks = '/:primary_tag/:slug/';
var testData = _.merge({}, testUtils.DataGenerator.Content.posts[2], {primary_tag: {slug: 'bitcoin'}}),
postLink = '/bitcoin/short-and-sweet/';
testData.published_at = new Date('2016-01-01T00:00:00.000Z');
utils.url.urlPathForPost(testData).should.equal(postLink);
});
it('permalink is /:primary_tag/:slug/ and there is NO primary_tag', function () {
localSettingsCache.active_timezone = 'Europe/Berlin';
localSettingsCache.permalinks = '/:primary_tag/:slug/';
var testData = testUtils.DataGenerator.Content.posts[2],
postLink = '/all/short-and-sweet/';
testData.published_at = new Date('2016-01-01T00:00:00.000Z');
utils.url.urlPathForPost(testData).should.equal(postLink);
});
it('shows "undefined" for unknown route segments', function () {
localSettingsCache.active_timezone = 'Europe/Berlin';
localSettingsCache.permalinks = '/:tag/:slug/';
var testData = testUtils.DataGenerator.Content.posts[2],
// @TODO: is this the correct behaviour?
postLink = '/undefined/short-and-sweet/';
testData.published_at = new Date('2016-01-01T00:00:00.000Z');
utils.url.urlPathForPost(testData).should.equal(postLink);
});
it('post is not published yet', function () {
localSettingsCache.active_timezone = 'Europe/London';
localSettingsCache.permalinks = '/:year/:month/:day/:slug/';
@ -577,6 +652,14 @@ describe('Url', function () {
});
});
describe('isSSL', function () {
it('detects https protocol correctly', function () {
utils.url.isSSL('https://my.blog.com').should.be.true();
utils.url.isSSL('http://my.blog.com').should.be.false();
utils.url.isSSL('http://my.https.com').should.be.false();
});
});
describe('redirects', function () {
it('performs 301 redirect correctly', function (done) {
var res = {};