mirror of
https://github.com/harttle/liquidjs.git
synced 2026-09-15 04:10:40 -07:00
fix(strip_html): rewrite as linear single-pass scan to avoid ReDoS (#896)
* fix(strip_html): rewrite as linear single-pass scan to avoid ReDoS The previous strip_html regex /<script[\s\S]*?<\/script>|<style[\s\S]*?<\/style>|<[\s\S]*?>|<!--[\s\S]*?-->/g contains lazy alternatives that backtrack O(n^2) on inputs with many unclosed `<script` / `<style` openers. A 350KB payload of `'<script'.repeat(50000)` blocked the Node.js event loop for ~10s, and cost grew quadratically with input size. memoryLimit only charged str.length, which does not bound regex CPU. Replace the regex with an indexOf-based single-pass scan. For each `<` we: - if `<script` opener: find next `</script>` and skip the whole block; cache "no closer after pos k" so subsequent unclosed `<script` openers do not re-scan the tail. - same for `<style` / `</style>`. - otherwise treat as a generic `<...>` tag (matches the original behavior, where the `<[\s\S]*?>` alternative also caught comments). - if no closing `>` exists, emit the tail as literal text and stop. Total work is O(n). All existing strip_html test cases pass unchanged. Add regression tests covering the PoCs (`<script` / `<style` repeats, and `<script>foo` repeats with `>` but no `</script>`) plus a memoryLimit assertion. Co-authored-by: Cursor <[email protected]> * refactor(strip_html): factor block kinds into a small table Same algorithm and complexity, fewer lines. Document why a regex-only solution can't be O(n) in V8 (no atomic groups / possessive quantifiers / memoization, so unrolled-loop patterns are still O(n^2) on unclosed openers — empirically confirmed: original 280KB ~4s, Friedl unrolled ~14s, atomic lookahead ~7s; tokenizer ~1ms). Co-authored-by: Cursor <[email protected]> * refactor(strip_html): inline block kinds to match file style Drop the module-level STRIP_BLOCKS table; the rest of the file keeps each filter self-contained (only escapeMap/unescapeMap are top-level maps shared across filters). Two openers don't justify a table. Co-authored-by: Cursor <[email protected]> * refactor(strip_html): unify raw-text blocks; treat <!--...--> as opaque In HTML5, <script>, <style>, and <!-- --> are all raw-text blocks: their content is opaque until the matching closer, so a `>` inside CSS, JS, or a comment must not be treated as a tag end. The previous code only had this special handling for <script> and <style>; comments containing `>` fell through to the generic `<...>` branch and were partially stripped (e.g. `<!-- a > b -->` left `b -->` in the output). Match Shopify Liquid's STRIP_HTML_BLOCKS set (script + style + comment), and consolidate the three near-identical branches into a small opener/closer table inside the function. Algorithm and complexity unchanged (O(n) via indexOf + cached closer positions). Add a regression test for `>` inside a comment. Co-authored-by: Cursor <[email protected]> * refactor(strip_html): drop position cache, delete dead blocks from Set Once `indexOf(closer, X)` returns -1, all subsequent searches (with monotonically increasing start) also return -1. So tracking absence is enough; storing positions is unnecessary. Make `blocks` a Set and delete a kind once its closer is known absent — no parallel `dead` bookkeeping. Use Jest's per-test timeout for the ReDoS regressions instead of manual Date.now() bookkeeping. Co-authored-by: Cursor <[email protected]> * refactor(strip_html): treat '<...>' as a catch-all block kind Adding ['<', '>'] as the lowest-priority entry of `blocks` lets the inner loop subsume the generic-tag fallback: the `end` sentinel and its `< 0` / `<= 0` follow-up checks disappear, the "no terminator" exit becomes a single `i === lt` test, and Set<[string, string]> collapses to Map<string, string>. Co-authored-by: Cursor <[email protected]> --------- Co-authored-by: Cursor <[email protected]>
This commit is contained in:
@@ -57,6 +57,9 @@ describe('filters/html', function () {
|
||||
it('should strip multiline comments', function () {
|
||||
expect(liquid.parseAndRenderSync('{{"<!--foo\r\nbar \ncoo\t \r\n -->"|strip_html}}')).toBe('')
|
||||
})
|
||||
it('should treat > inside comments as comment content (not a tag end)', function () {
|
||||
expect(liquid.parseAndRenderSync('{{ "<!-- a > b -->after" | strip_html }}')).toBe('after')
|
||||
})
|
||||
it('should strip all style tags and their contents', function () {
|
||||
return test('{{ "<style>cite { font-style: italic; }</style><cite>Ulysses<cite>?" | strip_html }}',
|
||||
'Ulysses?')
|
||||
|
||||
@@ -79,5 +79,30 @@ describe('DoS related', function () {
|
||||
await expect(liquid.parseAndRender(src, { array, count: 3 })).resolves.toBe('a a a a a a a a')
|
||||
await expect(liquid.parseAndRender(src, { array, count: 100 })).rejects.toThrow('memory alloc limit exceeded, line:1, col:26')
|
||||
})
|
||||
it('should charge strip_html input length to memoryLimit', () => {
|
||||
const liquid = new Liquid({ memoryLimit: 100 })
|
||||
expect(() => liquid.parseAndRenderSync('{{ s | strip_html }}', { s: 'a'.repeat(200) }))
|
||||
.toThrow('memory alloc limit exceeded')
|
||||
})
|
||||
})
|
||||
describe('strip_html ReDoS', () => {
|
||||
// Regression for O(n^2) backtracking on unclosed `<script` / `<style` openers.
|
||||
// The previous regex stalled the event loop for ~10s on 350KB of `'<script'.repeat`.
|
||||
// The per-test timeout below caps total time; an O(n^2) regression would blow it.
|
||||
it('should handle many unclosed <script openers in linear time', () => {
|
||||
const liquid = new Liquid()
|
||||
const payload = '<script'.repeat(50000)
|
||||
expect(liquid.parseAndRenderSync('{{ x | strip_html }}', { x: payload })).toBe(payload)
|
||||
}, 1000)
|
||||
it('should handle many unclosed <style openers in linear time', () => {
|
||||
const liquid = new Liquid()
|
||||
const payload = '<style'.repeat(50000)
|
||||
expect(liquid.parseAndRenderSync('{{ x | strip_html }}', { x: payload })).toBe(payload)
|
||||
}, 1000)
|
||||
it('should handle <script openers that have > but no </script> in linear time', () => {
|
||||
const liquid = new Liquid()
|
||||
const payload = '<script>foo'.repeat(50000)
|
||||
expect(liquid.parseAndRenderSync('{{ x | strip_html }}', { x: payload })).toBe('foo'.repeat(50000))
|
||||
}, 1000)
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user