Skip to content

Fix _evalCond silent-true on unknown operator + add model-layer operator validation #43

Description

@zntznt

Problem

_evalCond (js/engine.js:1319) returns default: return true for unknown operators. If an operator is corrupted (empty string, typo, null), conditions silently pass. This affects 5 call sites — the most dangerous being _checkEnd (simulation stops immediately if endOperator is corrupted, or silently runs forever if the default is changed naively).

Plan

Primary fix — model-layer operator validation in loadJSON

Add _normalizeOp helper in js/model.js:

const VALID_OPS = new Set(['>', '>=', '<', '<=', '==', '!=', 'between']);
function _normalizeOp(op, fallback) {
  return (op && VALID_OPS.has(op)) ? op : fallback;
}

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

this.endOperator = _normalizeOp(this.endOperator, '>=');

In MConnection.loadJSON (line 649, after Object.assign):

this.condOperator = _normalizeOp(this.condOperator, '>');
this.actOperator = _normalizeOp(this.actOperator, '>=');

This catches bad operators at the serialization entry point, before they reach the engine. Covers file load, autosave, shared URL, library, paste, undo/redo, Monte Carlo clones, and DSL parsing — all go through loadJSON.

Secondary fix — engine-level safety net

Change _evalCond default from return true to return false + console.warn. With model-layer validation, this path is never reached in normal operation — it's defense-in-depth.

Tests

  • loadJSON normalizes unknown/null/empty operators to defaults
  • _evalCond returns false for unknown operators
  • Known operators still work

What's NOT covered (low risk)

AI player rule.condOp (engine.js:1272) doesn't pass through loadJSON normalization. However: (1) the UI <select> constrains to valid operators, (2) the engine guard rule.condOp || '>=' catches falsy values, and (3) the revised return false + console.warn in _evalCond would catch truthy typos visibly.

Effort

~15 minutes. 3 files: js/model.js (+10 lines), js/engine.js (1 line), test/run.js (+10 lines).

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