mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-05 13:10:24 +00:00
[fix] Replace eval() with safe math expression parser (#9263)
## Summary Replace `eval()` in `evaluateInput()` with a custom recursive descent math parser, eliminating a security concern and enabling the `no-eval` lint rule. ## Changes - **New**: `mathParser.ts` — recursive descent parser for `+`, `-`, `*`, `/`, `%`, `()`, decimals, unary operators. Zero new dependencies. - **Modified**: `widget.ts` — replaced `eval()` call with `evaluateMathExpression()`, use `isFinite()` instead of `isNaN()` to reject `Infinity` - **Modified**: `.oxlintrc.json` — `no-eval` rule changed from `"off"` to `"error"` - **Tests**: 59 parser tests + 23 integration tests covering complex expressions, edge cases, and invalid input ## Review Feedback Addressed - Renamed `unit()` → `primary()` for clarity - Added modulo (`%`) operator support - Normalized negative zero to positive zero - Added depth limit (200) for nested parentheses - Used `isFinite()` instead of `isNaN()` to reject `Infinity`/`-Infinity` - Added tests for edge-case number formats, unary-after-binary operators, modulo, depth limits, scientific/hex notation, and `Infinity` Fixes #8032 Fixes #9272 Fixes #9273 Fixes #9274 Fixes #9275 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9263-fix-Replace-eval-with-safe-math-expression-parser-3136d73d3650812f9f8dea21d1ea4f06) by [Unito](https://www.unito.io)
This commit is contained in:
committed by
GitHub
parent
82750d629d
commit
df712953a3
@@ -35,7 +35,7 @@
|
||||
}
|
||||
],
|
||||
"no-control-regex": "off",
|
||||
"no-eval": "off",
|
||||
"no-eval": "error",
|
||||
"no-redeclare": "error",
|
||||
"no-restricted-imports": [
|
||||
"error",
|
||||
|
||||
@@ -145,7 +145,11 @@ export { isColorable } from './utils/type'
|
||||
export { createUuidv4 } from './utils/uuid'
|
||||
export type { UUID } from './utils/uuid'
|
||||
export { truncateText } from './utils/textUtils'
|
||||
export { getWidgetStep, resolveNodeRootGraphId } from './utils/widget'
|
||||
export {
|
||||
evaluateInput,
|
||||
getWidgetStep,
|
||||
resolveNodeRootGraphId
|
||||
} from './utils/widget'
|
||||
export { distributeSpace, type SpaceRequest } from './utils/spaceDistribution'
|
||||
|
||||
export { BaseWidget } from './widgets/BaseWidget'
|
||||
|
||||
121
src/lib/litegraph/src/utils/mathParser.test.ts
Normal file
121
src/lib/litegraph/src/utils/mathParser.test.ts
Normal file
@@ -0,0 +1,121 @@
|
||||
import { describe, expect, test } from 'vitest'
|
||||
|
||||
import { evaluateMathExpression } from '@/lib/litegraph/src/utils/mathParser'
|
||||
|
||||
describe('evaluateMathExpression', () => {
|
||||
test.each([
|
||||
['2+3', 5],
|
||||
['10-4', 6],
|
||||
['3*7', 21],
|
||||
['15/3', 5]
|
||||
])('basic arithmetic: %s = %d', (input, expected) => {
|
||||
expect(evaluateMathExpression(input)).toBe(expected)
|
||||
})
|
||||
|
||||
test.each([
|
||||
['2+3*4', 14],
|
||||
['(2+3)*4', 20],
|
||||
['10-2*3', 4],
|
||||
['10/2+3', 8]
|
||||
])('operator precedence: %s = %d', (input, expected) => {
|
||||
expect(evaluateMathExpression(input)).toBe(expected)
|
||||
})
|
||||
|
||||
test.each([
|
||||
['3.14*2', 6.28],
|
||||
['.5+.5', 1],
|
||||
['1.5+2.5', 4],
|
||||
['0.1+0.2', 0.1 + 0.2],
|
||||
['123.', 123],
|
||||
['123.+3', 126]
|
||||
])('decimals: %s', (input, expected) => {
|
||||
expect(evaluateMathExpression(input)).toBe(expected)
|
||||
})
|
||||
|
||||
test.each([
|
||||
[' 2 + 3 ', 5],
|
||||
[' 10 - 4 ', 6],
|
||||
[' ( 2 + 3 ) * 4 ', 20]
|
||||
])('whitespace handling: "%s" = %d', (input, expected) => {
|
||||
expect(evaluateMathExpression(input)).toBe(expected)
|
||||
})
|
||||
|
||||
test.each([
|
||||
['((2+3))', 5],
|
||||
['(1+(2*(3+4)))', 15],
|
||||
['((1+2)*(3+4))', 21]
|
||||
])('nested parentheses: %s = %d', (input, expected) => {
|
||||
expect(evaluateMathExpression(input)).toBe(expected)
|
||||
})
|
||||
|
||||
test.each([
|
||||
['-5', -5],
|
||||
['-(3+2)', -5],
|
||||
['--5', 5],
|
||||
['+5', 5],
|
||||
['-3*2', -6],
|
||||
['2*-3', -6],
|
||||
['1+-2', -1],
|
||||
['2--3', 5],
|
||||
['-2*-3', 6],
|
||||
['-(2+3)*-(4+5)', 45]
|
||||
])('unary operators: %s = %d', (input, expected) => {
|
||||
expect(evaluateMathExpression(input)).toBe(expected)
|
||||
})
|
||||
|
||||
test.each([
|
||||
['2 /2+3 * 4.75- -6', 21.25],
|
||||
['2 / (2 + 3) * 4.33 - -6', 7.732],
|
||||
['12* 123/-(-5 + 2)', 492],
|
||||
['((80 - (19)))', 61],
|
||||
['(1 - 2) + -(-(-(-4)))', 3],
|
||||
['1 - -(-(-(-4)))', -3],
|
||||
['12* 123/(-5 + 2)', -492],
|
||||
['12 * -123', -1476],
|
||||
['((2.33 / (2.9+3.5)*4) - -6)', 7.45625],
|
||||
['123.45*(678.90 / (-2.5+ 11.5)-(80 -19) *33.25) / 20 + 11', -12042.760875],
|
||||
[
|
||||
'(123.45*(678.90 / (-2.5+ 11.5)-(((80 -(19))) *33.25)) / 20) - (123.45*(678.90 / (-2.5+ 11.5)-(((80 -(19))) *33.25)) / 20) + (13 - 2)/ -(-11) ',
|
||||
1
|
||||
]
|
||||
])('complex expression: %s', (input, expected) => {
|
||||
expect(evaluateMathExpression(input)).toBeCloseTo(expected as number)
|
||||
})
|
||||
|
||||
test.each(['', 'abc', '2+', '(2+3', '2+3)', '()', '*3', '2 3', '.', '123..'])(
|
||||
'invalid input returns undefined: "%s"',
|
||||
(input) => {
|
||||
expect(evaluateMathExpression(input)).toBeUndefined()
|
||||
}
|
||||
)
|
||||
|
||||
test('division by zero returns Infinity', () => {
|
||||
expect(evaluateMathExpression('1/0')).toBe(Infinity)
|
||||
})
|
||||
|
||||
test('0/0 returns NaN', () => {
|
||||
expect(evaluateMathExpression('0/0')).toBeNaN()
|
||||
})
|
||||
|
||||
test.each([
|
||||
['10%3', 1],
|
||||
['10%3+1', 2],
|
||||
['7%2', 1]
|
||||
])('modulo: %s = %d', (input, expected) => {
|
||||
expect(evaluateMathExpression(input)).toBe(expected)
|
||||
})
|
||||
|
||||
test('negative zero is normalized to positive zero', () => {
|
||||
expect(Object.is(evaluateMathExpression('-0'), 0)).toBe(true)
|
||||
})
|
||||
|
||||
test('deeply nested parentheses exceeding depth limit returns undefined', () => {
|
||||
const input = '('.repeat(201) + '1' + ')'.repeat(201)
|
||||
expect(evaluateMathExpression(input)).toBeUndefined()
|
||||
})
|
||||
|
||||
test('parentheses within depth limit evaluate correctly', () => {
|
||||
const input = '('.repeat(200) + '1' + ')'.repeat(200)
|
||||
expect(evaluateMathExpression(input)).toBe(1)
|
||||
})
|
||||
})
|
||||
116
src/lib/litegraph/src/utils/mathParser.ts
Normal file
116
src/lib/litegraph/src/utils/mathParser.ts
Normal file
@@ -0,0 +1,116 @@
|
||||
type Token = { type: 'number'; value: number } | { type: 'op'; value: string }
|
||||
|
||||
function tokenize(input: string): Token[] | undefined {
|
||||
const tokens: Token[] = []
|
||||
const re = /(\d+(?:\.\d*)?|\.\d+)|([+\-*/%()])/g
|
||||
let lastIndex = 0
|
||||
|
||||
for (const match of input.matchAll(re)) {
|
||||
const gap = input.slice(lastIndex, match.index)
|
||||
if (gap.trim()) return undefined
|
||||
lastIndex = match.index + match[0].length
|
||||
|
||||
if (match[1]) tokens.push({ type: 'number', value: parseFloat(match[1]) })
|
||||
else tokens.push({ type: 'op', value: match[2] })
|
||||
}
|
||||
|
||||
if (input.slice(lastIndex).trim()) return undefined
|
||||
return tokens
|
||||
}
|
||||
|
||||
/**
|
||||
* Evaluates a basic arithmetic expression string containing
|
||||
* `+`, `-`, `*`, `/`, `%`, parentheses, and decimal numbers.
|
||||
* Returns `undefined` for empty or malformed input.
|
||||
*/
|
||||
export function evaluateMathExpression(input: string): number | undefined {
|
||||
const tokenized = tokenize(input)
|
||||
if (!tokenized || tokenized.length === 0) return undefined
|
||||
|
||||
const tokens: Token[] = tokenized
|
||||
let pos = 0
|
||||
let depth = 0
|
||||
const MAX_DEPTH = 200
|
||||
|
||||
function peek(): Token | undefined {
|
||||
return tokens[pos]
|
||||
}
|
||||
|
||||
function consume(): Token {
|
||||
return tokens[pos++]
|
||||
}
|
||||
|
||||
function primary(): number | undefined {
|
||||
const t = peek()
|
||||
if (!t) return undefined
|
||||
|
||||
if (t.type === 'number') {
|
||||
consume()
|
||||
return t.value
|
||||
}
|
||||
|
||||
if (t.type === 'op' && t.value === '(') {
|
||||
if (++depth > MAX_DEPTH) return undefined
|
||||
consume()
|
||||
const result = expr()
|
||||
if (result === undefined) return undefined
|
||||
const closing = peek()
|
||||
if (!closing || closing.type !== 'op' || closing.value !== ')') {
|
||||
return undefined
|
||||
}
|
||||
consume()
|
||||
depth--
|
||||
return result
|
||||
}
|
||||
|
||||
return undefined
|
||||
}
|
||||
|
||||
function unary(): number | undefined {
|
||||
const t = peek()
|
||||
if (t?.type === 'op' && (t.value === '+' || t.value === '-')) {
|
||||
consume()
|
||||
const operand = unary()
|
||||
if (operand === undefined) return undefined
|
||||
return t.value === '-' ? -operand : operand
|
||||
}
|
||||
return primary()
|
||||
}
|
||||
|
||||
function factor(): number | undefined {
|
||||
let left = unary()
|
||||
if (left === undefined) return undefined
|
||||
|
||||
while (
|
||||
peek()?.type === 'op' &&
|
||||
(peek()!.value === '*' || peek()!.value === '/' || peek()!.value === '%')
|
||||
) {
|
||||
const op = consume().value
|
||||
const right = unary()
|
||||
if (right === undefined) return undefined
|
||||
left =
|
||||
op === '*' ? left * right : op === '/' ? left / right : left % right
|
||||
}
|
||||
return left
|
||||
}
|
||||
|
||||
function expr(): number | undefined {
|
||||
let left = factor()
|
||||
if (left === undefined) return undefined
|
||||
|
||||
while (
|
||||
peek()?.type === 'op' &&
|
||||
(peek()!.value === '+' || peek()!.value === '-')
|
||||
) {
|
||||
const op = consume().value
|
||||
const right = factor()
|
||||
if (right === undefined) return undefined
|
||||
left = op === '+' ? left + right : left - right
|
||||
}
|
||||
return left
|
||||
}
|
||||
|
||||
const result = expr()
|
||||
if (result === undefined || pos !== tokens.length) return undefined
|
||||
return result === 0 ? 0 : result
|
||||
}
|
||||
@@ -3,6 +3,7 @@ import { describe, expect, test } from 'vitest'
|
||||
import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
|
||||
import type { IWidgetOptions } from '@/lib/litegraph/src/litegraph'
|
||||
import {
|
||||
evaluateInput,
|
||||
getWidgetStep,
|
||||
resolveNodeRootGraphId
|
||||
} from '@/lib/litegraph/src/litegraph'
|
||||
@@ -70,3 +71,57 @@ describe('resolveNodeRootGraphId', () => {
|
||||
expect(resolveNodeRootGraphId(node, 'app-root-id')).toBe('app-root-id')
|
||||
})
|
||||
})
|
||||
|
||||
describe('evaluateInput', () => {
|
||||
test.each([
|
||||
['42', 42],
|
||||
['3.14', 3.14],
|
||||
['-7', -7],
|
||||
['0', 0]
|
||||
])('plain number: "%s" = %d', (input, expected) => {
|
||||
expect(evaluateInput(input)).toBe(expected)
|
||||
})
|
||||
|
||||
test.each([
|
||||
['2+3', 5],
|
||||
['(4+2)*3', 18],
|
||||
['3.14*2', 6.28],
|
||||
['10/2+3', 8]
|
||||
])('expression: "%s" = %d', (input, expected) => {
|
||||
expect(evaluateInput(input)).toBe(expected)
|
||||
})
|
||||
|
||||
test('empty string returns 0 (Number("") === 0)', () => {
|
||||
expect(evaluateInput('')).toBe(0)
|
||||
})
|
||||
|
||||
test.each(['abc', 'hello world'])(
|
||||
'invalid input returns undefined: "%s"',
|
||||
(input) => {
|
||||
expect(evaluateInput(input)).toBeUndefined()
|
||||
}
|
||||
)
|
||||
|
||||
test('division by zero returns undefined', () => {
|
||||
expect(evaluateInput('1/0')).toBeUndefined()
|
||||
})
|
||||
|
||||
test('0/0 returns undefined (NaN is filtered)', () => {
|
||||
expect(evaluateInput('0/0')).toBeUndefined()
|
||||
})
|
||||
|
||||
test('scientific notation via Number() fallback', () => {
|
||||
expect(evaluateInput('1e5')).toBe(100000)
|
||||
})
|
||||
|
||||
test('hex notation via Number() fallback', () => {
|
||||
expect(evaluateInput('0xff')).toBe(255)
|
||||
})
|
||||
|
||||
test.each(['Infinity', '-Infinity'])(
|
||||
'"%s" returns undefined (non-finite rejected)',
|
||||
(input) => {
|
||||
expect(evaluateInput(input)).toBeUndefined()
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
@@ -2,6 +2,8 @@ import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
|
||||
import type { IWidgetOptions } from '@/lib/litegraph/src/types/widgets'
|
||||
import type { UUID } from '@/lib/litegraph/src/utils/uuid'
|
||||
|
||||
import { evaluateMathExpression } from '@/lib/litegraph/src/utils/mathParser'
|
||||
|
||||
/**
|
||||
* The step value for numeric widgets.
|
||||
* Use {@link IWidgetOptions.step2} if available, otherwise fallback to
|
||||
@@ -12,17 +14,13 @@ export function getWidgetStep(options: IWidgetOptions<unknown>): number {
|
||||
}
|
||||
|
||||
export function evaluateInput(input: string): number | undefined {
|
||||
// Check if v is a valid equation or a number
|
||||
if (/^[\d\s.()*+/-]+$/.test(input)) {
|
||||
// Solve the equation if possible
|
||||
try {
|
||||
input = eval(input)
|
||||
} catch {
|
||||
// Ignore eval errors
|
||||
}
|
||||
const result = evaluateMathExpression(input)
|
||||
if (result !== undefined) {
|
||||
if (!isFinite(result)) return undefined
|
||||
return result
|
||||
}
|
||||
const newValue = Number(input)
|
||||
if (isNaN(newValue)) return undefined
|
||||
if (!isFinite(newValue)) return undefined
|
||||
return newValue
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user