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

💄 Improve URL consistency, Part 1: urlJoin (#7668)

Browse source 
refs #7666

Use urlJoin for more consistency instead of concatenating url strings.
This commit is contained in:
Aileen Nowak 2016-11-14 21:38:55 +07:00 • committed by Hannah Wolfe
parent 4a2ddbe2ae
commit 06061d5d6c
18 changed files with 55 additions and 48 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
core/server/api/authentication.js
View file

@ -217,9 +217,7 @@ authentication = {
function sendResetNotification(data) {
var baseUrl = config.get('forceAdminSSL') ? (config.get('urlSSL') || config.get('url')) : config.get('url'),
resetUrl = baseUrl.replace(/\/$/, '') +
'/ghost/reset/' +
globalUtils.encodeBase64URLsafe(data.resetToken) + '/';
resetUrl = globalUtils.url.urlJoin(baseUrl, 'ghost/reset', globalUtils.encodeBase64URLsafe(data.resetToken), '/');
return mail.utils.generateContent({
data: {

14
core/server/api/index.js
View file

@ -93,7 +93,7 @@ cacheInvalidationHeader = function cacheInvalidationHeader(req, result) {
if (hasStatusChanged || wasPublishedUpdated) {
return INVALIDATE_ALL;
} else {
return utils.url.urlFor({relativeUrl: '/' + config.get('routeKeywords').preview + '/' + post.uuid + '/'});
return utils.url.urlFor({relativeUrl: utils.url.urlJoin('/', config.get('routeKeywords').preview, post.uuid, '/')});
}
}
}
@ -113,21 +113,23 @@ cacheInvalidationHeader = function cacheInvalidationHeader(req, result) {
locationHeader = function locationHeader(req, result) {
var apiRoot = utils.url.urlFor('api'),
location,
newObject;
newObject,
statusQuery;
if (req.method === 'POST') {
if (result.hasOwnProperty('posts')) {
newObject = result.posts[0];
location = apiRoot + '/posts/' + newObject.id + '/?status=' + newObject.status;
statusQuery = '/?status=' + newObject.status;
location = utils.url.urlJoin(apiRoot, 'posts', newObject.id, statusQuery);
} else if (result.hasOwnProperty('notifications')) {
newObject = result.notifications[0];
location = apiRoot + '/notifications/' + newObject.id + '/';
location = utils.url.urlJoin(apiRoot, 'notifications', newObject.id, '/');
} else if (result.hasOwnProperty('users')) {
newObject = result.users[0];
location = apiRoot + '/users/' + newObject.id + '/';
location = utils.url.urlJoin(apiRoot, 'users', newObject.id, '/');
} else if (result.hasOwnProperty('tags')) {
newObject = result.tags[0];
location = apiRoot + '/tags/' + newObject.id + '/';
location = utils.url.urlJoin(apiRoot, 'tags', newObject.id, '/');
}
}

2
core/server/api/invites.js
View file

@ -113,7 +113,7 @@ invites = {
invitedByName: loggedInUser.get('name'),
invitedByEmail: loggedInUser.get('email'),
// @TODO: resetLink sounds weird
resetLink: baseUrl.replace(/\/$/, '') + '/ghost/signup/' + globalUtils.encodeBase64URLsafe(invite.get('token')) + '/'
resetLink: globalUtils.url.urlJoin(baseUrl, 'ghost/signup', globalUtils.encodeBase64URLsafe(invite.get('token')), '/')
};
return mail.utils.generateContent({data: emailData, template: 'invite-user'});

3
core/server/api/settings.js
View file

@ -9,6 +9,7 @@ var _ = require('lodash'),
logging = require('../logging'),
utils = require('./utils'),
i18n = require('../i18n'),
generalUtils = require('../utils'),
docName = 'settings',
settings,
@ -64,7 +65,7 @@ updateConfigCache = function () {
config.set('theme:twitter', (settingsCache.twitter && settingsCache.twitter.value) || '');
config.set('theme:facebook', (settingsCache.facebook && settingsCache.facebook.value) || '');
config.set('theme:timezone', (settingsCache.activeTimezone && settingsCache.activeTimezone.value) || config.get('theme').timezone);
config.set('theme:url', config.get('url') ? config.get('url').replace(/\/$/, '') : '');
config.set('theme:url', config.get('url') ? generalUtils.url.urlJoin(config.get('url'), '/') : '');
_.each(labsValue, function (value, key) {
config.set('labs:' + key, value);

2
core/server/api/themes.js
View file

@ -64,7 +64,7 @@ themes = {
});
})
.then(function () {
return storageAdapter.exists(config.getContentPath('themes') + '/' + zip.shortName);
return storageAdapter.exists(utils.url.urlJoin(config.getContentPath('themes'), zip.shortName));
})
.then(function (themeExists) {
// delete existing theme

8
core/server/blog/routes.js
View file

@ -14,19 +14,19 @@ frontendRoutes = function frontendRoutes() {
// ### Admin routes
router.get(/^\/(logout|signout)\/$/, function redirectToSignout(req, res) {
utils.redirect301(res, subdir + '/ghost/signout/');
utils.redirect301(res, utils.url.urlJoin(subdir, '/ghost/signout/'));
});
router.get(/^\/signup\/$/, function redirectToSignup(req, res) {
utils.redirect301(res, subdir + '/ghost/signup/');
utils.redirect301(res, utils.url.urlJoin(subdir, '/ghost/signup/'));
});
// redirect to /ghost and let that do the authentication to prevent redirects to /ghost//admin etc.
router.get(/^\/((ghost-admin|admin|wp-admin|dashboard|signin|login)\/?)$/, function redirectToAdmin(req, res) {
utils.redirect301(res, subdir + '/ghost/');
utils.redirect301(res, utils.url.urlJoin(subdir, '/ghost/'));
});
// Post Live Preview
router.get('/' + routeKeywords.preview + '/:uuid', frontend.preview);
router.get(utils.url.urlJoin('/', routeKeywords.preview, ':uuid'), frontend.preview);
// Channels
router.use(channels.router());

5
core/server/controllers/frontend/channel-config.js
View file

@ -1,5 +1,6 @@
var _ = require('lodash'),
config = require('../../config'),
utils = require('../../utils'),
channelConfig;
channelConfig = function channelConfig() {
@ -11,7 +12,7 @@ channelConfig = function channelConfig() {
},
tag: {
name: 'tag',
route: '/' + config.get('routeKeywords').tag + '/:slug/',
route: utils.url.urlJoin('/', config.get('routeKeywords').tag, ':slug/'),
postOptions: {
filter: 'tags:\'%s\'+tags.visibility:\'public\''
},
@ -27,7 +28,7 @@ channelConfig = function channelConfig() {
},
author: {
name: 'author',
route: '/' + config.get('routeKeywords').author + '/:slug/',
route: utils.url.urlJoin('/', config.get('routeKeywords').author, ':slug/'),
postOptions: {
filter: 'author:\'%s\''
},

8
core/server/controllers/frontend/channels.js
View file

@ -45,9 +45,9 @@ rssRouter = function rssRouter(channelConfig) {
baseRoute = '/rss/';
router.get(baseRoute, stack);
router.get(baseRoute + ':page/', stack);
router.get(utils.url.urlJoin(baseRoute, ':page/'), stack);
router.get('/feed/', function redirectToRSS(req, res) {
return utils.redirect301(res, utils.url.getSubdir() + req.baseUrl + baseRoute);
return utils.redirect301(res, utils.url.urlJoin(utils.url.getSubdir(), req.baseUrl, baseRoute));
});
router.param('page', handlePageParam);
@ -64,7 +64,7 @@ channelRouter = function router() {
var channelsRouter = express.Router({mergeParams: true}),
baseRoute = '/',
pageRoute = '/' + config.get('routeKeywords').page + '/:page/';
pageRoute = utils.url.urlJoin('/', config.get('routeKeywords').page, ':page/');
_.each(channelConfig.list(), function (channel) {
var channelRouter = express.Router({mergeParams: true}),
@ -78,7 +78,7 @@ channelRouter = function router() {
if (channel.editRedirect) {
channelRouter.get('/edit/', function redirect(req, res) {
res.redirect(utils.url.getSubdir() + channel.editRedirect.replace(':slug', req.params.slug));
res.redirect(utils.url.urlJoin(utils.url.getSubdir(), channel.editRedirect.replace(':slug', req.params.slug)));
});
}

3
core/server/controllers/frontend/index.js
View file

@ -71,8 +71,7 @@ frontendControllers = {
// CASE: last param is of url is /edit, redirect to admin
if (lookup.isEditURL) {
return res.redirect(utils.url.getSubdir()
+ '/ghost/editor/' + post.id + '/');
return res.redirect(utils.url.urlJoin(utils.url.getSubdir(), '/ghost/editor', post.id, '/'));
}
// CASE: permalink is not valid anymore, we redirect him permanently to the correct one

5
core/server/data/importer/handlers/image.js
View file

@ -37,9 +37,8 @@ ImageHandler = {
return Promise.map(files, function (image) {
return store.getUniqueFileName(store, image, image.targetDir).then(function (targetFilename) {
image.newPath = (utils.url.getSubdir() + '/' +
config.get('paths').imagesRelPath + '/' + path.relative(config.getContentPath('images'), targetFilename))
.replace(new RegExp('\\' + path.sep, 'g'), '/');
image.newPath = utils.url.urlJoin('/', utils.url.getSubdir(), config.get('paths').imagesRelPath,
path.relative(config.getContentPath('images'), targetFilename));
return image;
});

3
core/server/data/meta/amp_url.js
View file

@ -6,8 +6,7 @@ function getAmplUrl(data) {
var context = data.context ? data.context : null;
if (_.includes(context, 'post') && !_.includes(context, 'amp')) {
return utils.url.urlJoin(utils.url.getBaseUrl(false),
getUrl(data, false)) + 'amp/';
return utils.url.urlJoin(utils.url.getBaseUrl(false), getUrl(data, false), 'amp/');
}
return null;
}

6
core/server/data/meta/asset_url.js
View file

@ -4,14 +4,14 @@ var config = require('../../config'),
function getAssetUrl(path, isAdmin, minify) {
var output = '';
output += utils.url.getSubdir() + '/';
output += utils.url.urlJoin(utils.url.getSubdir(), '/');
if (!path.match(/^favicon\.ico$/) && !path.match(/^shared/) && !path.match(/^asset/)) {
if (isAdmin) {
output += 'ghost/';
output = utils.url.urlJoin(output, 'ghost/');
}
output += 'assets/';
output = utils.url.urlJoin(output, 'assets/');
}
// Get rid of any leading slash on the path

10
core/server/data/meta/paginated_url.js
View file

@ -8,7 +8,7 @@ function getPaginatedUrl(page, data, absolute) {
return null;
}
var pagePath = '/' + config.get('routeKeywords').page + '/',
var pagePath = utils.url.urlJoin('/', config.get('routeKeywords').page, '/'),
// Try to match the base url, as whatever precedes the pagePath
baseUrlPattern = new RegExp('(.+)?(/' + config.get('routeKeywords').page + '/\\d+/)'),
baseUrlMatch = data.relativeUrl.match(baseUrlPattern),
@ -17,18 +17,18 @@ function getPaginatedUrl(page, data, absolute) {
newRelativeUrl;
if (page === 'next' && data.pagination.next) {
newRelativeUrl = pagePath + data.pagination.next + '/';
newRelativeUrl = utils.url.urlJoin(pagePath, data.pagination.next, '/');
} else if (page === 'prev' && data.pagination.prev) {
newRelativeUrl = data.pagination.prev > 1 ? pagePath + data.pagination.prev + '/' : '/';
newRelativeUrl = data.pagination.prev > 1 ? utils.url.urlJoin(pagePath, data.pagination.prev, '/') : '/';
} else if (_.isNumber(page)) {
newRelativeUrl = page > 1 ? pagePath + page + '/' : '/';
newRelativeUrl = page > 1 ? utils.url.urlJoin(pagePath, page, '/') : '/';
} else {
// If none of the cases match, return null right away
return null;
}
// baseUrl can be undefined, if there was nothing preceding the pagePath (e.g. first page of the index channel)
newRelativeUrl = baseUrl ? baseUrl + newRelativeUrl : newRelativeUrl;
newRelativeUrl = baseUrl ? utils.url.urlJoin(baseUrl, newRelativeUrl) : newRelativeUrl;
return utils.url.urlFor({relativeUrl: newRelativeUrl, secure: data.secure}, absolute);
}

3
core/server/data/migration/backup.js
View file

@ -6,12 +6,13 @@ var _ = require('lodash'),
Promise = require('bluebird'),
config = require('../../config'),
exporter = require('../export'),
utils = require('../../utils'),
writeExportFile,
backup;
writeExportFile = function writeExportFile(exportResult) {
var filename = path.resolve(config.get('paths').contentPath + '/data/' + exportResult.filename);
var filename = path.resolve(utils.url.urlJoin(config.get('paths').contentPath, 'data', exportResult.filename));
return Promise.promisify(fs.writeFile)(filename, JSON.stringify(exportResult.data)).return(filename);
};

10
core/server/data/xml/rss/index.js
View file

@ -18,11 +18,11 @@ var crypto = require('crypto'),
feedCache = {};
function isTag(req) {
return req.originalUrl.indexOf('/' + config.get('routeKeywords').tag + '/') !== -1;
return req.originalUrl.indexOf(utils.url.urlJoin('/', config.get('routeKeywords').tag, '/')) !== -1;
}
function isAuthor(req) {
return req.originalUrl.indexOf('/' + config.get('routeKeywords').author + '/') !== -1;
return req.originalUrl.indexOf(utils.url.urlJoin('/', config.get('routeKeywords').author, '/')) !== -1;
}
function handleError(next) {
@ -56,11 +56,11 @@ function getBaseUrl(req, slugParam) {
var baseUrl = utils.url.getSubdir();
if (isTag(req)) {
baseUrl += '/' + config.get('routeKeywords').tag + '/' + slugParam + '/rss/';
baseUrl = utils.url.urlJoin(baseUrl, config.get('routeKeywords').tag, slugParam, 'rss/');
} else if (isAuthor(req)) {
baseUrl += '/' + config.get('routeKeywords').author + '/' + slugParam + '/rss/';
baseUrl = utils.url.urlJoin(baseUrl, config.get('routeKeywords').author, slugParam, 'rss/');
} else {
baseUrl += '/rss/';
baseUrl = utils.url.urlJoin(baseUrl, 'rss/');
}
return baseUrl;

7
core/server/storage/local-file-store.js
View file

@ -37,10 +37,9 @@ LocalFileStore.prototype.save = function save(image, targetDir) {
// The src for the image must be in URI format, not a file system path, which in Windows uses \
// For local file system storage can use relative path so add a slash
var fullUrl = (
utils.url.getSubdir() + '/' +
config.get('paths').imagesRelPath +
'/' +
path.relative(config.getContentPath('images'), targetFilename)
utils.url.urlJoin('/', utils.url.getSubdir(),
config.get('paths').imagesRelPath,
path.relative(config.getContentPath('images'), targetFilename))
).replace(new RegExp('\\' + path.sep, 'g'), '/');
return fullUrl;

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

@ -47,6 +47,14 @@ function getProtectedSlugs() {
}
}
// ## urlJoin
// concats arguments to a path/URL
// Usage:
// urlJoin(getBaseUrl(), 'content', '/') -> http://my-ghost-blog.com/content/
// Returns a URL or relative path
// Only to use for Ghost URLs and paths
// TODO: urlJoin needs to be optimised and to validate the URL/path properly.
// e. g. URLs should end with a trailing `/` at the end of the pathname.
function urlJoin() {
var args = Array.prototype.slice.call(arguments),
prefixDoubleSlash = false,

2
core/test/functional/routes/frontend_spec.js
View file

@ -657,7 +657,7 @@ describe('Frontend Routing', function () {
request.get('/')
.expect(200)
.expect(/<link rel="canonical" href="http:\/\/127.0.0.1:2370\/" \/\>/)
.expect(/<a href="http:\/\/127.0.0.1:2370">Ghost<\/a\>/)
.expect(/<a href="http:\/\/127.0.0.1:2370\/">Ghost<\/a\>/)
.end(doEnd(done));
});