From cb72bf6f016904d8f883394a9789a67a1865f9d4 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Mon, 23 Mar 2026 20:26:00 +0800 Subject: [PATCH 1/4] chore: fix pre-existing biome lint in template.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - isNaN → Number.isNaN (2 occurrences) - string concatenation → template literal - biome-ignore for intentional control chars in sanitize regex --- src/pipeline/template.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/pipeline/template.ts b/src/pipeline/template.ts index 8535bba0..7e7d2dc5 100644 --- a/src/pipeline/template.ts +++ b/src/pipeline/template.ts @@ -55,7 +55,7 @@ export function evalExpr(expr: string, ctx: RenderContext): unknown { const val = resolvePath(varName, { args, item, data, index }); if (val !== null && val !== undefined) { const numVal = Number(val); const num = Number(numStr); - if (!isNaN(numVal)) { + if (!Number.isNaN(numVal)) { switch (op) { case '+': return numVal + num; case '-': return numVal - num; case '*': return numVal * num; case '/': return num !== 0 ? numVal / num : 0; @@ -95,7 +95,7 @@ function applyFilter(filterExpr: string, value: unknown): unknown { case 'default': { if (value === null || value === undefined || value === '') { const intVal = parseInt(filterArg, 10); - if (!isNaN(intVal) && String(intVal) === filterArg.trim()) return intVal; + if (!Number.isNaN(intVal) && String(intVal) === filterArg.trim()) return intVal; return filterArg; } return value; @@ -110,7 +110,7 @@ function applyFilter(filterExpr: string, value: unknown): unknown { return typeof value === 'string' ? value.trim() : value; case 'truncate': { const n = parseInt(filterArg, 10) || 50; - return typeof value === 'string' && value.length > n ? value.slice(0, n) + '...' : value; + return typeof value === 'string' && value.length > n ? `${value.slice(0, n)}...` : value; } case 'replace': { if (typeof value !== 'string') return value; @@ -138,6 +138,7 @@ function applyFilter(filterExpr: string, value: unknown): unknown { case 'sanitize': // Remove invalid filename characters return typeof value === 'string' + // biome-ignore lint/suspicious/noControlCharactersInRegex: intentional - strips C0 control chars from filenames ? value.replace(/[<>:"/\\|?*\x00-\x1f]/g, '_') : value; case 'ext': { From 9e6b2a1cd9595a9d962fc7ed7da51960522a4a4e Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Mon, 23 Mar 2026 20:26:24 +0800 Subject: [PATCH 2/4] fix(pipeline): evaluate chained || in template engine (#303) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The || handler in evalExpr returned the right side as a literal string instead of recursively evaluating it. This broke chained fallbacks like `item.a || item.b || 'default'` — when item.a was falsy, the entire `item.b || 'default'` was returned as text. Fix: call evalExpr on the right side so chained || works at any depth. --- src/pipeline/template.test.ts | 22 ++++++++++++++++++++++ src/pipeline/template.ts | 5 +++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/pipeline/template.test.ts b/src/pipeline/template.test.ts index 5805ef80..f5876c38 100644 --- a/src/pipeline/template.test.ts +++ b/src/pipeline/template.test.ts @@ -54,6 +54,28 @@ describe('evalExpr', () => { it('evaluates || with truthy left', () => { expect(evalExpr("item.name || 'N/A'", { item: { name: 'Alice' } })).toBe('Alice'); }); + it('evaluates chained || fallback (issue #303)', () => { + // When first two are falsy, should evaluate through to the string literal + expect(evalExpr("item.a || item.b || 'default'", { item: {} })).toBe('default'); + }); + it('evaluates chained || with middle value truthy', () => { + expect(evalExpr("item.a || item.b || 'default'", { item: { b: 'middle' } })).toBe('middle'); + }); + it('evaluates chained || with first value truthy', () => { + expect(evalExpr("item.a || item.b || 'default'", { item: { a: 'first', b: 'middle' } })).toBe('first'); + }); + it('evaluates || with 0 as falsy left (JS semantics)', () => { + expect(evalExpr("item.count || 'N/A'", { item: { count: 0 } })).toBe('N/A'); + }); + it('evaluates || with empty string as falsy left', () => { + expect(evalExpr("item.name || 'unknown'", { item: { name: '' } })).toBe('unknown'); + }); + it('evaluates || with numeric fallback returning number type', () => { + expect(evalExpr('item.a || 42', { item: {} })).toBe(42); + }); + it('evaluates 4-way chained ||', () => { + expect(evalExpr("item.a || item.b || item.c || 'last'", { item: { c: 'third' } })).toBe('third'); + }); it('resolves simple path', () => { expect(evalExpr('item.title', { item: { title: 'Test' } })).toBe('Test'); }); diff --git a/src/pipeline/template.ts b/src/pipeline/template.ts index 7e7d2dc5..5e9a4217 100644 --- a/src/pipeline/template.ts +++ b/src/pipeline/template.ts @@ -65,12 +65,13 @@ export function evalExpr(expr: string, ctx: RenderContext): unknown { } // JS-like fallback expression: item.tweetCount || 'N/A' + // Recursively evaluate the right side so chained || works: + // item.a || item.b || 'default' → eval(item.a) || eval(item.b || 'default') const orMatch = expr.match(/^(.+?)\s*\|\|\s*(.+)$/); if (orMatch) { const left = evalExpr(orMatch[1].trim(), ctx); if (left) return left; - const right = orMatch[2].trim(); - return right.replace(/^['"]|['"]$/g, ''); + return evalExpr(orMatch[2].trim(), ctx); } const resolved = resolvePath(expr, { args, item, data, index }); From 170a35004249588203b1cd16d6b8b4bfbb4ebe27 Mon Sep 17 00:00:00 2001 From: jackwener Date: Wed, 25 Mar 2026 13:28:22 +0800 Subject: [PATCH 3/4] perf(pipeline): fast-path string literals in evalExpr to skip VM When the right side of || is a quoted string like 'N/A', detect it with a simple regex and return directly instead of falling through to evalJsExpr which spins up a node:vm sandbox. --- src/pipeline/template.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/pipeline/template.ts b/src/pipeline/template.ts index 5e9a4217..9e6542e5 100644 --- a/src/pipeline/template.ts +++ b/src/pipeline/template.ts @@ -74,6 +74,10 @@ export function evalExpr(expr: string, ctx: RenderContext): unknown { return evalExpr(orMatch[2].trim(), ctx); } + // Fast path for quoted string literals – avoids VM overhead from evalJsExpr + const strLit = expr.match(/^(['"])(.*)\1$/); + if (strLit) return strLit[2]; + const resolved = resolvePath(expr, { args, item, data, index }); if (resolved !== null && resolved !== undefined) return resolved; From 5a1d0bc237bb38bb50787339734dd98e8651cc2c Mon Sep 17 00:00:00 2001 From: jackwener Date: Wed, 25 Mar 2026 13:41:48 +0800 Subject: [PATCH 4/4] refactor(pipeline): simplify evalExpr by removing hand-rolled operator parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the manual regex-based || and arithmetic handlers with a streamlined flow: pipe filters → fast-path literals → resolvePath → evalJsExpr (VM). The VM already handles ||, ??, arithmetic, ternary, etc. natively, so reimplementing them with regex was redundant and bug-prone (see issue #303). Key improvements: - Fix pipe | vs || disambiguation with lookbehind/lookahead regex (? { it('evaluates 4-way chained ||', () => { expect(evalExpr("item.a || item.b || item.c || 'last'", { item: { c: 'third' } })).toBe('third'); }); + it('handles || combined with pipe filter', () => { + expect(evalExpr("item.a || item.b | upper", { item: { b: 'hello' } })).toBe('HELLO'); + }); it('resolves simple path', () => { expect(evalExpr('item.title', { item: { title: 'Test' } })).toBe('Test'); }); diff --git a/src/pipeline/template.ts b/src/pipeline/template.ts index 9e6542e5..1f34bbe5 100644 --- a/src/pipeline/template.ts +++ b/src/pipeline/template.ts @@ -37,50 +37,29 @@ export function evalExpr(expr: string, ctx: RenderContext): unknown { const index = ctx.index ?? 0; // ── Pipe filters: expr | filter1(arg) | filter2 ── - // Supports: default(val), join(sep), upper, lower, truncate(n), trim, replace(old,new) - if (expr.includes('|') && !expr.includes('||')) { - const segments = expr.split('|').map(s => s.trim()); - const mainExpr = segments[0]; - let result = resolvePath(mainExpr, { args, item, data, index }); - for (let i = 1; i < segments.length; i++) { - result = applyFilter(segments[i], result); + // Split on single | (not ||) so "item.a || item.b | upper" works correctly. + const pipeSegments = expr.split(/(? s.trim()); + if (pipeSegments.length > 1) { + let result = evalExpr(pipeSegments[0], ctx); + for (let i = 1; i < pipeSegments.length; i++) { + result = applyFilter(pipeSegments[i], result); } return result; } - // Arithmetic: index + 1 - const arithMatch = expr.match(/^([\w][\w.]*)\s*([+\-*/])\s*(\d+)$/); - if (arithMatch) { - const [, varName, op, numStr] = arithMatch; - const val = resolvePath(varName, { args, item, data, index }); - if (val !== null && val !== undefined) { - const numVal = Number(val); const num = Number(numStr); - if (!Number.isNaN(numVal)) { - switch (op) { - case '+': return numVal + num; case '-': return numVal - num; - case '*': return numVal * num; case '/': return num !== 0 ? numVal / num : 0; - } - } - } - } - - // JS-like fallback expression: item.tweetCount || 'N/A' - // Recursively evaluate the right side so chained || works: - // item.a || item.b || 'default' → eval(item.a) || eval(item.b || 'default') - const orMatch = expr.match(/^(.+?)\s*\|\|\s*(.+)$/); - if (orMatch) { - const left = evalExpr(orMatch[1].trim(), ctx); - if (left) return left; - return evalExpr(orMatch[2].trim(), ctx); - } - - // Fast path for quoted string literals – avoids VM overhead from evalJsExpr + // Fast path: quoted string literal — skip VM overhead const strLit = expr.match(/^(['"])(.*)\1$/); if (strLit) return strLit[2]; + // Fast path: numeric literal + if (/^\d+(\.\d+)?$/.test(expr)) return Number(expr); + + // Try resolving as a simple dotted path (item.foo.bar, args.limit, index) const resolved = resolvePath(expr, { args, item, data, index }); if (resolved !== null && resolved !== undefined) return resolved; + // Fallback: evaluate as JS in a sandboxed VM. + // Handles ||, ??, arithmetic, ternary, method calls, etc. natively. return evalJsExpr(expr, { args, item, data, index }); }