From 6a66569f767a7f00e60a460001d6b59e66f9a4b5 Mon Sep 17 00:00:00 2001 From: Jody McIntyre Date: Mon, 1 Aug 2016 16:48:55 -0400 Subject: [PATCH 1/3] Remove html entity decoding from convertToSVG This is unnecessary and imposes a performance hit on long strings. --- src/lib/svg_text_utils.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/lib/svg_text_utils.js b/src/lib/svg_text_utils.js index fe995cc74f7..fa0382eb54b 100644 --- a/src/lib/svg_text_utils.js +++ b/src/lib/svg_text_utils.js @@ -252,8 +252,7 @@ function encodeForHTML(_str) { } function convertToSVG(_str) { - var htmlEntitiesDecoded = Plotly.util.html_entity_decode(_str); - var result = htmlEntitiesDecoded + var result = _str .split(/(<[^<>]*>)/).map(function(d) { var match = d.match(/<(\/?)([^ >]*)\s*(.*)>/i), tag = match && match[2].toLowerCase(), From 6cf80dec6b318a0522726351c9eadb9da1f6b637 Mon Sep 17 00:00:00 2001 From: Jody McIntyre Date: Tue, 2 Aug 2016 11:47:57 -0400 Subject: [PATCH 2/3] Update code & mocks to not depend on entity decode --- src/plots/cartesian/axes.js | 5 +++-- src/plots/cartesian/graph_interact.js | 4 ++-- test/image/mocks/axes_enumerated_ticks.json | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 4db4ca2c642..b1b33dd48ee 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -1053,7 +1053,8 @@ function formatLinear(ax, out, hover, extraPrecision, hideexp) { // new, more reliable procedure than d3.round or similar: // add half the rounding increment, then stringify and truncate // also automatically switch to sci. notation -var SIPREFIXES = ['f', 'p', 'n', 'μ', 'm', '', 'k', 'M', 'G', 'T']; +var SIPREFIXES = ['f', 'p', 'n', 'μ', 'm', '', 'k', 'M', 'G', 'T']; + function numFormat(v, ax, fmtoverride, hover) { // negative? var isNeg = v < 0, @@ -1144,7 +1145,7 @@ function numFormat(v, ax, fmtoverride, hover) { v += 'E' + signedExponent; } else if(exponentFormat === 'power') { - v += '×10' + signedExponent + ''; + v += '×10' + signedExponent + ''; } else if(exponentFormat === 'B' && exponent === 9) { v += 'B'; diff --git a/src/plots/cartesian/graph_interact.js b/src/plots/cartesian/graph_interact.js index a016b8a5cc5..540577d922c 100644 --- a/src/plots/cartesian/graph_interact.js +++ b/src/plots/cartesian/graph_interact.js @@ -695,7 +695,7 @@ function cleanPoint(d, hovermode) { d.xLabel += ' +' + xeText + ' / -' + Axes.tickText(d.xa, d.xa.c2l(d.xerrneg), 'hover').text; } - else d.xLabel += ' ± ' + xeText; + else d.xLabel += ' ± ' + xeText; // small distance penalty for error bars, so that if there are // traces with errors and some without, the error bar label will @@ -708,7 +708,7 @@ function cleanPoint(d, hovermode) { d.yLabel += ' +' + yeText + ' / -' + Axes.tickText(d.ya, d.ya.c2l(d.yerrneg), 'hover').text; } - else d.yLabel += ' ± ' + yeText; + else d.yLabel += ' ± ' + yeText; if(hovermode === 'y') d.distance += 1; } diff --git a/test/image/mocks/axes_enumerated_ticks.json b/test/image/mocks/axes_enumerated_ticks.json index b9cc9f48c1b..cbe53ce1e10 100644 --- a/test/image/mocks/axes_enumerated_ticks.json +++ b/test/image/mocks/axes_enumerated_ticks.json @@ -33,7 +33,7 @@ "xaxis": { "ticktext": [ "green eggs", - "& ham", + "& ham", "H2O", "Gorgonzola" ], @@ -47,4 +47,4 @@ ] } } -} \ No newline at end of file +} From 7649e5d5d294279d844f2c9d36019bb0252fa3bf Mon Sep 17 00:00:00 2001 From: Jody McIntyre Date: Tue, 2 Aug 2016 12:02:14 -0400 Subject: [PATCH 3/3] Update tests to not depend on entity decode --- test/jasmine/tests/svg_text_utils_test.js | 43 ++++++++++++----------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/test/jasmine/tests/svg_text_utils_test.js b/test/jasmine/tests/svg_text_utils_test.js index be4601743c8..7daa24f4bfb 100644 --- a/test/jasmine/tests/svg_text_utils_test.js +++ b/test/jasmine/tests/svg_text_utils_test.js @@ -121,33 +121,34 @@ describe('svg+text utils', function() { }); it('wrap XSS attacks in href', function() { - var textCases = [ - 'Subtitle', - 'Subtitle' - ]; + var node = mockTextSVGElement( + 'Subtitle' + ); - textCases.forEach(function(textCase) { - var node = mockTextSVGElement(textCase); + expect(node.text()).toEqual('Subtitle'); + assertAnchorAttrs(node); + assertAnchorLink(node, 'XSS onmouseover=alert(1) style=font-size:300px'); + }); - expect(node.text()).toEqual('Subtitle'); - assertAnchorAttrs(node); - assertAnchorLink(node, 'XSS onmouseover=alert(1) style=font-size:300px'); - }); + it('wrap XSS attacks with quoted entities in href', function() { + var node = mockTextSVGElement( + 'Subtitle' + ); + + console.log(node.select('a').attr('xlink:href')); + expect(node.text()).toEqual('Subtitle'); + assertAnchorAttrs(node); + assertAnchorLink(node, 'XSS" onmouseover="alert(1)" style="font-size:300px'); }); it('should keep query parameters in href', function() { - var textCases = [ - 'abc.com?shared-key', - 'abc.com?shared-key' - ]; - - textCases.forEach(function(textCase) { - var node = mockTextSVGElement(textCase); + var node = mockTextSVGElement( + 'abc.com?shared-key' + ); - assertAnchorAttrs(node); - expect(node.text()).toEqual('abc.com?shared-key'); - assertAnchorLink(node, 'https://abc.com/myFeature.jsp?name=abc&pwd=def'); - }); + assertAnchorAttrs(node); + expect(node.text()).toEqual('abc.com?shared-key'); + assertAnchorLink(node, 'https://abc.com/myFeature.jsp?name=abc&pwd=def'); }); it('allow basic spans', function() {