Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-01-06 22:40:14 -05:00
Code Issues Activity

urlencode navigation URLs rather than HTML escape (#7836)

Browse source 
closes #7826

- expose raw url value inside `{{navigation}}` helper
- modify `{{url}}` helper to urlencode values and mark as HTML-safe to avoid Handlebars additional HTML-escaping
This commit is contained in:
Kevin Ansfield 2017-01-11 10:45:56 +00:00 committed by kirrg001
parent 49e99c5dfd
commit 8d88f5b6a5
4 changed files with 93 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
core/server/helpers/navigation.js
View file

@ -66,7 +66,7 @@ navigation = function (options) {
out.current = _isCurrentUrl(e.url, currentUrl);
out.label = e.label;
out.slug = _slugify(e.label);
out.url = hbs.handlebars.Utils.escapeExpression(e.url);
out.url = e.url;
out.secure = self.secure;
return out;
});

10
core/server/helpers/url.js
View file

@ -4,12 +4,16 @@
// Returns the URL for the current object scope i.e. If inside a post scope will return post permalink
// `absolute` flag outputs absolute URL, else URL is relative
var getMetaDataUrl = require('../data/meta/url');
var hbs = require('express-hbs'),
getMetaDataUrl = require('../data/meta/url');
function url(options) {
var absolute = options && options.hash.absolute;
var absolute = options && options.hash.absolute,
url = getMetaDataUrl(this, absolute);
return getMetaDataUrl(this, absolute);
url = encodeURI(decodeURI(url));
return new hbs.SafeString(url);
}
module.exports = url;

37
core/test/unit/server_helpers/navigation_spec.js
View file

