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.

Sign up for GitHub

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

Jump to bottom

Mermaid diagrams broken due to DOMPurify release v3.1.7 (empty <foreignObject) #5904

Closed
slorber opened this issue Sep 27, 2024 · 6 comments · Fixed by #5914 · 4 remaining pull requests
Closed

Mermaid diagrams broken due to DOMPurify release v3.1.7 (empty <foreignObject) #5904

slorber opened this issue Sep 27, 2024 · 6 comments · Fixed by #5914 · 4 remaining pull requests
Labels
Type: Bug / Error Something isn't working or is incorrect

Comments

@slorber
Copy link

slorber commented Sep 27, 2024

Description

Upgrade transitive dependency DOMPurify from 3.1.6 to 3.1.7

See flowchart diagram labels being empty, because <foreignObject> tags are empty

Steps to reproduce

Upgrade transitive dependency DOMPurify from 3.1.6 to 3.1.7

```mermaid
graph TD;
    A-->B;
    A-->C;
    B-->D;
    C-->D;
```

Screenshots

image

Code Sample

No response

Setup

  • Mermaid version: 10.x, 11.x
  • Browser and Version: all

Suggested Solutions

Convince DOMPurify to revert breaking change.

DOMPurify report here: cure53/DOMPurify#1002

Workaround, force resolution to v3.1.6

 "resolutions": {
    "dompurify": "3.1.6"
  },

Additional Context

I'm the maintainer of Docusaurus and this issue affects all our newly initialized sites. The problematic version was published 1 day ago and we already got 2 bug reports that Mermaid diagrams do not work well.

I'm opening this issue mostly to let you know of the problem.

I don't really know how Mermaid uses that lib exactly, nor if it's possible for you to patch usage on your side to make Mermaid code compatible with v3.1.7+. You could also decide to pin the dependency.

Personally I consider it a breaking change and they should rather revert it, but it's their responsibility to decide.
@slorber slorber added Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect labels Sep 27, 2024
This was referenced Sep 27, 2024
Closed
Closed
@imzbf imzbf mentioned this issue Sep 30, 2024
Closed
@nijinekoyo
Copy link

Is this issue being fixed? Pinning dependency versions is not a good idea :)

@slorber
Copy link
Author

slorber commented Sep 30, 2024

The maintainer of DOMPurify doesn't plan to revert the behavior so in the meantime the best option is to pin the dependency in Mermaid.

In the next version (v3.1.8) they plan to add an option that allows Mermaid to restore the previous behavior.

See discussions here: cure53/DOMPurify#1002 (comment)

So to me the plan for Mermaid should be:

  • pin to 3.1.6 until 3.1.8 gets released with the new option
  • then upgrade to ^3.1.8 and use the new option

aloisklink added a commit to aloisklink/mermaid that referenced this issue Oct 1, 2024
@aloisklink
fix: ban version v3.1.7 of DOMPurify
7dfeae3 
[DOMPurify v3.1.7][1] forbids the use of `<foreignElement>` for HTML
inside of an `<svg>` element, which breaks many mermaid diagrams.

It is likely that v3.1.8 will add a new option that will allow us to
re-enable this behaviour, but v3.1.7 definitely does not work.

[1]: https://github.com/cure53/DOMPurify/releases/tag/3.1.7

See: cure53/DOMPurify#1002
Fix: mermaid-js#5904
aloisklink added a commit to aloisklink/mermaid that referenced this issue Oct 1, 2024
@aloisklink
fix: ban version v3.1.7 of DOMPurify
de2c05c 
[DOMPurify v3.1.7][1] forbids the use of `<foreignElement>` for HTML
inside of an `<svg>` element, which breaks many mermaid diagrams.

It is likely that v3.1.8 will add a new option that will allow us to
re-enable this behaviour, but v3.1.7 definitely does not work.

[1]: https://github.com/cure53/DOMPurify/releases/tag/3.1.7

See: cure53/DOMPurify#1002
Fix: mermaid-js#5904
@aloisklink aloisklink mentioned this issue Oct 1, 2024
Merged
4 tasks
@aloisklink
Copy link
Member

The maintainer of DOMPurify doesn't plan to revert the behavior so in the meantime the best option is to pin the dependency in Mermaid.

🚀 Thanks for the help @slorber! I've made #5914 that will pin DOMPurify to:

mermaid/packages/mermaid/package.json

Line 80 in de2c05c

"dompurify": "^3.0.11 <3.1.7",

I'm guessing DOMPurify 3.1.8 will require some config/code changes, so we'd have to make a new Mermaid release that supports ^3.1.8 once it's out!

@aloisklink aloisklink removed the Status: Triage Needs to be verified, categorized, etc label Oct 1, 2024
@slorber
Copy link
Author

slorber commented Oct 1, 2024

Thanks!

Not sure what's your plan but it may be useful for the community to backport these changes to v10.x patch as well. Although technically v11 was quite easy to adopt for us we only allow 11.x on upcoming Docusaurus v3.6+ and not v2.0-3.5.

aloisklink added a commit that referenced this issue Oct 2, 2024
@aloisklink
[10] fix: ban version v3.1.7 of DOMPurify
402abdf 
[DOMPurify v3.1.7][1] forbids the use of `<foreignElement>` for HTML
inside of an `<svg>` element, which breaks many mermaid diagrams.

It is likely that v3.1.8 will add a new option that will allow us to
re-enable this behaviour, but v3.1.7 definitely does not work.

(cherry picked from commit de2c05c)

[1]: https://github.com/cure53/DOMPurify/releases/tag/3.1.7

See: cure53/DOMPurify#1002
Fix: #5904
@aloisklink
Copy link
Member

Thanks for bringing that up @slorber!

We've made v10.9.2 that has this change!

@slorber
Copy link
Author

slorber commented Oct 2, 2024

Thanks 👍 Docusaurus users will thank you without knowing any of this ever happened 😄

@ThatOneBro ThatOneBro mentioned this issue Oct 3, 2024
Closed
@WontonSam WontonSam mentioned this issue Oct 6, 2024
Open
@X-oss-byte X-oss-byte mentioned this issue Oct 27, 2024
Open
@rsaz rsaz mentioned this issue Oct 28, 2024
Open
@WontonSam WontonSam mentioned this issue Nov 1, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Type: Bug / Error Something isn't working or is incorrect
Projects
None yet
Milestone
No milestone
3 participants
@slorber @aloisklink @nijinekoyo