From 91f668c03e5b474152842565f54ced1b577d4340 Mon Sep 17 00:00:00 2001 From: Martin Lui Date: Sun, 27 Oct 2024 04:22:14 -0700 Subject: [PATCH 1/3] Apply styles to SVG text elements allowed by strict CSPs Strict Content Security Policies (those without 'unsafe-inline' keyword) does not permit inline styles (setting the 'style' attribute in code). However, setting individual style properties on an element object is allowed. Therefore, fix the "svg_text_utils.js" by changing the code that retrieves, manipulates and applies the style attribute strings of the "pseudo-HTML" configuration to instead parse and/or apply styles directly on the element. In other words, instead of using `d3.select(node).attr("style", "some string value")`, use `d3.select(node).style(name, value)` as shown in the D3JS docs: https://d3js.org/d3-selection/selecting#select With this method, in addition to it being allowed by string CSPs, the D3 JS library and/or the browser seems to do some level of input validation and normalization. As such, unit test cases were updated to account for this differences, which includes: - Order and format of the attributes were changed. For example, there will be a space after the colon of the CSS style when read back from the browser. - Invalid style attributes would not be applied. Thus, fixed test cases with actual valid styles. - Setting the "color" style attribute in SVG text actually normalizes to setting the "fill" color attribute. - Using "Times New Roman" font will cause "make-baseline" test to fail due to "error 525: plotly.js error" when run by the Kaleido Python library. Root cause of that is probably too deep to get into and removing it does not change the substance of that test case (using "Times, serif" achieves the same result). --- src/lib/svg_text_utils.js | 83 ++++++++++++++---- test/image/mocks/pseudo_html.json | 2 +- test/jasmine/tests/hover_label_test.js | 6 +- test/jasmine/tests/icicle_test.js | 2 +- test/jasmine/tests/sunburst_test.js | 2 +- test/jasmine/tests/svg_text_utils_test.js | 100 +++++++++++++--------- test/jasmine/tests/treemap_test.js | 2 +- 7 files changed, 130 insertions(+), 67 deletions(-) diff --git a/src/lib/svg_text_utils.js b/src/lib/svg_text_utils.js index 2c5ddc7f1d9..ff7396e5dd1 100644 --- a/src/lib/svg_text_utils.js +++ b/src/lib/svg_text_utils.js @@ -328,15 +328,15 @@ var TAG_STYLES = { // would like to use baseline-shift for sub/sup but FF doesn't support it // so we need to use dy along with the uber hacky shift-back-to // baseline below - sup: 'font-size:70%', - sub: 'font-size:70%', - s: 'text-decoration:line-through', - u: 'text-decoration:underline', - b: 'font-weight:bold', - i: 'font-style:italic', - a: 'cursor:pointer', - span: '', - em: 'font-style:italic;font-weight:bold' + sup: {'font-size':'70%'}, + sub: {'font-size':'70%'}, + s: {'text-decoration':'line-through'}, + u: {'text-decoration':'underline'}, + b: {'font-weight': 'bold'}, + i: {'font-style':'italic'}, + a: {'cursor':'pointer'}, + span: {}, + em: {'font-style':'italic','font-weight':'bold'} }; // baseline shifts for sub and sup @@ -383,9 +383,6 @@ exports.BR_TAG_ALL = //gi; * convention and will not make a popup if this string is empty. * per the spec, cannot contain whitespace. * - * 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 - * * These are for tag attributes; Chrome anyway will convert entities in * attribute values, but not in attribute names * you can test this by for example: @@ -394,7 +391,7 @@ exports.BR_TAG_ALL = //gi; * > p.innerHTML * <- 'Hi' */ -var STYLEMATCH = /(^|[\s"'])style\s*=\s*("([^"]*);?"|'([^']*);?')/i; +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*("([\w=,]*)"|'([\w=,]*)')/i; @@ -495,7 +492,8 @@ var entityToUnicode = { nbsp: ' ', times: '×', plusmn: '±', - deg: '°' + deg: '°', + quot: "'", }; // NOTE: in general entities can contain uppercase too (so [a-zA-Z]) but all the @@ -537,6 +535,50 @@ function fromCodePoint(code) { ); } +var SPLIT_STYLES = /([^;]+;|$)|&(#\d+|#x[\da-fA-F]+|[a-z]+);/; + +var ONE_STYLE = /^\s*([^:]+)\s*:\s*(.+?)\s*;?$/i; + +function applyStyles(node, styles) { + var parts = styles.split(SPLIT_STYLES); + var filteredParts = []; + for(var i = 0; i < parts.length; i++) { + if(parts[i] && typeof parts[i] === "string" && parts[i].length > 0) { + filteredParts.push(parts[i]); + } + } + parts = filteredParts; + + for(var i = 0; i < parts.length; i++) { + var parti = parts[i]; + + // Recombine parts that was split due to HTML entity's semicolon + var partToSearch = parti; + do { + var matchEntity = partToSearch.match(ENTITY_MATCH); + if(matchEntity) { + var entity = matchEntity[0]; + partToSearch = parts[i+1]; + if(parti.endsWith(entity) && partToSearch) { + // Matched HTML entity is at the end, and thus, need to + // combine with next part to complete the style (when it ends + // with a semicolon that is not part of a HTML entity) + parti += partToSearch; + i++; + } + } else { + partToSearch = undefined; + } + } while (partToSearch); + + var match = parti.match(ONE_STYLE); + if(match) { + var decodedStyle = convertEntities(match[2]); + d3.select(node).style(match[1], decodedStyle); + } + } +} + /* * buildSVGText: convert our pseudo-html into SVG tspan elements, and attach these * to containerNode @@ -613,8 +655,6 @@ function buildSVGText(containerNode, str) { } } else nodeType = 'tspan'; - if(nodeSpec.style) nodeAttrs.style = nodeSpec.style; - var newNode = document.createElementNS(xmlnsNamespaces.svg, nodeType); if(type === 'sup' || type === 'sub') { @@ -633,6 +673,10 @@ function buildSVGText(containerNode, str) { } d3.select(newNode).attr(nodeAttrs); + if(nodeSpec.style) applyStyles(newNode, nodeSpec.style) + if(nodeSpec.tagStyle) { + d3.select(newNode).style(nodeSpec.tagStyle); + } currentNode = nodeSpec.node = newNode; nodeStack.push(nodeSpec); @@ -693,10 +737,10 @@ function buildSVGText(containerNode, str) { var css = getQuotedMatch(extra, STYLEMATCH); if(css) { css = css.replace(COLORMATCH, '$1 fill:'); - if(tagStyle) css += ';' + tagStyle; - } else if(tagStyle) css = tagStyle; + } if(css) nodeSpec.style = css; + if(tagStyle) nodeSpec.tagStyle = tagStyle; if(tagType === 'a') { hasLink = true; @@ -770,7 +814,7 @@ exports.sanitizeHTML = function sanitizeHTML(str) { var extra = match[4]; var css = getQuotedMatch(extra, STYLEMATCH); - var nodeAttrs = css ? {style: css} : {}; + var nodeAttrs = {}; if(tagType === 'a') { var href = getQuotedMatch(extra, HREFMATCH); @@ -790,6 +834,7 @@ exports.sanitizeHTML = function sanitizeHTML(str) { var newNode = document.createElement(tagType); currentNode.appendChild(newNode); d3.select(newNode).attr(nodeAttrs); + if(css) applyStyles(newNode, css); currentNode = newNode; nodeStack.push(newNode); diff --git a/test/image/mocks/pseudo_html.json b/test/image/mocks/pseudo_html.json index b2d9deb8d11..1762be0c712 100644 --- a/test/image/mocks/pseudo_html.json +++ b/test/image/mocks/pseudo_html.json @@ -3,7 +3,7 @@ { "x": ["b i", "line 1
line 2"], "y": ["sub1", "sup2"], - "name": "test pseudoHTML
3H2O is heavy!
and Fonts,
oh my?" + "name": "test pseudoHTML
3H2O is heavy!
and Fonts,
oh my?" } ], "layout": { diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index a9af59100c0..9f77b97086f 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -748,7 +748,7 @@ describe('hover info', function() { it('provides exponents correctly for z data', function(done) { function expFmt(val, exp) { - return val + '×10\u200b' + + return val + '×10\u200b' + (exp < 0 ? MINUS_SIGN + -exp : exp) + '\u200b'; } @@ -2071,7 +2071,7 @@ describe('hover info', function() { expect(hoverTrace.y).toEqual(1); assertHoverLabelContent({ - nums: '$1.00\nPV learning curve.txt', + nums: '$1.00\nPV learning curve.txt', name: '', axis: '0.388' }); @@ -2120,7 +2120,7 @@ describe('hover info', function() { expect(hoverTrace.y).toEqual(1); assertHoverLabelContent({ - nums: 'Cost ($/W​P​):$1.00\nCumulative Production (GW):0.3880', + nums: 'Cost ($/W​P​):$1.00\nCumulative Production (GW):0.3880', name: '', axis: '0.388' }); diff --git a/test/jasmine/tests/icicle_test.js b/test/jasmine/tests/icicle_test.js index 7032478731c..b7efca690f5 100644 --- a/test/jasmine/tests/icicle_test.js +++ b/test/jasmine/tests/icicle_test.js @@ -635,7 +635,7 @@ describe('Test icicle hover:', function() { exp: { label: { nums: 'Abel :: 6.00', - name: 'N.B.' + name: 'N.B.' }, ptData: { curveNumber: 0, diff --git a/test/jasmine/tests/sunburst_test.js b/test/jasmine/tests/sunburst_test.js index 719d9f61bae..d5bb80cb175 100644 --- a/test/jasmine/tests/sunburst_test.js +++ b/test/jasmine/tests/sunburst_test.js @@ -648,7 +648,7 @@ describe('Test sunburst hover:', function() { exp: { label: { nums: 'Abel :: 6.00', - name: 'N.B.' + name: 'N.B.' }, ptData: { curveNumber: 0, diff --git a/test/jasmine/tests/svg_text_utils_test.js b/test/jasmine/tests/svg_text_utils_test.js index 307f8d8c45c..ad88eedf3a2 100644 --- a/test/jasmine/tests/svg_text_utils_test.js +++ b/test/jasmine/tests/svg_text_utils_test.js @@ -64,8 +64,8 @@ describe('svg+text utils', function() { var style = expectedAttrs.style || ''; var fullStyle = style || ''; - if(style) fullStyle += ';'; - fullStyle += 'cursor:pointer'; + if(style) fullStyle += '; '; + fullStyle += 'cursor: pointer;'; expect(a.attr('style')).toBe(fullStyle, msg); @@ -167,27 +167,27 @@ describe('svg+text utils', function() { var node = mockTextSVGElement(textCase); expect(node.text()).toEqual('Subtitle'); - assertAnchorAttrs(node, {style: 'font-size:300px'}); + assertAnchorAttrs(node, {style: 'font-size: 300px'}); assertAnchorLink(node, 'XSS'); }); }); it('accepts href and style in in any order and tosses other stuff', function() { var textCases = [ - 'z', - 'z', - 'z', - 'z', - 'z', - 'z', - 'z', + 'z', + 'z', + 'z', + 'z', + 'z', + 'z', + 'z', ]; textCases.forEach(function(textCase) { var node = mockTextSVGElement(textCase); expect(node.text()).toEqual('z'); - assertAnchorAttrs(node, {style: 'y'}); + assertAnchorAttrs(node, {style: 'font-variant: small-caps'}); assertAnchorLink(node, 'x'); }); }); @@ -288,29 +288,47 @@ describe('svg+text utils', function() { it('allows quoted styles in spans', function() { var node = mockTextSVGElement( - 'text' + 'text' ); expect(node.text()).toEqual('text'); - assertTspanStyle(node, 'quoted: yeah;'); + assertTspanStyle(node, 'fill: green;'); + }); + + it('adjusts quoted styles in spans', function() { + var node = mockTextSVGElement( + 'text' + ); + + expect(node.text()).toEqual('text'); + assertTspanStyle(node, 'fill: green;'); }); it('ignores extra stuff after span styles', function() { var node = mockTextSVGElement( - 'text' + 'text' ); expect(node.text()).toEqual('text'); - assertTspanStyle(node, 'quoted: yeah;'); + assertTspanStyle(node, 'fill: green;'); }); - it('escapes HTML entities in span styles', function() { + it('decodes some HTML entities in span styles', function() { var node = mockTextSVGElement( - 'text' + 'text' ); expect(node.text()).toEqual('text'); - assertTspanStyle(node, 'quoted: yeah&\';;'); + assertTspanStyle(node, "font-family: \"Times New Roman\";"); + }); + + it('ignores invalid HTML entities in span styles', function() { + var node = mockTextSVGElement( + 'text' + ); + + expect(node.text()).toEqual('text'); + assertTspanStyle(node, null); }); it('decodes some HTML entities in text', function() { @@ -367,30 +385,30 @@ describe('svg+text utils', function() { var controlNode = mockTextSVGElement('bold'); expect(controlNode.html()).toBe( - 'bold' + 'bold' ); }); it('supports superscript by itself', function() { var node = mockTextSVGElement('123'); expect(node.html()).toBe( - '\u200b123' + + '\u200b123' + '\u200b'); }); it('supports subscript by itself', function() { var node = mockTextSVGElement('123'); expect(node.html()).toBe( - '\u200b123' + + '\u200b123' + '\u200b'); }); it('supports superscript and subscript together with normal text', function() { var node = mockTextSVGElement('SO42-'); expect(node.html()).toBe( - 'SO\u200b4' + + 'SO\u200b4' + '\u200b\u200b' + - '2-' + + '2-' + '\u200b'); }); @@ -398,22 +416,22 @@ describe('svg+text utils', function() { var node = mockTextSVGElement('be Bold
and
Strong'); expect(node.html()).toBe( 'be ' + - 'Bold' + + 'Bold' + '' + - 'and' + + 'and' + '' + - '' + - 'Strong'); + '' + + 'Strong'); }); it('allows one to span
s', function() { var node = mockTextSVGElement('SO4
44'); expect(node.html()).toBe( 'SO\u200b' + - '4' + + '4' + '\u200b' + '\u200b' + - '44' + + '44' + '\u200b'); }); @@ -428,9 +446,9 @@ describe('svg+text utils', function() { var node = mockTextSVGElement(textCase); function opener(dy) { return '' + - '' + - '' + - '\u200b'; + '' + + '' + + '\u200b'; } var closer = '\u200b' + ''; @@ -589,25 +607,25 @@ describe('sanitizeHTML', function() { textCases.forEach(function(textCase) { var innerHTML = mockHTML(textCase); - expect(innerHTML).toEqual('Subtitle'); + expect(innerHTML).toEqual('Subtitle'); }); }); it('accepts href and style in in any order and tosses other stuff', function() { var textCases = [ - 'z', - 'z', - 'z', - 'z', - 'z', - 'z', - 'z', + 'z', + 'z', + 'z', + 'z', + 'z', + 'z', + 'z', ]; textCases.forEach(function(textCase) { var innerHTML = mockHTML(textCase); - expect(innerHTML).toEqual('z'); + expect(innerHTML).toEqual('z'); }); }); diff --git a/test/jasmine/tests/treemap_test.js b/test/jasmine/tests/treemap_test.js index d70ad287b31..b2cdafe2f33 100644 --- a/test/jasmine/tests/treemap_test.js +++ b/test/jasmine/tests/treemap_test.js @@ -737,7 +737,7 @@ describe('Test treemap hover:', function() { exp: { label: { nums: 'Abel :: 6.00', - name: 'N.B.' + name: 'N.B.' }, ptData: { curveNumber: 0, From d561644b23a543d26a9012fc86c3f7e6cfa27ddf Mon Sep 17 00:00:00 2001 From: Martin Lui Date: Sun, 27 Oct 2024 05:23:43 -0700 Subject: [PATCH 2/3] Add draftlogs for Pull Request #7256 --- draftlogs/7256_fix.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 draftlogs/7256_fix.md diff --git a/draftlogs/7256_fix.md b/draftlogs/7256_fix.md new file mode 100644 index 00000000000..1b235d6f7a1 --- /dev/null +++ b/draftlogs/7256_fix.md @@ -0,0 +1 @@ +Remove inline styles in SVG text elements generated from pseudo-HTML configurations, which break with strict CSP setups [[#7256](https://github.com/plotly/plotly.js/pull/7256)] From 3ded59e3257dd3c5335307d17c41efe3b366dfbe Mon Sep 17 00:00:00 2001 From: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com> Date: Mon, 16 Dec 2024 13:05:43 -0500 Subject: [PATCH 3/3] Update draftlogs/7256_fix.md --- draftlogs/7256_fix.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/draftlogs/7256_fix.md b/draftlogs/7256_fix.md index 1b235d6f7a1..36d1b45a874 100644 --- a/draftlogs/7256_fix.md +++ b/draftlogs/7256_fix.md @@ -1 +1,2 @@ -Remove inline styles in SVG text elements generated from pseudo-HTML configurations, which break with strict CSP setups [[#7256](https://github.com/plotly/plotly.js/pull/7256)] + - Remove inline styles in SVG text elements generated from pseudo-HTML configurations, which break with strict CSP setups [[#7256](https://github.com/plotly/plotly.js/pull/7256)], + with thanks to @martian111 for the contribution!