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

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

Open
sapphi-red opened this issue May 29, 2023 · 8 comments · May be fixed by #4379
Open

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

sapphi-red opened this issue May 29, 2023 · 8 comments · May be fixed by #4379
Assignees
Labels
bug Something isn't working

Comments

@sapphi-red
Copy link
Contributor

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

になってリンクされる

@sapphi-red sapphi-red added the bug Something isn't working label May 29, 2023
@mehm8128
Copy link
Contributor

mehm8128 commented Jul 6, 2023

@TwoSquirrels
Copy link
Member

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

https://github.com/traPtitech/traQ/blob/ceebf98b3dc21f533923a5e3a7d417a8902b55dc/utils/message/replacer.go#L44-120

/**
* コードブロックとLaTeXブロック内でない箇所の内部リンク埋め込みを行う
* replacer.goのReplaceと同様のコード
*/
export const replace = (m: string, getters: Readonly<ReplaceGetters>) => {
let inCodeBlock = false
let inLatexBlock = false
let codeTokenLength = defaultCodeTokenLength
const lines = m.split('\n')
const newLines = lines.map(line => {
if (!inLatexBlock && line.startsWith('`'.repeat(codeTokenLength))) {
// `の数が一致するものと組み合うようにする
if (!inCodeBlock) {
codeTokenLength = countPrefix(line, backQuote)
} else {
codeTokenLength = defaultCodeTokenLength
}
inCodeBlock = !inCodeBlock
}
if (!inCodeBlock && line.startsWith('$$')) {
inLatexBlock = !inLatexBlock
}
if (inCodeBlock || inLatexBlock) {
return line
}
// 「```」のブロックでも「$$」ブロック内でもないときに置換
let newLine = ''
// 「`」「$」で囲まれていないところの始めの文字のindex
let noExpressionStartIndex = 0
const chs = [...line]
for (let i = 0; i < chs.length; i++) {
const ch = chs[i]
if (ch !== backQuote && ch !== dollar) {
continue
}
// 囲まれていない場所が終了したのでその箇所は置換する
newLine += replaceAll(
chs.slice(noExpressionStartIndex, i).join(''),
getters
)
if (ch === dollar) {
// 「`」は「$」よりも優先されるので
// 「$ ` $」のように「`」がペアの「$」より前にあるときは
// 「$」のペアとして処理しない
const backQuoteI = chs.indexOf(backQuote, i + 1)
const dollarI = chs.indexOf(dollar, i + 1)
if (backQuoteI !== -1 && dollarI !== -1 && backQuoteI < dollarI) {
newLine += ch
noExpressionStartIndex = i + 1
continue
}
}
const newI = chs.indexOf(ch, i + 1)
if (newI === -1) {
// 「$」/「`」のペアがないとき
newLine += ch
noExpressionStartIndex = i + 1
continue
}
newLine += chs.slice(i, newI).join('')
i = newI
noExpressionStartIndex = newI
}
// 最後のペア以降の置換
newLine += replaceAll(chs.slice(noExpressionStartIndex).join(''), getters)
return newLine
})
return newLines.join('\n')
}

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

@TwoSquirrels
Copy link
Member

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

@TwoSquirrels
Copy link
Member

TwoSquirrels commented Sep 30, 2024

現在 @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 TwoSquirrels self-assigned this Sep 30, 2024
@TwoSquirrels
Copy link
Member

TwoSquirrels commented Oct 1, 2024

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

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


A. 依存関係無しで実装

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

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


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

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

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


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

@TwoSquirrels
Copy link
Member

TwoSquirrels commented Oct 1, 2024

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

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

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

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

@TwoSquirrels
Copy link
Member

議論の traQ

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

@TwoSquirrels
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants