Strip out tags in title displays

[nicolaskruchten]

We display trace names as Fold titles and axis names in dropdowns, and we should strip out tags when we do, perhaps using https://www.npmjs.com/package/striptags

[nicolaskruchten]

@VeraZab what do you think of https://www.npmjs.com/package/striptags ? Do we have a different library we use for this sort of thing? Something built in to plotly.js?

[VeraZab]

Striptags looks good, let's use that if plotly.js doesn't have anything built in. (cc @alexcjohnson we don't have anything in plotly.js doing this correct?)

[alexcjohnson]

We do have svgTextUtils.plainText - which has the distinction of only stripping the tags that plotly.js actually supports. I think that means the results will be more consistent with what you see on the actual plot, in case someone uses an unsupported tag.

[VeraZab]

oh nice! Then I think it'd be a good idea @nicolaskruchten to use svgTextUtils. All the places we allow the user to use html tags is because it's an input to plotly.js afaik..

[nicolaskruchten]

Hmmm, this seems less safe than the NPM module which isn't regex-based...

[alexcjohnson]

Hmmm, this seems less safe than the NPM module which isn't regex-based...

Does that mean there's a bug in plotly.js? That we should fix (possibly by using the npm module) and then use it here too?

[nicolaskruchten]

I don't know if there's a bug in plotly.js, just that regex-based tag sanitizers are sometimes considered less safe than simpler state-machine-based ones... See this for example: https://stackoverflow.com/questions/24723/best-regex-to-catch-xss-cross-site-scripting-attack-in-java/535022#535022

[nicolaskruchten]

For example:

plainText("<<a>script>alert('ciao');<</a>/script>")
returns
'< script>alert(\'ciao\');< /script>'

[alexcjohnson]

Fair enough - though I'm not concerned about XSS as far as plainText is concerned, we don't use its output as HTML but as text... so the would-be attacker will see some ugly "text" just as they will within plotly.js, but won't hurt anyone.

[nicolaskruchten]

Ahhh right ok, I was stuck in an XSS frame here because of what I was doing this morning, but that's not what we're talking about, either in plotly.js or in the editor: we do in fact want to safely render text just the way plotly.js would, so there's "no risk" of XSS here anyway because we wouldn't be using dangerouslySetInnerHTML. OK, I'll use this then. Shall I load it directly from that file or is it available on the Plotly object somewhere?

[alexcjohnson]

I don't think it's exposed on the Plotly object, no.

[nicolaskruchten]

I opted for striptags in the end because plainText seems to insert spaces at the tag boundaries... NBD either way :)