fix(render-html): Harden HTML embedding boundary and metadata escaping#163
Merged
Conversation
HTML parsers terminate <script> blocks on any case variant of the </script tag, including </SCRIPT>, </Script>, or </script foo>. The previous escape only matched the exact lowercase </script> string, which let metadata containing those variants escape the JSON island and execute as markup. Match the </script prefix case-insensitively and preserve the original casing when emitting the inert backslash sequence, and cover the variants with regression tests.
render_html previously accepted any &str as the SVG payload, so the crate boundary allowed arbitrary <script> blocks, event handlers, or external resource references to be wrapped in the trusted HTML document. Introduce SvgArtifact in relune-render-svg as the typed output of render_svg / render_svg_with_overlay, and require &SvgArtifact in render_html / render_html_with_overlay so the only zero-friction path is through the SVG renderer that already escapes user-derived content. SvgArtifact::from_trusted remains for callers that need to wrap an externally produced SVG and accept the same escaping contract.
…rkers Previously `<!--` / `-->` were rewritten as `<\!--` / `-\->`, which are not valid JSON escape sequences. Browser-side `JSON.parse` of the embedded metadata would therefore fail on any metadata containing those markers, leaving search/highlight/etc. with `null`. Use `<!--` and `-->` instead. The HTML parser still cannot see a literal `<!--` or `-->` in the script body, but `JSON.parse` losslessly recovers the original characters.
Code Metrics Report
Details | | main (b9a997f) | #163 (288f5d9) | +/- |
|---------------------|----------------|----------------|-------|
+ | Coverage | 94.4% | 94.4% | +0.0% |
| Files | 100 | 100 | 0 |
| Lines | 36742 | 36825 | +83 |
+ | Covered | 34704 | 34784 | +80 |
+ | Test Execution Time | 2m12s | 1m46s | -26s |Code coverage of files in pull request scope (94.4% → 94.4%)
Reported by octocov |
Schema reviewTip ✅ No risk findings — schema changes look safe to merge. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
</script>neutralization in embedded JSON metadata ASCII case-insensitive so</SCRIPT>and</Script foo>cannot terminate the script block in HTML parsers.<!--/-->neutralization to JSON Unicode escapes (</>) so the embedded metadata stays valid JSON andJSON.parserecovers the original characters losslessly.SvgArtifactnewtype produced by the SVG renderer and consumed byrender_html(_with_overlay), making it impossible to pass arbitrary&strinto the HTML embedding boundary by accident.Changes
escape_json_for_scriptas a byte scanner that detects</scriptcase-insensitively, preserves the original casing, and inserts only a backslash before the slash.matches_ascii_cihelper and tests for uppercase/mixed-case end tags and Unicode-bearing JSON.SvgArtifactnewtype torelune-render-svg;render_svg/render_svg_with_overlaynow returnResult<SvgArtifact, _>with an explicitfrom_trustedescape hatch.SvgArtifactfromrelune-render-htmland changerender_html/render_html_with_overlayto take&SvgArtifact, threading the typed boundary through the app use cases.<\!--/-\->output (which is not a valid JSON escape) with<!--/-->so browser-sideJSON.parseof the embedded metadata no longer fails on payloads containing HTML comment markers.serde_json::from_strback to the original characters.