Skip to content

Remove legacy new Function() fallback from evalFormula + add formula migration #42

Description

@zntznt

Problem

evalFormula (js/model.js:157-179) falls back to new Function() when math.js cannot evaluate an expression. After item #41 hardens validateFormula, the validation gate is closed — but the execution path remains. An old diagram loaded from autosave, shared URL, or library can still execute arbitrary JS through the legacy fallback. This is the actual injection surface.

Dependencies

Plan

Step 1 — Strip evalFormula to math.js-only

Remove lines 161-179 (legacy new Function() path). After removal:

math.js success → return result
math.js failure → return 0
math.js unavailable → one-shot console.warn, return 0

Also remove _seededMath (line 131) — it was only referenced from the legacy path at line 173.

Step 2 — Add migrateFormula(expr)

A pure string→string function. Converts legacy JS patterns to math.js equivalents using \b word-boundary regex. Guards non-strings: if (typeof expr !== 'string') return expr;.

Migration table:

Legacy math.js
Math.round(X) round(X)
Math.floor(X) floor(X)
Math.ceil(X) ceil(X)
Math.abs(X) abs(X)
Math.max(A,B) max(A,B)
Math.min(A,B) min(A,B)
Math.sqrt(X) sqrt(X)
Math.log(X) log(X)
Math.exp(X) exp(X)
Math.pow(A,B) pow(A,B)
Math.random() random()
Math.PI pi
Math.E e
&& and
|| or

Idempotent: migrateFormula(migrateFormula(s)) === migrateFormula(s).

Step 3 — Call migration in loadJSON methods

In MNode.loadJSON (line 502): after Object.assign, migrate this.formula.

In MConnection.loadJSON (line 649): after Object.assign, migrate this.formula, this.weightFormula, this.modFormula.

In Diagram.loadJSON (line 816): after loading, migrate customVars[].formula and assertions[] only (nodes/connections are already handled by their own loadJSON).

This covers all entry points: file load, autosave, shared URL, library, clipboard paste, undo/redo, Monte Carlo clones.

Step 4 — Update tests in test/run.js

Remove:

  • Lines 1536-1543: 'legacy-fallback formulas (Math.random) are seeded as well' — legacy path no longer exists
  • Lines 2413-2414: 'legacy JS-syntax formulas still evaluate (fallback path)' — legacy path no longer exists

Add:

  • Migration conversion tests (all patterns)
  • Migration idempotency test
  • Migrated formulas evaluate correctly via math.js
  • evalFormula returns 0 for unevaluable expressions
  • Diagram loadJSON migrates legacy formulas end-to-end

Step 5 — Verify

npm install          # mathjs required
npm test             # 229 tests
npm run smoke        # Playwright browser test

Side effect: fixes test #3182

The 'negative-slope rate formulas probe as balancing feedback' test currently fails because max() requires math.js and the legacy path cannot evaluate it. After removing the legacy path, math.js is always used (when available), so _loopProbeSign correctly detects the negative slope. This test passes with mathjs installed.

Effort

~1.5 hours. 2 files changed (js/model.js, test/run.js).

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions