search

JiLiZART / BBob

⚡️Blazing fast js bbcode parser, that transforms and parses bbcode to AST and transform it to HTML, React, Vue with plugin support in pure javascript, no dependencies
https://codepen.io/JiLiZART/full/vzMvpd
MIT License
166 stars 19 forks source link

Misleading name/behavior for `escapeHTML`: Should denote "use for attribute values" #203

Open mmichaelis opened 1 year ago

mmichaelis commented 1 year ago

https://github.com/JiLiZART/BBob/blob/3575982b280cc45c9cedaf7a059491a324c1b514/packages/bbob-plugin-helper/src/helpers.js#L28-L39

The name escapeHTML suggests, that the method may be used to sanitize text-content and get rid of probably malicious nested HTML in BBCode, like [i]<script>javascript:alert("XSS!"</script>[/i]. Unfortunately, the method has an extra turn, to support escaping of probably unsafe href attributes: It also escapes problematic protocols assuming, we are in a URL-context.

Thus, naively reused in custom API the above will escape the text content to:

&lt;script&gt;javascript%3Aalert... (etc.)

The suggestion for clarity is to name the method escapeHTMLAttribute or, as this is considered breaking, at least mention this usage in the JSdoc.

Otherwise, I think the best option for escaping (and I tend to switch to it) is to rely on DOM processing as suggested in https://github.com/JiLiZART/BBob/issues/148#issuecomment-1287970048.

JiLiZART commented 1 year ago

DOM processing is not possible because this library is isomorphic. But you can escape html attributes in your own plugin using DOM API. I have ideas to extract this function to separate folder with browser.js and node.js version (using platform API like DOM or node js builtin functions)