fix: Handle nested braces in expression parsing#28585
fix: Handle nested braces in expression parsing#28585sjz-ks wants to merge 1 commit inton8n-io:masterfrom
Conversation
fe10f24 to
7a7faa8
Compare
There was a problem hiding this comment.
No issues found across 10 files
Architecture diagram
sequenceDiagram
participant UI as User Interface
participant CM as CodeMirror (Editor)
participant ET as NEW: External Tokenizer
participant WE as Workflow Engine
participant EP as Expression Parser
participant JS as JS Sandbox
Note over UI, ET: UI Tokenization Flow (Editor Highlighting)
UI->>CM: Type expression (e.g., {{ {a:{}} }} )
CM->>ET: CHANGED: Request Resolvable token
ET->>ET: Initialize braceDepth = 0
loop Scan characters
alt Found {
ET->>ET: Increment braceDepth
else Found }
ET->>ET: Decrement braceDepth (if > 0)
else Found }} AND braceDepth == 0
ET-->>CM: Return Resolvable token
end
end
CM-->>UI: Render syntax highlighting
Note over WE, JS: Runtime Execution Flow
WE->>EP: splitExpression(text)
EP->>EP: Find next {{ marker
EP->>EP: NEW: findClosingExpressionIndex()
loop Scan until end of expression
alt NEW: Found escaped bracket (\}})
EP->>EP: Skip marker (preserve escape)
else NEW: Found nested braces { or }
EP->>EP: Update braceDepth state
else NEW: Found }} AND braceDepth == 0
EP->>EP: Identify valid closing index
end
end
EP-->>WE: Return ExpressionChunks[] (Code + Text)
WE->>WE: CHANGED: extendSyntax()
Note right of WE: Check if chunk includes nested }}<br/>to determine if extension is required
WE->>JS: evaluate(jsCode)
JS-->>WE: Return result (e.g., Object)
WE-->>UI: Display execution result
7a7faa8 to
f87ea70
Compare
There was a problem hiding this comment.
2 issues found across 12 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/workflow/src/extensions/expression-parser.ts">
<violation number="1" location="packages/workflow/src/extensions/expression-parser.ts:134">
P2: Malformed but closed expressions can now consume the rest of the template instead of ending at their real `}}`, swallowing later text/chunks.</violation>
</file>
<file name="packages/@n8n/codemirror-lang/src/expressions/expressions.grammar">
<violation number="1" location="packages/@n8n/codemirror-lang/src/expressions/expressions.grammar:9">
P2: Removing the marker tokens drops delimiter metadata and breaks downstream code that still expects `OpenMarker`/`CloseMarker` nodes.</violation>
</file>
Architecture diagram
sequenceDiagram
participant User as User / UI
participant CM as CodeMirror (@n8n/codemirror-lang)
participant E_Tok as External Tokenizer (Esprima-next)
participant Engine as Workflow Engine (packages/workflow)
participant Splitter as Expression Parser
participant Extender as Syntax Extender
participant Sandbox as JS Sandbox (IsolatedVM)
Note over User, E_Tok: Editor Feedback Loop (Frontend)
User->>CM: Input expression (e.g. {{ {a:{}} }} )
CM->>E_Tok: NEW: Scan for Resolvable token
loop For each candidate '}}'
E_Tok->>E_Tok: Extract code segment
E_Tok->>E_Tok: NEW: Check local scan cache
E_Tok->>E_Tok: NEW: Attempt parse (esprima-next)
alt Valid JavaScript
E_Tok-->>CM: Match found: Mark as Resolvable
else Invalid
E_Tok->>E_Tok: Continue scanning
end
end
CM-->>User: Highlighting / Syntax validation
Note over User, Sandbox: Runtime Execution Flow (Backend)
Engine->>Splitter: splitExpression(rawExpression)
loop Find Boundaries
Splitter->>Splitter: NEW: Find closing '}}' using parse-based detection
Splitter->>Splitter: NEW: Verify candidate via esprima-next
end
Splitter-->>Engine: List of Chunks (Text | Code)
Engine->>Extender: extendSyntax(bracketedExpression, forceExtend)
Extender->>Extender: CHANGED: Check cache using (expression + forceExtend) key
alt Cache Miss
Extender->>Extender: NEW: Normalize bare objects (add spacing for '}}')
Extender->>Extender: Perform AST transformations (e.g. .compact())
Extender->>Extender: Update Cache
end
Extender-->>Engine: Transformed JS Code
Engine->>Sandbox: Execute JS
Sandbox-->>Engine: Evaluation Result
Engine-->>User: Final Output
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| const expressionStart = index + res.index + 2; | ||
| const closingIndex = findClosingExpressionIndexByParsing(expression, expressionStart); | ||
|
|
||
| if (closingIndex === null) { |
There was a problem hiding this comment.
P2: Malformed but closed expressions can now consume the rest of the template instead of ending at their real }}, swallowing later text/chunks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/workflow/src/extensions/expression-parser.ts, line 134:
<comment>Malformed but closed expressions can now consume the rest of the template instead of ending at their real `}}`, swallowing later text/chunks.</comment>
<file context>
@@ -15,68 +17,136 @@ export interface ExpressionCode {
+ const expressionStart = index + res.index + 2;
+ const closingIndex = findClosingExpressionIndexByParsing(expression, expressionStart);
+
+ if (closingIndex === null) {
chunks.push({
type: 'code',
</file context>
There was a problem hiding this comment.
I don’t think this is a user-visible regression.
For malformed inputs like {{ a. }} tail {{ 2 }}, the splitter behavior does change internally, but the backend behavior was already an overall expression failure before this PR as well. So this is not a case where a previously valid template started producing a different successful result.
Handling this would mean adding fallback heuristics for invalid expressions on top of the new parser-based delimiter detection, which would expand the scope and risk re-breaking the valid cases this PR is fixing. I’d prefer not to extend this PR further in that direction.
| @@ -4,18 +4,10 @@ entity { Plaintext | Resolvable } | |||
|
|
|||
There was a problem hiding this comment.
P2: Removing the marker tokens drops delimiter metadata and breaks downstream code that still expects OpenMarker/CloseMarker nodes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/@n8n/codemirror-lang/src/expressions/expressions.grammar, line 9:
<comment>Removing the marker tokens drops delimiter metadata and breaks downstream code that still expects `OpenMarker`/`CloseMarker` nodes.</comment>
<file context>
@@ -4,18 +4,10 @@ entity { Plaintext | Resolvable }
- resolvableChar { unicodeChar | "}" ![}] | "\\}}" }
-
- unicodeChar { $[\u0000-\u007C] | $[\u007E-\u{10FFFF}] }
+@external tokens tokens from "./tokens" {
+ Resolvable
}
</file context>
There was a problem hiding this comment.
I don’t think this is a regression from this PR.
The downstream code in RuleMappingExpressionInput.vue only applies the bracket decoration when the syntax tree contains nodes named OpenMarker and CloseMarker. But that logic was already ineffective before this change, because the expression parser tree shape did not expose those as standalone nodes in practice — the consumer only ever saw Resolvable for {{ ... }}.
So this is an existing mismatch between the consumer’s traversal logic and the parser output, rather than a newly introduced breakage from this PR.
Summary
This PR fixes expression boundary detection for
{{ ... }}/={{ ... }}when the expression body itself contains valid JavaScript}}sequences, especially nested object literals.Examples now handled correctly include:
Before this change, the runtime splitter and CodeMirror tokenizer could stop at the wrong
}}, which either:invalid syntaxat execution time, orImplementation notes:
packages/workflownow finds the closing}}by testing candidate boundaries withesprima-next, instead of relying on brace-depth scanning.@n8n/codemirror-langnow uses the same parser-based candidate-close approach so runtime and editor stay aligned for valid expressions.\}}) is preserved.extendSyntax()now caches whole-expression results earlier, including no-op results, and separates cache entries byforceExtendScope of this fix is intentionally narrow:
}}Test coverage added:
}}}}forceExtend=falseandforceExtend=trueresults do not share the same whole-expression cache entryNote on the
object-extensionstest update:One existing
object-extensionsassertion previously depended on the old parser truncating the expression and throwinginvalid syntax.After fixing expression parsing, that assertion no longer reflected the underlying safety property being tested. This PR updates the test to assert the original intent directly: the expression evaluates without prototype pollution and
({}).pollutedremainsundefined.How to test:
Run package-level checks:
pushd packages/@n8n/codemirror-lang && pnpm lint && pnpm typecheck && pnpm build && pnpm test && popdpushd packages/workflow && pnpm lint && pnpm typecheck && pnpm build > build.log 2>&1 && popdRun targeted workflow regressions:
pushd packages/workflow && pnpm exec vitest run --project legacy-engine test/ExpressionExtensions/expression-extension.test.ts test/expression.test.ts test/ExpressionExtensions/object-extensions.test.ts && popdOptional manual UI verification:
HTTP RequestJSON body expression input, verify{{ {values:{}} }}is accepted and behaves the same as{{ {values:{} } }}Before:


After:

Validation notes:
packages/@n8n/codemirror-lang:lint,typecheck,build,testpassedpackages/workflow:lint,typecheck,buildpassedpackages/workflowtargeted regressions passedpackages/frontend/editor-uiexpression-related confidence tests passedpnpm test:affectedonly retains the existinglegacy-engine/vm-enginefailures inpackages/workflow/test/ExpressionExtensions/date-extensions.test.ts, which are already reproducible onupstream/masterRelated Linear tickets, Github issues, and Community forum posts
Review / Merge checklist
fix: Handle nested braces in expression parsing)Backport to Beta,Backport to Stable, orBackport to v1(if the PR is an urgent fix that needs to be backported)