From d6c952a6fc94d51f5d1faf398708bfa9bfeb9c40 Mon Sep 17 00:00:00 2001 From: Yang Jun Date: Mon, 6 Jul 2026 23:36:22 +0800 Subject: [PATCH] revert(memory): drop emitter output charge, restore filter output-size accounting join/array_to_sentence_string/json/inspect charge memoryLimit by the string they materialize (not element count), so discarded results like {% assign out = a | join %}{{ out | size }} are still bounded. Remove the emitter-level limiter added in 2f343f063; it cannot catch materialized-but-not-emitted values. Co-authored-by: Cursor --- src/drop/block-drop.ts | 1 - src/emitters/keeping-type-emitter.ts | 9 ++------- src/emitters/simple-emitter.ts | 8 ++------ src/emitters/streamed-emitter.ts | 7 ++----- src/filters/array.ts | 4 +++- src/filters/misc.ts | 12 +++++++++--- src/filters/string.ts | 4 +++- src/liquid-options.ts | 12 ++---------- src/render/render.ts | 4 ++-- test/integration/liquid/dos.spec.ts | 26 +++++++++++++++++++++++++- 10 files changed, 50 insertions(+), 37 deletions(-) diff --git a/src/drop/block-drop.ts b/src/drop/block-drop.ts index 51a75f930..2ff1aa297 100644 --- a/src/drop/block-drop.ts +++ b/src/drop/block-drop.ts @@ -13,7 +13,6 @@ export class BlockDrop extends Drop { * {{ block.super }} */ public * super (): IterableIterator { - // memory limit already enforced by final emitter, not passing memory here const emitter = new SimpleEmitter() yield this.superBlockRender(emitter) return emitter.buffer diff --git a/src/emitters/keeping-type-emitter.ts b/src/emitters/keeping-type-emitter.ts index 3061f0806..91d406286 100644 --- a/src/emitters/keeping-type-emitter.ts +++ b/src/emitters/keeping-type-emitter.ts @@ -1,11 +1,9 @@ -import { Limiter, stringify, toValue } from '../util' +import { stringify, toValue } from '../util' import { Emitter } from './emitter' export class KeepingTypeEmitter implements Emitter { public buffer: any = ''; - public constructor (private memoryLimit?: Limiter) {} - public write (html: any) { html = toValue(html) // This will only preserve the type if the value is isolated. @@ -13,12 +11,9 @@ export class KeepingTypeEmitter implements Emitter { // {{ my-port }} -> 42 // {{ my-host }}:{{ my-port }} -> 'host:42' if (typeof html !== 'string' && this.buffer === '') { - this.memoryLimit?.use(stringify(html).length) this.buffer = html } else { - const str = stringify(html) - this.memoryLimit?.use(str.length) - this.buffer = stringify(this.buffer) + str + this.buffer = stringify(this.buffer) + stringify(html) } } } diff --git a/src/emitters/simple-emitter.ts b/src/emitters/simple-emitter.ts index 7be791c9b..f1d048206 100644 --- a/src/emitters/simple-emitter.ts +++ b/src/emitters/simple-emitter.ts @@ -1,14 +1,10 @@ -import { Limiter, stringify } from '../util' +import { stringify } from '../util' import { Emitter } from './emitter' export class SimpleEmitter implements Emitter { public buffer = ''; - public constructor (private memoryLimit?: Limiter) {} - public write (html: any) { - const str = stringify(html) - this.memoryLimit?.use(str.length) - this.buffer += str + this.buffer += stringify(html) } } diff --git a/src/emitters/streamed-emitter.ts b/src/emitters/streamed-emitter.ts index 1a920b06d..6750ea3b4 100644 --- a/src/emitters/streamed-emitter.ts +++ b/src/emitters/streamed-emitter.ts @@ -1,15 +1,12 @@ -import { Limiter, stringify } from '../util' +import { stringify } from '../util' import { Emitter } from './emitter' import { PassThrough } from 'stream' export class StreamedEmitter implements Emitter { public buffer = ''; public stream: NodeJS.ReadWriteStream = new PassThrough() - public constructor (private memoryLimit?: Limiter) {} public write (html: any) { - const str = stringify(html) - this.memoryLimit?.use(str.length) - this.stream.write(str) + this.stream.write(stringify(html)) } public error (err: Error) { this.stream.emit('error', err) diff --git a/src/filters/array.ts b/src/filters/array.ts index 20caa95f6..57a7d802d 100644 --- a/src/filters/array.ts +++ b/src/filters/array.ts @@ -7,8 +7,10 @@ import { EmptyDrop } from '../drop' export const join = argumentsToValue(function (this: FilterImpl, v: any[], arg: string) { const array = toArray(v) - this.context.memoryLimit.use(array.length) const sep = isNil(arg) ? ' ' : stringify(arg) + let outputSize = sep.length * Math.max(array.length - 1, 0) + for (let i = 0; i < array.length; i++) outputSize += String(array[i]).length + this.context.memoryLimit.use(outputSize) return Array.prototype.join.call(array, sep) }) export const last = argumentsToValue(function (this: FilterImpl, v: any) { diff --git a/src/filters/misc.ts b/src/filters/misc.ts index 57ed57812..a80dd6ece 100644 --- a/src/filters/misc.ts +++ b/src/filters/misc.ts @@ -9,13 +9,19 @@ function defaultFilter (this: FilterImpl, value: T1, def return isFalsy(value, this.context) ? defaultValue : value } -function json (value: any, space = 0) { - return JSON.stringify(value, null, space) +function json (this: FilterImpl, value: any, space = 0) { + const memoryLimit = this.context.memoryLimit + return JSON.stringify(value, (_key, val) => { + memoryLimit.use(typeof val === 'string' ? val.length : 1) + return val + }, space) } -function inspect (value: any, space = 0) { +function inspect (this: FilterImpl, value: any, space = 0) { + const memoryLimit = this.context.memoryLimit const ancestors: object[] = [] return JSON.stringify(value, function (this: unknown, _key: unknown, value: any) { + memoryLimit.use(typeof value === 'string' ? value.length : 1) if (typeof value !== 'object' || value === null) return value // `this` is the object that value is contained in, i.e., its direct parent. while (ancestors.length > 0 && ancestors[ancestors.length - 1] !== this) ancestors.pop() diff --git a/src/filters/string.ts b/src/filters/string.ts index f96d4b492..d0c59708e 100644 --- a/src/filters/string.ts +++ b/src/filters/string.ts @@ -209,7 +209,9 @@ export function number_of_words (this: FilterImpl, input: string, mode?: 'cjk' | export function array_to_sentence_string (this: FilterImpl, array: unknown[], connector = 'and') { connector = stringify(connector) - this.context.memoryLimit.use(array.length + connector.length) + let outputSize = connector.length + array.length * 2 + for (let i = 0; i < array.length; i++) outputSize += stringify(array[i]).length + this.context.memoryLimit.use(outputSize) switch (array.length) { case 0: return '' diff --git a/src/liquid-options.ts b/src/liquid-options.ts index 4e9075af9..7ab1cbe2d 100644 --- a/src/liquid-options.ts +++ b/src/liquid-options.ts @@ -91,11 +91,7 @@ export interface LiquidOptions { parseLimit?: number; /** For DoS handling, limit total time (in ms) for each `render()` call. */ renderLimit?: number; - /** - * For DoS handling, caps the memory allocated while rendering. Operations allocating asymptotically more - * than their input (template and context) charge upfront to abort before allocating; everything else is - * charged as the output is written out. A typical PC can handle 1e9 (1G) memory without issue. - */ + /** For DoS handling, limit new objects creation, including array concat/join/strftime, etc. A typical PC can handle 1e9 (1G) memory without issue. */ memoryLimit?: number; } @@ -120,11 +116,7 @@ export interface RenderOptions { templateLimit?: number; /** For DoS handling, limit total time (in ms) for each `render()` call. */ renderLimit?: number; - /** - * For DoS handling, caps the memory allocated while rendering. Operations allocating asymptotically more - * than their input (template and context) charge upfront to abort before allocating; everything else is - * charged as the output is written out. A typical PC can handle 1e9 (1G) memory without issue. - */ + /** For DoS handling, limit new objects creation, including array concat/join/strftime, etc. A typical PC can handle 1e9 (1G) memory without issue.. */ memoryLimit?: number; } diff --git a/src/render/render.ts b/src/render/render.ts index b9be0b9ee..8d0b7d806 100644 --- a/src/render/render.ts +++ b/src/render/render.ts @@ -6,14 +6,14 @@ import { Emitter, KeepingTypeEmitter, StreamedEmitter, SimpleEmitter } from '../ export class Render { public renderTemplatesToNodeStream (templates: Template[], ctx: Context): NodeJS.ReadableStream { - const emitter = new StreamedEmitter(ctx.memoryLimit) + const emitter = new StreamedEmitter() Promise.resolve().then(() => toPromise(this.renderTemplates(templates, ctx, emitter))) .then(() => emitter.end(), err => emitter.error(err)) return emitter.stream } public * renderTemplates (templates: Template[], ctx: Context, emitter?: Emitter): IterableIterator { if (!emitter) { - emitter = ctx.opts.keepOutputType ? new KeepingTypeEmitter(ctx.memoryLimit) : new SimpleEmitter(ctx.memoryLimit) + emitter = ctx.opts.keepOutputType ? new KeepingTypeEmitter() : new SimpleEmitter() } ctx.renderLimit.check(getPerformance().now()) const errors = [] diff --git a/test/integration/liquid/dos.spec.ts b/test/integration/liquid/dos.spec.ts index 0ce9712d0..65d78adb5 100644 --- a/test/integration/liquid/dos.spec.ts +++ b/test/integration/liquid/dos.spec.ts @@ -70,7 +70,7 @@ describe('DoS related', function () { const array = Array(1e3).fill(0) const liquid = new Liquid({ memoryLimit: 100 }) await expect(liquid.parseAndRender('{{ array | slice: 0, 300 | join }}', { array })).rejects.toThrow('memory alloc limit exceeded, line:1, col:1') - await expect(liquid.parseAndRender('{{ array | slice: 0, 300 | join }}', { array }, { memoryLimit: 2e3 })).resolves.toBe(Array(300).fill(0).join(' ')) + await expect(liquid.parseAndRender('{{ array | slice: 0, 300 | join }}', { array }, { memoryLimit: 1e3 })).resolves.toBe(Array(300).fill(0).join(' ')) }) it('should throw for too many array iteration in tags', async () => { const array = ['a'] @@ -100,12 +100,36 @@ describe('DoS related', function () { const liquid = new Liquid({ memoryLimit: 100 }) expect(liquid.parseAndRenderSync('{{ array | join: "" }}', { array })).toBe('a'.repeat(20) + 'b'.repeat(20)) }) + it('should prevent concat doubling from bypassing join memoryLimit', () => { + const liquid = new Liquid({ memoryLimit: 1e4 }) + const src = '{%- assign a = s | split: "NOSEP" -%}' + + '{%- assign a = a | concat: a -%}{%- assign a = a | concat: a -%}{%- assign a = a | concat: a -%}' + + '{{ a | join: "" | size }}' + expect(() => liquid.parseAndRenderSync(src, { s: 'a'.repeat(5000) })) + .toThrow('memory alloc limit exceeded') + }) it('should charge array_to_sentence_string by produced output size', () => { const array = ['a'.repeat(100), 'b'.repeat(100), 'c'.repeat(100)] const liquid = new Liquid({ memoryLimit: 100 }) expect(() => liquid.parseAndRenderSync('{{ array | array_to_sentence_string }}', { array })) .toThrow('memory alloc limit exceeded') }) + it('should charge json serialization of concat-doubled arrays', () => { + const liquid = new Liquid({ memoryLimit: 1e4 }) + const src = '{%- assign a = s | split: "NOSEP" -%}' + + '{%- assign a = a | concat: a -%}{%- assign a = a | concat: a -%}{%- assign a = a | concat: a -%}' + + '{{ a | json | size }}' + expect(() => liquid.parseAndRenderSync(src, { s: 'a'.repeat(5000) })) + .toThrow('memory alloc limit exceeded') + }) + it('should charge inspect serialization of concat-doubled arrays', () => { + const liquid = new Liquid({ memoryLimit: 1e4 }) + const src = '{%- assign a = s | split: "NOSEP" -%}' + + '{%- assign a = a | concat: a -%}{%- assign a = a | concat: a -%}{%- assign a = a | concat: a -%}' + + '{{ a | inspect | size }}' + expect(() => liquid.parseAndRenderSync(src, { s: 'a'.repeat(5000) })) + .toThrow('memory alloc limit exceeded') + }) 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) }))