traPtitech / traQ_S-UI

traQ S - traP Internal Messenger Application Frontend
MIT License
108 stars 40 forks source link

URL内にチャンネルの表記が入ると意図しないリンクが生成される #3978

Open sapphi-red opened 1 year ago

sapphi-red commented 1 year ago
https://docs.python.org/ja/3.9/library/random.html#random.sample

https://docs.python.org/ja/3.9/library/random.html!{"type":"channel","raw":"#random","id":"uuid}.sample

になってリンクされる

mehm8128 commented 1 year ago

https://q.trap.jp/messages/898973e6-b677-4dab-8254-3a15f1bdb9a5 メンションも

TwoSquirrels commented 1 month ago

チャンネルとメンションの置換処理はバックとフロント両方にあるので、両方弄らないとだめそう?

https://github.com/traPtitech/traQ/blob/ceebf98b3dc21f533923a5e3a7d417a8902b55dc/utils/message/replacer.go#L44-120 https://github.com/traPtitech/traQ_S-UI/blob/d55e4bc6115d2188db0e9f432d4831dec6da0a73/src/lib/markdown/internalLinkEmbedder.ts#L36-L109

「コードブロックとLaTeXブロック内でない箇所」この条件に URL 内でないも加えればよさそう。

TwoSquirrels commented 1 month ago

URL の検出ロジックは @traptitech/traq-markdown-itmarkdown-itlinkify-it と辿れたので、@traptitech/traq-markdown-it からどうにか linkify を使えればいいかなと思ったが、 バックも同じロジックにする必要があるので依存関係無しで実装すべきやつかこれ?それなら linkify-it の内部実装を Go で再現する必要がありそうかな

TwoSquirrels commented 1 month ago

現在 @traptitech/traq-markdown-it は TLD 追加してる以外 linkify-it はデフォルト設定で使ってるので、依存関係無しで再現するなら linkify-it のソースコードだけ見てどうにかすればよさそうだけど……

https://github.com/traPtitech/traq-markdown-it/blob/67f68d03f3b1430812ccf67d5a1ec5702a6aa51d/src/traQMarkdownIt.ts#L90

    this.md.linkify.tlds(['app', 'dev', 'games', 'tech', 'show'], true)

どうするのが最善なのかな

TwoSquirrels commented 1 month ago

フロントのコードはユーザー用で、バックのコードは BOT 用でオプションが有効の時だけ置換されるやつらしい。 (そもそも同じロジックが 2 言語で書かれていることに問題があるという意見もある)

まとめると僕は URL の検出に 2 つの実装方針を考えていて、


A. 依存関係無しで実装

別言語であるバックとフロントを同じロジックにするために、traQ のフロントが内部で使っているマークダウンパーサーの実装をある程度再現する。ライブラリに依存しないことで二言語で同じロジックにしやすい。

メリット: ユーザーと BOT で置換のロジックが揃う デメリット: 実装が大変、MD の URL の検出ロジックと少し変わる


B. ロジックを揃えないでそれぞれ実装

フロントは traQ のマークダウンパーサーで使われている linkify-it を呼ぶ形で実装し、バックは実装しないか A のような再現を行うか適当なライブラリを使う。

メリット: 実装が楽、ユーザーのみ MD の URL の検出ロジックと一致する デメリット: ユーザーと BOT で置換のロジックが異なる


つまり、BOT とユーザーを合わせるのが A、ユーザーと MD を合わせるのが B って感じかな。 どっちもロジックが重い場合オーバーヘッドが怖いかもってのがあるけどどうなんでしょう?

TwoSquirrels commented 1 month ago

C. フロントの実装を消し、ユーザーのもバックにさせる

(そもそも同じロジックが 2 言語で書かれていることに問題があるという意見もある)

ので、フロントの置換ロジックを消してしまって、常にバックに任せるとよさそう。バックは linkify-it の実装を参考にがんばるか、適当なライブラリを使う。

メリット: ライブラリなら実装が楽、ユーザーと BOT で URL の検出ロジックが一致する、同じロジックが複数存在しなくなる デメリット: MD の URL の検出ロジックと少し変わる

TwoSquirrels commented 1 month ago

議論の traQ

C がよさそうかな~という感じになってる。ただフロントでの置換結果のテキストはそのまま送信する前に長さチェックに使われているので、そこを少し書き換える必要がありそうという感じに

TwoSquirrels commented 1 month ago

フロントは置換 → 長さチェックの順だけど、バックは長さチェック → 置換らしいので、そこもバックに合わせるならフロントの長さチェックは置換無しのでそのままやっちゃえばよさそう