Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

support target and popup attributes in pseudo-html text links #1726

Merged
merged 1 commit into from May 25, 2017
Merged

support target and popup attributes in pseudo-html text links #1726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
72 changes: 61 additions & 11 deletions src/lib/svg_text_utils.js
View file
Open in desktop
Expand Up @@ -282,11 +282,36 @@ var SPLIT_TAGS = /(<[^<>]*>)/;

var ONE_TAG = /<(\/?)([^ >]*)(\s+(.*))?>/i;

// Style and href: pull them out of either single or double quotes.
// Because we hack in other attributes with style (sub & sup), drop any trailing
// semicolon in user-supplied styles so we can consistently append the tag-dependent style
/*
* style and href: pull them out of either single or double quotes. Also
* - target: (_blank|_self|_parent|_top|framename)
* note that you can't use target to get a popup but if you use popup,
* a `framename` will be passed along as the name of the popup window.
* per the spec, cannot contain whitespace.
* for backward compatibility we default to '_blank'
* - popup: a custom one for us to enable popup (new window) links. String
* for window.open -> strWindowFeatures, like 'menubar=yes,width=500,height=550'
* note that at least in Chrome, you need to give at least one property
* in this string or the page will open in a new tab anyway. We follow this
* convention and will not make a popup if this string is empty.
* per the spec, cannot contain whitespace.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems to be a point of browser inconsistency whether it still works with whitespace... alternatively I guess we could accept whitespace in the input but strip it from the output, but this was easier.

*
* Because we hack in other attributes with style (sub & sup), drop any trailing
* semicolon in user-supplied styles so we can consistently append the tag-dependent style
*/
var STYLEMATCH = /(^|[\s"'])style\s*=\s*("([^"]*);?"|'([^']*);?')/i;
var HREFMATCH = /(^|[\s"'])href\s*=\s*("([^"]*)"|'([^']*)')/i;
var TARGETMATCH = /(^|[\s"'])target\s*=\s*("([^"\s]*)"|'([^'\s]*)')/i;
var POPUPMATCH = /(^|[\s"'])popup\s*=\s*("([^"\s]*)"|'([^'\s]*)')/i;

// dedicated matcher for these quoted regexes, that can return their results
// in two different places
function getQuotedMatch(_str, re) {
if(!_str) return null;
var match = _str.match(re);
return match && (match[3] || match[4]);
}


var COLORMATCH = /(^|;)\s*color:/;

Expand All @@ -297,14 +322,14 @@ exports.plainText = function(_str) {
};

function replaceFromMapObject(_str, list) {
var out = _str || '';
if(!_str) return '';
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

馃悗 no need to loop over all these replacements on an empty string


for(var i = 0; i < list.length; i++) {
var item = list[i];
out = out.replace(item.regExp, item.sub);
_str = _str.replace(item.regExp, item.sub);
}

return out;
return _str;
}

function convertEntities(_str) {
Expand Down Expand Up @@ -354,8 +379,7 @@ function convertToSVG(_str) {

// anchor is the only tag that doesn't turn into a tspan
if(tag === 'a') {
var hrefMatch = extra && extra.match(HREFMATCH);
var href = hrefMatch && (hrefMatch[3] || hrefMatch[4]);
var href = getQuotedMatch(extra, HREFMATCH);

out = '<a';

Expand All @@ -364,7 +388,34 @@ function convertToSVG(_str) {
var dummyAnchor = document.createElement('a');
dummyAnchor.href = href;
if(PROTOCOLS.indexOf(dummyAnchor.protocol) !== -1) {
out += ' xlink:show="new" xlink:href="' + encodeForHTML(href) + '"';
href = encodeForHTML(href);

// look for target and popup specs
var target = encodeForHTML(getQuotedMatch(extra, TARGETMATCH)) || '_blank';
var popup = encodeForHTML(getQuotedMatch(extra, POPUPMATCH));

/*
* xlink:show is for backward compatibility only,
* newer browsers allow target just like html links.
*/
var xlinkShow = (target === '_blank' || target.charAt(0) !== '_') ?
'new' : 'replace';

out += ' xlink:show="' + xlinkShow + '" target="' + target +
'" xlink:href="' + href + '"';

if(popup) {
/*
* Add the window.open call to create a popup
* Even when popup is specified, we leave the original
* href and target in place in case javascript is
* unavailable in this context (like downloaded svg)
* and for accessibility and so users can see where
* the link will lead.
*/
out += ' onclick="window.open(\'' + href + '\',\'' + target +
'\',\'' + popup + '\');return false;"';
}
}
}
}
Expand All @@ -380,8 +431,7 @@ function convertToSVG(_str) {
// now add style, from both the tag name and any extra css
// Most of the svg css that users will care about is just like html,
// but font color is different (uses fill). Let our users ignore this.
var cssMatch = extra && extra.match(STYLEMATCH);
var css = cssMatch && (cssMatch[3] || cssMatch[4]);
var css = getQuotedMatch(extra, STYLEMATCH);
if(css) {
css = encodeForHTML(css.replace(COLORMATCH, '$1 fill:'));
if(tagStyle) css += ';' + tagStyle;
Expand Down
63 changes: 47 additions & 16 deletions test/jasmine/tests/svg_text_utils_test.js
View file
Open in desktop
Expand Up @@ -18,38 +18,49 @@ describe('svg+text utils', function() {
.attr('transform', 'translate(50,50)');
}

function assertAnchorLink(node, href) {
function assertAnchorLink(node, href, target, show, msg) {
var a = node.select('a');

expect(a.attr('xlink:href')).toBe(href);
expect(a.attr('xlink:show')).toBe(href === null ? null : 'new');
if(target === undefined) target = href === null ? null : '_blank';
if(show === undefined) show = href === null ? null : 'new';

expect(a.attr('xlink:href')).toBe(href, msg);
expect(a.attr('target')).toBe(target, msg);
expect(a.attr('xlink:show')).toBe(show, msg);
}

function assertTspanStyle(node, style) {
function assertTspanStyle(node, style, msg) {
var tspan = node.select('tspan');
expect(tspan.attr('style')).toBe(style);
expect(tspan.attr('style')).toBe(style, msg);
}

function assertAnchorAttrs(node, style) {
function assertAnchorAttrs(node, expectedAttrs, msg) {
var a = node.select('a');

var WHITE_LIST = ['xlink:href', 'xlink:show', 'style'],
if(!expectedAttrs) expectedAttrs = {};

var WHITE_LIST = ['xlink:href', 'xlink:show', 'style', 'target', 'onclick'],
attrs = listAttributes(a.node());

// check that no other attribute are found in anchor,
// which can be lead to XSS attacks.

var hasWrongAttr = attrs.some(function(attr) {
return WHITE_LIST.indexOf(attr) === -1;
var wrongAttrs = [];
attrs.forEach(function(attr) {
if(WHITE_LIST.indexOf(attr) === -1) wrongAttrs.push(attr);
});

expect(hasWrongAttr).toBe(false);
expect(wrongAttrs).toEqual([], msg);

var style = expectedAttrs.style || '';
var fullStyle = style || '';
if(style) fullStyle += ';';
fullStyle += 'cursor:pointer';

expect(a.attr('style')).toBe(fullStyle);
expect(a.attr('style')).toBe(fullStyle, msg);

expect(a.attr('onclick')).toBe(expectedAttrs.onclick || null, msg);

}

function listAttributes(node) {
Expand Down Expand Up @@ -137,7 +148,7 @@ describe('svg+text utils', function() {
var node = mockTextSVGElement(textCase);

expect(node.text()).toEqual('Subtitle');
assertAnchorAttrs(node, 'font-size:300px');
assertAnchorAttrs(node, {style: 'font-size:300px'});
assertAnchorLink(node, 'XSS');
});
});
Expand All @@ -157,11 +168,31 @@ describe('svg+text utils', function() {
var node = mockTextSVGElement(textCase);

expect(node.text()).toEqual('z');
assertAnchorAttrs(node, 'y');
assertAnchorAttrs(node, {style: 'y'});
assertAnchorLink(node, 'x');
});
});

it('accepts `target` with links and tries to translate it to `xlink:show`', function() {
var specs = [
{target: '_blank', show: 'new'},
{target: '_self', show: 'replace'},
{target: '_parent', show: 'replace'},
{target: '_top', show: 'replace'},
{target: 'some_frame_name', show: 'new'}
];
specs.forEach(function(spec) {
var node = mockTextSVGElement('<a href="x" target="' + spec.target + '">link</a>');
assertAnchorLink(node, 'x', spec.target, spec.show, spec.target);
});
});

it('attaches onclick if popup is specified', function() {
var node = mockTextSVGElement('<a href="x" target="fred" popup="width=500,height=400">link</a>');
assertAnchorLink(node, 'x', 'fred', 'new');
assertAnchorAttrs(node, {onclick: 'window.open(\'x\',\'fred\',\'width=500,height=400\');return false;'});
});

it('keeps query parameters in href', function() {
var textCases = [
'<a href="https://abc.com/myFeature.jsp?name=abc&pwd=def">abc.com?shared-key</a>',
Expand All @@ -171,9 +202,9 @@ describe('svg+text utils', function() {
textCases.forEach(function(textCase) {
var node = mockTextSVGElement(textCase);

assertAnchorAttrs(node);
expect(node.text()).toEqual('abc.com?shared-key');
assertAnchorLink(node, 'https://abc.com/myFeature.jsp?name=abc&pwd=def');
assertAnchorAttrs(node, {}, textCase);
expect(node.text()).toEqual('abc.com?shared-key', textCase);
assertAnchorLink(node, 'https://abc.com/myFeature.jsp?name=abc&pwd=def', undefined, undefined, textCase);
});
});

Expand Down