-
Notifications
You must be signed in to change notification settings - Fork 77
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
feat: mermaid.js 対応 #150
feat: mermaid.js 対応 #150
Conversation
@catnose99 お時間あるときに以下2点、見解を教えていただけると嬉しいです
|
TODO: |
packages/example/articles/embed.md
Outdated
@@ -23,3 +23,192 @@ https://gist.github.com/mattpodwysocki/218388 | |||
@[gist](https://gist.github.com/hofmannsven/9164408?file=my.cnf) | |||
|
|||
ssfafafaffafa | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[note] サンプルを記載していますがこんなに数がいらなければ言ってください。
packages/example/articles/embed.md
Outdated
N-->O | ||
O-->P | ||
P-->ID1[ノード1<br>ノード2] | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
packages/example/articles/embed.md
Outdated
one --> two | ||
three --> two | ||
two --> c2 | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[note] フローチャートの例
packages/example/articles/embed.md
Outdated
光輝-->>アリス: Great! | ||
光輝->>Bob: How about you? | ||
Bob-->>光輝: Jolly good! | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
packages/example/articles/embed.md
Outdated
+bool is_wild | ||
+run() | ||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[note]
- クラス図の例
packages/example/articles/embed.md
Outdated
ScrollLockOn --> ScrollLockOff : EvScrollLockPressed | ||
|
||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[note] 状態遷移図の例
packages/example/articles/embed.md
Outdated
A-->B & C-->D & E-->F & Z-->X; | ||
A-->B & C-->D & E-->F & Z-->X; | ||
A-->B & C-->D & E-->F & Z-->X; | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
packages/example/articles/embed.md
Outdated
click alert`md5_salt` eval "Tooltip for a callback" | ||
click B "javascript:alert('XSS')" "This is a tooltip for a link" | ||
link Zebra "http://www.github.com" "This is a link" | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[note]
- mermaid 側のセキュリティ対策により
link Zebra "http://www.github.com" "This is a link"
は文法エラーになる例
packages/example/articles/embed.md
Outdated
alert`md5_salt`-->B; | ||
click alert`md5_salt` eval "Tooltip for a callback" | ||
click B "javascript:alert('XSS')" "This is a tooltip for a link" | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[note]
- prevent xss な例
- 基本的には
escapeHTML
済みのコードを mermaidAPI がレンダリングすることになるのでXSSは最後は mermaid 側にゆだねられます
yes: (source.match(/&/g) || []).length > MAX_CHAINING_OF_LINKS_LIMIT, | ||
message: `<li>ブロックあたりの&によるチェイン上限は${MAX_CHAINING_OF_LINKS_LIMIT}です</li>`, | ||
}, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[note] エラー時の表示方法を相談したいです。変わりのHTMLを表示するのは GitLab のマネです。
@@ -2,7 +2,7 @@ module.exports = { | |||
globals: { | |||
'ts-jest': { | |||
// avoid "jsx" treated as "preserved" | |||
tsConfig: 'tsconfig.json', | |||
tsconfig: 'tsconfig.json', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[note] warning がコンソールにでてたのでついでに
} | ||
return defaultRender(tokens, idx, options, env, slf); | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[note] fence
ルールへ追加することになります。<embed-mermaid>
を embed ライブラリが探し当てます。pre
タグにclass="mermaid"
としてしまうと mermaid.js を読み込んだ瞬間にレンダリングが走ってしまうのでそれぞ防ぐためにzenn-mermaid
としています。
@catnose99 もろもろ対応しまして、月曜日以降レビューいただければと思います:bow: |
click B "javascript:alert('XSS')" "This is a tooltip for a link" | ||
link Zebra "http://www.github.com" "This is a link" | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[note]
- サンプルを書く先のファイル(embed.md or example.md)
- どのくらい例を書くか
アドバイスいただければと:bow:
mermaid!.mermaidAPI.parse(source); | ||
return true; | ||
} catch (e) { | ||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
「レンダリングがなぜか上手くいかない」というケースでの原因究明のため、e
を一応consoleに出力するようにしますか?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
こちら承知しました。
@catnose99 出力するよう修正しました。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
いい感じです!ありがとうございます!
resolve: zenn-dev/zenn-community#299
SVG が生成されることから、変換負荷やキャッシュ量を考慮します。markdownToHtml では一次変換し、embed で埋め込み表示する方針です。
以下のようなイメージ
なぜやりたいか
Zenn における記事の新しい埋め込み・レンダリング対応は、対象ライブラリやツールが以下をすべて満たす場合に対応するという方針を考えています。
いくつか開発チームの主観的な指標もありますが、定量評価のみだとハックされる恐れがあるためです。
mermaid.js
は、記事にグラフやシーケンス図の図示を可能とし、ITシステムにおける上流工程・設計工程の知見や、機器同士のシーケンスなどを表現することを助けてくれるはずです。レンダリング方針
当初より大きくは変えておらず、
markdownToHtml
では<embed-mermaid>
へ変換し、zenn-embed-elements
で CDN から mermaid.js をブラウザ側で読み込んだ上で、mermaidAPI
という ブラウザ側でレンダリングできる 関数 を利用しています。これによりsvgコードによるキャッシュの肥大化を防ぎつつも、読み込んだ瞬間に勝手にレンダリングはさせずこちらのバリデーションを通した上でレンダリングすることで、セキュリティリスクを最小限に押さえています。セキュリティリスクへの考え方
利用するバージョンを現時点で最新の
8.10.x
に固定します。XSS
過去、XSSが存在し、mermaid.js側で対応した実績がありました。
8.2.3で修正されました。
基本的には Markdown をレンダリングする際に
escapeHtml
し、エスケープした文字列をmermaidAPI.render()
へ渡すことになります。なので最後にはセキュリティ対策はライブラリ側に委ねられており、私達としてはリスクの早期発見と無効化を行う必要があるという認識です。Interaction
mermaid は interactiion 機能を提供しており、クリックイベントに対して図示を変えるといった表現力があります。しかしクリックイベントはセキュリティリスクをはらんでいると考えており、以下の措置をとっています。
securityLebel
を最高のstrict
とする(GitLabも同様)。このレベルはクリックイベントが無効化される「Interactionは使えないよ」ということはユーザーマニュアルにも記載するつもりです。
相談事項
レビューいただいたいのですが(お忙しいところすいません)、中でもバリデーションエラー発生時の表示をご確認いただきたく。現状は図の代わりに
<ul><li>
でこんな感じの表示にしています。