3 ms·
> It then removes any HTML entities that aren't allowed by the sanitizer configuration, and further removes any XSS-unsafe elements or attributes — whether or n
by ishouldbework 11mo ago
> It then removes any HTML entities that aren't allowed by the sanitizer configuration, and further removes any XSS-unsafe elements or attributes — whether or not they are allowed by the sanitizer configuration.
Emphasis mine. I do not understand this design choice. If I explicitly allow `script` tag, why should it be stripped?
If the method was called setXSSSafeSubsetOfHTML sure I guess, but feels weird for setHTML to have impossible-to-override filter.
- evilpie 11mo agoIf you want to use an XSS-unsafe Sanitizer you have to use setHTMLUnsafe.
- jmull 11mo agoI guess they are going for a safe default... the idea is people who don't carefully read the docs or carefully monitor the provenance of their dynamically generated HTML will probably reach for "setHTML()". Meanwhile, there's "setHTMLUnsafe()" and, of course, good old .innerHTML.
- strbean 11mo agoThis is primarily an ergonomic addition, so it kinda makes sense to me to not make the dangerous footguns more ergonomic in the process. You can still assign `innerHTML` etc. to do the dangerous thing.
- hsbauauvhabzb 11mo agoIdeally this should be called dangerouslySetInnerHTML but hindsight blah blah
- meowface 11mo agoI agree, though I also agree with the parent that the method name is a little bit confusing. "safeSetHTML" or "setUntrustedHTML" or something would be clearer.
- strbean 11mo agoIdk about that, there's a good argument that the most obvious methods should be the safe ones. That's what juniors will probably jump to first. If you need the unsafe ones, you'll probably be able to figure that out and find them quickly.
- jfengel 11mo agoI like React's dangerouslySetInnerHTML. The name so clearly conveys "you can do this but you really, really, really shouldn't".
- domenicd 11mo agoIndeed, the web platform now has setHTML() and setHTMLUnsafe() to replace the innerHTML setter. There's also getHTML() (which has extra capabilities over the innerHTML getter).
- meowface 11mo agoOkay, I've changed my mind and agree this is better, then. I wasn't aware they were adding two new methods. That is the safest way to do it.
- SoftTalker 11mo agoWhy not name it what it does: sanitizeAndSetHTML
- xp84 11mo agoNaming things in that manner hasn’t proven to be a good idea over the years. When you have 2 of something and one is safe/better and the other one is known to be problematic, you give the awkward name to the problematic one and the obvious name to the safe/better one. Noobs oughtn’t to be attempting the other one, and anyone who is mature enough to have reason to do it, are mature enough to appreciate the reason behind that complexity.
- pwdisswordfishy 11mo ago
- deleted 11mo ago[deleted]
- wewtyflakes 11mo agoWouldn't that open the floodgates by allowing code that could itself call `setHTML` again but then further revise the args to escalate its privileges?
- systoll 11mo agoA script tag would be able to call setHTMLUnsafe, bypassing whatever sanitation you configured. I’d’ve made it a runtime error to call setHTML with an unsafe config, but Javascript tends toward implicit reinterpretation rather than erroring-out.
- recursivecaveat 11mo agoYou have to make the safe version the ergonomic one. Many many C++ memory bugs are a result of the standards committee making the undefined behaviour version of an operation even 3 characters shorter than the safe one. (They're still doing it too! I found another example added in C++23 recently)
- masklinn 11mo ago> feels weird for setHTML to have impossible-to-override filter. It really doesn’t. We’ve decades of experience telling us that safe behaviour is critical. > I do not understand this design choice. If I explicitly allow `script` tag, why should it be stripped? Because there’s an infinitesimal number of situations where it’s not broken, and that means you should have to put in work to get there. `innerHTML` still exists, and `setHTMLUnsafe` has no filtering whatsoever by default (not even the script deactivation innerHTML performs).
- ishouldbework 11mo agoI did not notice setHTMLUnsafe exists. That makes it (in my, unimportant, opinion) fine.