Closed wasamasa closed 3 years ago
Thanks for bringing this up, @wasamasa! this is something I've been meaning to work on for sure and your request prompt me to finally get to it. I have a preliminary version that I plan to merge if my solution isn't too cumbersome for the main use cases: https://github.com/clarete/templatel/pull/5
I just need to do some more testing to make sure this change is compatible with the static HTML generator that depends on this library and also add some documentation.
Please let me know if you think there should be a different solution for the issue 🙇🏾
This looks good. I haven't tested it yet though. Some more thoughts:
* Autoescaping works on a best effort basis. If someone were to interpolate user input into an event handler attribute, no amount of escaping would help. I assume Jinja is the same in this aspect.
Yeah, I believe so. I am wondering if it's worth covering this behavior with tests or if it'd be enough to document it with together with the auto escaping feature.
* The tests look very simple. One problem with manual use of escaping is that there's the danger of double escaping if you have nested blocks of markup. I'd make sure to verify that this usecase works as expected.
oh, you saw that one coming! because of this comment I did find a little glitch with inheriting blocks that I tried to fix with this commit. I wonder if there's any other weird scenario out there for this type of case.
* Jinja has a feature to automatically enable autoescaping based on the file extension of a file. I'm not sure whether this library is intended to be used outside of HTML/XML templates, so not sure how much sense this feature would make.
I think I'd want to follow jinja's steps here: be very good option for templating HTML/XML but also a good enough option for other text formats. With that in mind, maybe we could go with the following APIs:
For strings:
;; auto escaping is initially disabled but can be enabled with an optional flag
(templatel-render-string template variables &optional autoescape)
For files:
;; autoescape-extensions defaults to '("html" "xml") and enables auto escaping if the file extension
;; matches any element within that list
(templatel-render-file path variables &optional autoescape-extensions)
What do you think?
I'm not sure an optional flag is good, especially with inverted meaning. Other options:
Oh I already had something that resembles keyword arguments. I pushed a change to allow doing this instead:
(templatel-render-string "<p>{{ name }}</p>" '(("name" . "<b>Someone</b>")) :autoescape t)
I merged #5 and added a bit of documentation here: https://clarete.li/templatel/api.html please let reopen the issue or create a new one if you find anything else!
Thank you for all the help 🙇🏾
I've done a quick XSS test:
I see this less of a security issue as the only user of this package seems to be a blog (where the hostile user input comes from the author themselves) and more of a UX one (it's very bothersome to remember escaping user input so that all special characters are rendered correctly).
It seems that Jinja does this automatically for HTML/XML: https://jinja2docs.readthedocs.io/en/stable/api.html#autoescaping. They use this library for it: https://github.com/pallets/markupsafe.