Skip to content

Commit

Permalink
urlencode navigation URLs rather than HTML escape (#7836)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@kevinansfield @kirrg001
kevinansfield authored and kirrg001 committed Jan 17, 2017
1 parent 49e99c5 commit 8d88f5b
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 31 deletions.
2 changes: 1 addition & 1 deletion core/server/helpers/navigation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
Expand Down
10 changes: 7 additions & 3 deletions core/server/helpers/url.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 changes: 37 additions & 0 deletions core/test/unit/server_helpers/navigation_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
75 changes: 48 additions & 27 deletions core/test/unit/server_helpers/url_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand All @@ -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 () {
Expand All @@ -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 () {
Expand All @@ -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 () {
Expand All @@ -93,64 +93,64 @@ 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 () {
rendered = helpers.url.call(
{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 () {
rendered = helpers.url.call(
{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 () {
rendered = helpers.url.call(
{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 () {
rendered = helpers.url.call(
{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 () {
Expand All @@ -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 () {
Expand All @@ -168,69 +168,90 @@ 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 () {
rendered = helpers.url.call(
{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 () {
rendered = helpers.url.call(
{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 () {
rendered = helpers.url.call(
{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 () {
rendered = helpers.url.call(
{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 () {
Expand All @@ -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 () {
Expand All @@ -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');
});
});
});

0 comments on commit 8d88f5b

Please sign in to comment.