Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

protect config.js from attempting to use invalid theme name (which corrupted mermaid use until reset()) #2987

Merged
merged 3 commits into from
May 2, 2022

Conversation

timmaffett
Copy link
Contributor

@timmaffett timmaffett commented Apr 30, 2022

This protects the use of invalid theme names the same way they are protected in MermaidAPI.js.

The situation that led to finding this bug was a mistyped:
%%{init: {"theme": "neutrl" }}%%

This led to the directive array getting corrupted with a theme:'neutrl' value, which in turn prevented all future use of the library entirely until reset() was called. (any attempt to call anything but reset() causes exception again)
Calls to setConfig() or initialize() also failed because the code in config.js would exception out because the invalid theme name was being use to index into theme[] and causing an exception.

This change simply protects from the use of invalid theme names:
(please refer to changes for more context)

if (sumOfDirectives.theme) {
becomes:
if (sumOfDirectives.theme && theme[sumOfDirectives.theme]) {

(and for good measure, but not where exception was)
if (conf.theme) {
becomes
if (conf.theme && theme[conf.theme]) {

again this is identical to how MermaidAPI.js protects against using invalid theme names.

Copy link
Member

@Yash-Singh1 Yash-Singh1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@knsv knsv merged commit 8583c7b into mermaid-js:develop May 2, 2022
@knsv
Copy link
Collaborator

knsv commented May 2, 2022

Thanks! A good approach!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants