Repository navigation
fix: escape HTML in the serve build-error overlay - #3293
Conversation
Keats
left a comment
Keats
left a comment
There was a problem hiding this comment.
Just use a tera template so it's handled automatically, no need to rewrite another escape_html
Address review by replacing hand-rolled escape_html with Tera one_off autoescape.
|
I appreciate the pointer, that is cleaner than a hand rolled escape. Switched the overlay to a small Tera template with autoescape on, so the error text is escaped automatically and escape_html is gone. Livereload, the 404 path, and CLI error printing are unchanged. The existing build_error_overlay_escapes_html test still covers the <img and & case from #3288. Let me know if you want the template moved elsewhere. |
|
Done, the overlay test and its helpers are out. The Tera autoescape path is unchanged and still covers the case from #3288. Let me know if you want the template moved elsewhere. |
|
Thanks |
|
Thanks Vincent, switching the overlay to a Tera template was a much cleaner fix than my hand-rolled escape. I appreciate the quick reviews and the merge! |
2 participants
IMPORTANT: Please do not create a Pull Request adding a new feature without discussing it first.
The place to discuss new features is the forum: https://zola.discourse.group/
If you want to add a new feature, please open a thread there first in the feature requests section.
Sanity check:
Code changes
error_injection_middlewareputserror_strstraight into the overlay<pre>, with no escaping.A build error that contains
<imgor&is then parsed by the browser, which is what #3288 shows with{{ <img src=error onerror=alert(1)> }}underzola serve.A successful build already emits the author's HTML, so this is the error screen eating the source, not a new boundary.
I escape
&,<,>, and"on that string before it goes into the markup, and I left the<pre>in place so the line breaks still show.Livereload, the 404 page around the overlay, and CLI error printing are unchanged.
The 404 page reuses the same string, so it gets the escaping too.
build_error_overlay_escapes_htmlchecks that<imgand&come back as<imgand&, and that a plain message stays literal.Fixes #3288.
nextbranch?Yes. I used Grok 4.7 (effort/thinking: xhigh) to draft the escape and the test, then I read the diff, ran the test, and I'm standing by the change.