-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Safer fields (escaping fields in xhtm) #21
Comments
Sorry for delay. A bit loaded these days. Will look at it today. |
@dy No rush whatsoever. You’re not blocking me in any way. Whenever you can is fine :) |
I gave it a thought - I wonder why escaping wouldn't work for you if implemented on attributes level? I don't think merging sanititzer is right move here, a bit mixing concerns and also some unnecessary slowdown? |
The other concern here is that sanitizing doesn't necessarily include all html rules or html at all. Ie. xhtml can be used with some XML without these escaping rules. |
Based on this issue on vhtml, I don’t think the hyperscript → HTML renderer is the right place to escape (untrusted) data interpolated into templates. Since hyperscript is stateless, a renderer like vhtml has no way I can see of carrying out proper escaping of code – what is referred to as sanitisation in vhtml, although it’s not really sanitisation but escaping – without being open to the linked replay/injection attack. (Whether or not that attack has meaningful real-world exploits is another issue but I’d rather not find out.) What we can do, however, is easily carry out escaping of fields in xhtm because we know what is an interpolated string (“field”) and what is not.
In my fork of vhtml, I’ve removed the “sanitisation” code from tag contents. And I’m going to open a pull request next with that functionality added to xhtm instead.
Would love your thoughts, @dy.
Also, while vhtml is aware of attributes and can do attribute escaping properly (I’ve kept that for the time being), it feels wrong to separate that functionality between two modules. If we’re going to escape fields here, we should probably be escaping attributes here too. If this is a direction you think is worth pursuing? Should I add that to that PR? Or open a different one?
(I also completely understand if this is not something you feel belongs in xhtm. I’m not sure of your other use cases. While it makes sense for me, I realise it might not for you when using a stateful DOM-based renderer in the browser, for example. I’m happy to keep a soft fork that does sanitisation and keep it synced with xhtm.) :)
The text was updated successfully, but these errors were encountered: