fix(sequence): prevent hang when "as" lacks trailing space#7437
Conversation
The sequence diagram JISON parser had no catch-all rule in the exclusive ID lexer state. When input like `participant X asAlias:text` was parsed (missing space after "as"), none of the ID-state rules could match due to the colon terminating the character class match before end-of-line. This caused a lexer error that could hang browsers depending on error recovery behavior. Add a catch-all `<ID>[^\n]+` rule that produces an INVALID token when no other ID-state rule matches, ensuring the parser always makes progress and produces a clean parse error. Resolves #6399 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🦋 Changeset detectedLatest commit: 1f1484a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
the
Type: Bug / Error
@mermaid-js/examples
mermaid
@mermaid-js/layout-elk
@mermaid-js/layout-tidy-tree
@mermaid-js/mermaid-zenuml
@mermaid-js/parser
@mermaid-js/tiny
commit: |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #7437 +/- ##
==========================================
- Coverage 3.54% 3.54% -0.01%
==========================================
Files 490 491 +1
Lines 48938 48949 +11
Branches 766 766
==========================================
Hits 1734 1734
- Misses 47204 47215 +11
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
knsv
left a comment
knsv
left a comment
There was a problem hiding this comment.
[sisyphus-bot]
Review: fix(sequence): prevent hang when "as" lacks trailing space
Nice work, @ashishjain0512 — this is a clean, targeted fix for a real DoS-class bug. Since this is still a draft, keeping feedback to architecture-level observations only.
What's working well
🎉 [praise] Excellent root-cause analysis in the PR description. The explanation of why each existing <ID> rule fails to match asAAAAAAAAAAAAA:AAAAAAAAAAAAA (the colon terminates the character class) is clear and correct. This kind of documentation makes reviews and future debugging much easier.
🎉 [praise] The catch-all rule <ID>[^\n]+ is correctly positioned as the last rule in the <ID> state (after the specific matchers at lines 35-38). JISON tries rules in order, so the catch-all only fires when no specific rule matches — no risk of accidentally consuming valid input.
🎉 [praise] The popState() in the catch-all is essential and correct — it returns from the ID exclusive state, preventing the lexer from getting permanently stuck. The INVALID token then triggers a clean parse error downstream.
🎉 [praise] Good test design: the 5-second timeout catches the hang, expect(errorMessage).not.toBe('') confirms an error is raised, and expect(errorMessage).not.toContain('Lexical error') confirms it's a parse error (not a lexer error). TDD cycle documented in the PR body.
Security
No security concerns. This is a defensive parser change that converts an infinite-loop hang (DoS) into a clean parse error. No rendering code, DOM sinks, or sanitization paths are affected.
Architecture direction
The fix looks solid. A couple of observations for when you're ready to finalize:
💡 [suggestion] — The <ID> state catch-all pattern is a good defensive practice. Other exclusive states in this lexer (e.g., <ALIAS>, <LINE>, <CONFIG>) may have similar hang risks if they encounter unexpected input. Not something for this PR, but could be worth a follow-up audit.
Self-check
- At least one 🎉 [praise] item exists
- No duplicate comments
- Severity tally: 0 🔴 / 0 🟡 / 0 🟢 / 1 💡 / 4 🎉
- Verdict matches criteria: COMMENT (draft PR — no REQUEST_CHANGES regardless)
- Draft PR: COMMENT only ✓
- No inline comments used
- Tone check: encouraging and constructive
2 participants
Summary\n\nResolves #6399\n\n- Add catch-all
<ID>[^\\n]+rule in the sequence diagram JISON lexer to handle unmatched input in the exclusive ID state\n- Whenparticipant X_AutoPublishable asAAAAAAAAAAAAA:AAAAAAAAAAAAAis parsed (missing space after "as"), the existing ID-state rules all fail to match because the colon terminates the character class before end-of-line. Without a catch-all, this left the lexer with no token to produce, causing browsers to hang.\n- The new rule produces an INVALID token, allowing the parser to emit a clean parse error instead of hanging\n\n## Classification\n\n- Change type: Parser (JISON grammar)\n- Breaking change: No — malformed input that previously hung now produces a parse error\n- Shared code touched: No\n\n## Verification\n\n- [x] TDD: test failed before fix ("Lexical error"), passes after (clean parse error)\n- [x] Lint: passed\n- [x] Unit tests: 142/142 passed\n- [ ] Visual spot-check: N/A — parser-only change\n- [ ] Full e2e: Not run — parser-only change\n- [x] Changeset: generated (patch)\n\n🤖 Generated with Claude Code