@ -143,6 +143,43 @@ describe('{{navigation}} helper', function () {
rendered.string.should.containEql('nav-foo nav-current');
rendered.string.should.containEql('nav-bar"');
});
it('doesn\'t html-escape URLs', function () {
var firstItem = {label: 'Foo', url: '/?foo=bar&baz=qux'},
rendered;
optionsData.data.blog.navigation = [firstItem];
rendered = helpers.navigation(optionsData);
should.exist(rendered);
rendered.string.should.not.containEql('=');
rendered.string.should.not.containEql('&');
rendered.string.should.containEql('/?foo=bar&baz=qux');
});
it('encodes URLs', function () {
var firstItem = {label: 'Foo', url: '/?foo=space bar&<script>alert("gotcha")</script>'},
rendered;
optionsData.data.blog.navigation = [firstItem];
rendered = helpers.navigation(optionsData);
should.exist(rendered);
rendered.string.should.containEql('foo=space%20bar');
rendered.string.should.not.containEql('<script>alert("gotcha")</script>');
rendered.string.should.containEql('%3Cscript%3Ealert(%22gotcha%22)%3C/script%3E');
});
it('doesn\'t double-encode URLs', function () {
var firstItem = {label: 'Foo', url: '/?foo=space%20bar'},
rendered;
optionsData.data.blog.navigation = [firstItem];
rendered = helpers.navigation(optionsData);
should.exist(rendered);
rendered.string.should.not.containEql('foo=space%2520bar');
});
});
describe('{{navigation}} helper with custom template', function () {

75
core/test/unit/server_helpers/url_spec.js
View file

@ -49,7 +49,7 @@ describe('{{url}} helper', function () {
});
should.exist(rendered);
rendered.should.equal('/slug/');
rendered.string.should.equal('/slug/');
});
it('should output an absolute URL if the option is present', function () {
@ -59,7 +59,7 @@ describe('{{url}} helper', function () {
);
should.exist(rendered);
rendered.should.equal('http://testurl.com/slug/');
rendered.string.should.equal('http://testurl.com/slug/');
});
it('should output an absolute URL with https if the option is present and secure', function () {
@ -70,7 +70,7 @@ describe('{{url}} helper', function () {
);
should.exist(rendered);
rendered.should.equal('https://testurl.com/slug/');
rendered.string.should.equal('https://testurl.com/slug/');
});
it('should output an absolute URL with https if secure', function () {
@ -81,7 +81,7 @@ describe('{{url}} helper', function () {
);
should.exist(rendered);
rendered.should.equal('https://testurl.com/slug/');
rendered.string.should.equal('https://testurl.com/slug/');
});
it('should return the slug with a prefixed /tag/ if the context is a tag', function () {
@ -93,32 +93,32 @@ describe('{{url}} helper', function () {
});
should.exist(rendered);
rendered.should.equal('/tag/the-tag/');
rendered.string.should.equal('/tag/the-tag/');
});
it('should return / if not a post or tag', function () {
rendered = helpers.url.call({markdown: 'ff', title: 'title', slug: 'slug'});
should.exist(rendered);
rendered.should.equal('/');
rendered.string.should.equal('/');
rendered = helpers.url.call({html: 'content', title: 'title', slug: 'slug'});
should.exist(rendered);
rendered.should.equal('/');
rendered.string.should.equal('/');
rendered = helpers.url.call({html: 'content', markdown: 'ff', slug: 'slug'});
should.exist(rendered);
rendered.should.equal('/');
rendered.string.should.equal('/');
rendered = helpers.url.call({html: 'content', markdown: 'ff', title: 'title'});
should.exist(rendered);
rendered.should.equal('/');
rendered.string.should.equal('/');
});
it('should return a relative url if passed through a nav context', function () {
rendered = helpers.url.call(
{url: '/foo', label: 'Foo', slug: 'foo', current: true});
should.exist(rendered);
rendered.should.equal('/foo');
rendered.string.should.equal('/foo');
});
it('should return an absolute url if passed through a nav context', function () {
@ -126,7 +126,7 @@ describe('{{url}} helper', function () {
{url: '/bar', label: 'Bar', slug: 'bar', current: true},
{hash: {absolute: 'true'}});
should.exist(rendered);
rendered.should.equal('http://testurl.com/bar');
rendered.string.should.equal('http://testurl.com/bar');
});
it('should return an absolute url with https if context is secure', function () {
@ -134,7 +134,7 @@ describe('{{url}} helper', function () {
{url: '/bar', label: 'Bar', slug: 'bar', current: true, secure: true},
{hash: {absolute: 'true'}});
should.exist(rendered);
rendered.should.equal('https://testurl.com/bar');
rendered.string.should.equal('https://testurl.com/bar');
});
it('external urls should be retained in a nav context', function () {
@ -142,7 +142,7 @@ describe('{{url}} helper', function () {
{url: 'http://casper.website/baz', label: 'Baz', slug: 'baz', current: true},
{hash: {absolute: 'true'}});
should.exist(rendered);
rendered.should.equal('http://casper.website/baz');
rendered.string.should.equal('http://casper.website/baz');
});
it('should handle hosted urls in a nav context', function () {
@ -150,7 +150,7 @@ describe('{{url}} helper', function () {
{url: 'http://testurl.com/qux', label: 'Qux', slug: 'qux', current: true},
{hash: {absolute: 'true'}});
should.exist(rendered);
rendered.should.equal('http://testurl.com/qux');
rendered.string.should.equal('http://testurl.com/qux');
});
it('should handle hosted urls in a nav context with secure', function () {
@ -159,7 +159,7 @@ describe('{{url}} helper', function () {
secure: true},
{hash: {absolute: 'true'}});
should.exist(rendered);
rendered.should.equal('https://testurl.com/qux');
rendered.string.should.equal('https://testurl.com/qux');
});
it('should handle hosted https urls in a nav context with secure', function () {
@ -168,7 +168,7 @@ describe('{{url}} helper', function () {
secure: true},
{hash: {absolute: 'true'}});
should.exist(rendered);
rendered.should.equal('https://testurl.com/qux');
rendered.string.should.equal('https://testurl.com/qux');
});
it('should handle hosted urls with the wrong protocol in a nav context', function () {
@ -176,7 +176,7 @@ describe('{{url}} helper', function () {
{url: 'https://testurl.com/quux', label: 'Quux', slug: 'quux', current: true},
{hash: {absolute: 'true'}});
should.exist(rendered);
rendered.should.equal('http://testurl.com/quux');
rendered.string.should.equal('http://testurl.com/quux');
});
it('should pass through protocol-less URLs regardless of absolute setting', function () {
@ -184,13 +184,13 @@ describe('{{url}} helper', function () {
{url: '//casper.website/baz', label: 'Baz', slug: 'baz', current: true},
{hash: {}});
should.exist(rendered);
rendered.should.equal('//casper.website/baz');
rendered.string.should.equal('//casper.website/baz');
rendered = helpers.url.call(
{url: '//casper.website/baz', label: 'Baz', slug: 'baz', current: true},
{hash: {absolute: 'true'}});
should.exist(rendered);
rendered.should.equal('//casper.website/baz');
rendered.string.should.equal('//casper.website/baz');
});
it('should pass through URLs with alternative schemes regardless of absolute setting', function () {
@ -198,25 +198,25 @@ describe('{{url}} helper', function () {
{url: 'tel:01234567890', label: 'Baz', slug: 'baz', current: true},
{hash: {}});
should.exist(rendered);
rendered.should.equal('tel:01234567890');
rendered.string.should.equal('tel:01234567890');
rendered = helpers.url.call(
{url: 'mailto:example@ghost.org', label: 'Baz', slug: 'baz', current: true},
{hash: {}});
should.exist(rendered);
rendered.should.equal('mailto:example@ghost.org');
rendered.string.should.equal('mailto:example@ghost.org');
rendered = helpers.url.call(
{url: 'tel:01234567890', label: 'Baz', slug: 'baz', current: true},
{hash: {absolute: 'true'}});
should.exist(rendered);
rendered.should.equal('tel:01234567890');
rendered.string.should.equal('tel:01234567890');
rendered = helpers.url.call(
{url: 'mailto:example@ghost.org', label: 'Baz', slug: 'baz', current: true},
{hash: {absolute: 'true'}});
should.exist(rendered);
rendered.should.equal('mailto:example@ghost.org');
rendered.string.should.equal('mailto:example@ghost.org');
});
it('should pass through anchor-only URLs regardless of absolute setting', function () {
@ -224,13 +224,34 @@ describe('{{url}} helper', function () {
{url: '#thatsthegoodstuff', label: 'Baz', slug: 'baz', current: true},
{hash: {}});
should.exist(rendered);
rendered.should.equal('#thatsthegoodstuff');
rendered.string.should.equal('#thatsthegoodstuff');
rendered = helpers.url.call(
{url: '#thatsthegoodstuff', label: 'Baz', slug: 'baz', current: true},
{hash: {absolute: 'true'}});
should.exist(rendered);
rendered.should.equal('#thatsthegoodstuff');
rendered.string.should.equal('#thatsthegoodstuff');
});
it('should not HTML-escape URLs', function () {
rendered = helpers.url.call(
{url: '/foo?foo=bar&baz=qux', label: 'Foo', slug: 'foo', current: true});
should.exist(rendered);
rendered.string.should.equal('/foo?foo=bar&baz=qux');
});
it('should encode URLs', function () {
rendered = helpers.url.call(
{url: '/foo?foo=bar&baz=qux&<script>alert("gotcha")</script>', label: 'Foo', slug: 'foo', current: true});
should.exist(rendered);
rendered.string.should.equal('/foo?foo=bar&baz=qux&%3Cscript%3Ealert(%22gotcha%22)%3C/script%3E');
});
it('should not double-encode URLs', function () {
rendered = helpers.url.call(
{url: '/?foo=space%20bar', label: 'Foo', slug: 'foo', current: true});
should.exist(rendered);
rendered.string.should.equal('/?foo=space%20bar');
});
describe('with subdir', function () {
@ -240,7 +261,7 @@ describe('{{url}} helper', function () {
{url: 'http://casper.website/baz', label: 'Baz', slug: 'baz', current: true},
{hash: {absolute: 'true'}});
should.exist(rendered);
rendered.should.equal('http://casper.website/baz');
rendered.string.should.equal('http://casper.website/baz');
});
it('should handle subdir being set in nav context', function () {
@ -250,7 +271,7 @@ describe('{{url}} helper', function () {
{url: '/xyzzy', label: 'xyzzy', slug: 'xyzzy', current: true},
{hash: {absolute: 'true'}});
should.exist(rendered);
rendered.should.equal('http://testurl.com/blog/xyzzy');
rendered.string.should.equal('http://testurl.com/blog/xyzzy');
});
});
});