バグ #1233
closed
Twitterのメッセージに含まれる <>& がHTML実体参照のままになっている
Added by cob odo almost 7 years ago. Updated almost 6 years ago.
Description
表題の通り。
twitter_api_keysとdisplay_requirements以外のサードパーティプラグインが無い状態のconfで、master/developともに再現しました。
Files
| tsutsuii-995119161520013312.png (13.6 KB) tsutsuii-995119161520013312.png | Izumi Tsutsui, 2018-05-12 10:52 | ||
| tsutsuii-995119161520013312-web.png (23.7 KB) tsutsuii-995119161520013312-web.png | Izumi Tsutsui, 2018-05-12 10:56 | ||
| tsutsuii-995119161520013312-patched.png (14 KB) tsutsuii-995119161520013312-patched.png | Izumi Tsutsui, 2018-05-12 11:14 | ||
| スクリーンショット 2018-05-12 19.23.02.png (41.9 KB) スクリーンショット 2018-05-12 19.23.02.png | cob odo, 2018-05-12 19:23 | ||
| 1233-dummynote.patch (1.17 KB) 1233-dummynote.patch | cob odo, 2018-05-17 01:07 | ||
| tsutsuii-995119161520013312-dummynote.png (14.3 KB) tsutsuii-995119161520013312-dummynote.png | Izumi Tsutsui, 2018-07-22 09:46 | ||
| 0001-html-entity.patch (2.67 KB) 0001-html-entity.patch | cob odo, 2018-08-02 19:02 | ||
| 0001-copy_description_filter_dm.patch (3.75 KB) 0001-copy_description_filter_dm.patch | cob odo, 2018-08-03 18:51 |
Updated by toshi_a 初音 almost 7 years ago
mikutterのCKの凍結解除をお願いするためにフォールバック用のコンシューマキーをすべて手放したため完全にTwitterができません。
誰か実際に再現するかやってみてほしいです。
Updated by Izumi Tsutsui almost 7 years ago
- File tsutsuii-995119161520013312.png tsutsuii-995119161520013312.png added
- File tsutsuii-995119161520013312-web.png tsutsuii-995119161520013312-web.png added
いつもの NetBSD/i386 8.0_RC1 + pkgsrc mikutter 3.7.0 + 2ee74d6c の #1229 の修正の環境で
以下のように再現しますね。
https://twitter.com/tsutsuii/status/995119161520013312
Updated by Izumi Tsutsui almost 7 years ago
#1227 の c95d16b2 が原因なのかなあ、ということで
差分を見て野生の勘で以下のパッチを当てると直るっぽいです。
どこかの alias 修正漏れ?
diff --git a/core/plugin/twitter/model/message.rb b/core/plugin/twitter/model/message.rb
index 4f58e235..ed2061fb 100644
--- a/core/plugin/twitter/model/message.rb
+++ b/core/plugin/twitter/model/message.rb
@@ -660,6 +660,7 @@ class Plugin::Twitter::Message < Diva::Model
# 本文を人間に読みやすい文字列に変換する
def to_show
@to_show ||= body.gsub(DESCRIPTION_UNESCAPE_REGEXP, &DESCRIPTION_UNESCAPE_RULE).freeze end
+ alias_method :description, :to_show
# このMessageのパーマリンクを取得する
# ==== Return
Updated by toshi_a 初音 almost 7 years ago
Updated by Izumi Tsutsui almost 7 years ago
#1227 の本質をわかっていなかったりしますが、
認証のPINを入れたあとに「追加しますか」のウインドウが出なかった
ということがあって、それと同じ現象になるということでしょうか。
とりあえず twitter_api_keys を入れた状態で
Twitter world 削除→ mikutter 終了 → mikutter 再起動 → Twitterアカウントworld追加
を1回やったところでは正常に Twitter World 追加されましたが、
再現するしないの問題でもなさそうですね。
いずれにせよ #1227 では意図して
「description の挙動としてはエスケープ文字列を置き換えずに渡す」
ということであれば、 to_show を使うべきところで description が使われてしまっている
とかなんでしょうか。（深く考えずに書いてます）
Updated by cob odo almost 7 years ago
下から順に、
> https://mstdn.kanagu.info
> mstdn.kanagu.info
> mstdn
と入力したものです。
要するに、Entityが存在し、Entityの処理のために（Entityより前の部分を）TextNoteにする場合には、unescapeが効いたTextNoteが生成されますが、Entityが無い場合にはそういったことは起こらず、unescape処理が行われないために、こうなってしまうのだと思います。
恐らくこの場合のTextNoteはscore_by_scoreのelse節で生成されているものなのではないでしょうか？
Updated by Izumi Tsutsui almost 7 years ago
いろいろ試した結果のあまり役に立たないメモですが
source:core/plugin/twitter/twitter.rb の text_note に以下の notice を入れてみて、
diff --git a/core/plugin/twitter/twitter.rb b/core/plugin/twitter/twitter.rb
index 1cb0208d..938e19c5 100644
--- a/core/plugin/twitter/twitter.rb
+++ b/core/plugin/twitter/twitter.rb
@@ -492,6 +492,7 @@ Plugin.create(:twitter) do
# Plugin::Twitter::Message#descriptionの結果が実体参照をエスケープすると
# Entityのインデックスがずれるので、このメソッドで行う。
def text_note(description:)
+ notice description.gsub(Plugin::Twitter::Message::DESCRIPTION_UNESCAPE_REGEXP, &Plugin::Twitter::Message::DESCRIPTION_UNESCAPE_RULE).freeze
Diva::Model(:score_text).new(description: description.gsub(Plugin::Twitter::Message::DESCRIPTION_UNESCAPE_REGEXP, &Plugin::Twitter::Message::DESCRIPTION_UNESCAPE_RULE))
end
#1233#note-3 のツイートを show_tweet プラグイン
https://github.com/osak/show_tweet
で受信してみると、一応以下のように text_note は複数回呼ばれているようです。
warning: /usr/pkg/lib/ruby/2.4.0/rubygems/deprecate.rb:62:in `block (2 levels) in deprecate': NOTE: Skin.get is deprecated; use get_path instead. It will be removed on or after 2018-01-01.
Skin.get called from /home/tsutsui/.mikutter-test/plugin/show_tweet/show_tweet.rb:17.
notice: {MIKUTTER_DIR}/core/plugin/twitter/twitter.rb:495:in `text_note': <>&
<>&
という mikutter twitter プラグインのエスケープ問題再発問題
notice: {MIKUTTER_DIR}/core/plugin/twitter/twitter.rb:495:in `text_note': <>&
<>&
という mikutter twitter プラグインのエスケープ問題再発問題
notice: {MIKUTTER_DIR}/core/plugin/twitter/twitter.rb:495:in `text_note': <>&
<>&
という mikutter twitter プラグインのエスケープ問題再発問題
notice: {MIKUTTER_DIR}/core/plugin/twitter/twitter.rb:495:in `text_note': <>&
<>&
という mikutter twitter プラグインのエスケープ問題再発問題
notice: {MIKUTTER_DIR}/core/plugin/twitter/twitter.rb:495:in `text_note': <>&
<>&
という mikutter twitter プラグインのエスケープ問題再発問題
notice: {MIKUTTER_DIR}/core/plugin/twitter/twitter.rb:495:in `text_note': <>&
<>&
という mikutter twitter プラグインのエスケープ問題再発問題
notice: {MIKUTTER_DIR}/core/plugin/twitter/twitter.rb:495:in `text_note': <>&
<>&
という mikutter twitter プラグインのエスケープ問題再発問題
notice: {MIKUTTER_DIR}/core/plugin/twitter/twitter.rb:495:in `text_note': <>&
<>&
という mikutter twitter プラグインのエスケープ問題再発問題
notice: {MIKUTTER_DIR}/core/plugin/twitter/twitter.rb:495:in `text_note': <>&
<>&
という mikutter twitter プラグインのエスケープ問題再発問題
notice: {MIKUTTER_DIR}/core/plugin/twitter/twitter.rb:495:in `text_note': <>&
<>&
という mikutter twitter プラグインのエスケープ問題再発問題
notice: {MIKUTTER_DIR}/core/plugin/twitter/twitter.rb:495:in `text_note': <>&
<>&
という mikutter twitter プラグインのエスケープ問題再発問題
notice: {MIKUTTER_DIR}/core/plugin/twitter/twitter.rb:495:in `text_note': <>&
<>&
という mikutter twitter プラグインのエスケープ問題再発問題
notice: {MIKUTTER_DIR}/core/plugin/twitter/twitter.rb:495:in `text_note': <>&
<>&
という mikutter twitter プラグインのエスケープ問題再発問題
notice: {MIKUTTER_DIR}/core/plugin/twitter/twitter.rb:495:in `text_note': <>&
<>&
という mikutter twitter プラグインのエスケープ問題再発問題
notice: {MIKUTTER_DIR}/core/plugin/twitter/twitter.rb:495:in `text_note': <>&
<>&
という mikutter twitter プラグインのエスケープ問題再発問題
notice: {MIKUTTER_DIR}/core/plugin/twitter/twitter.rb:495:in `text_note': <>&
<>&
という mikutter twitter プラグインのエスケープ問題再発問題
notice: {MIKUTTER_DIR}/core/plugin/twitter/twitter.rb:495:in `text_note': <>&
<>&
という mikutter twitter プラグインのエスケープ問題再発問題
notice: {MIKUTTER_DIR}/core/plugin/twitter/twitter.rb:495:in `text_note': <>&
<>&
という mikutter twitter プラグインのエスケープ問題再発問題
notice: {MIKUTTER_DIR}/core/plugin/streaming/perma_streamer.rb:33:in `block in mainloop': PermaStreamer exit
ただ、どれがTL描画に使われているのかというのは把握できていません。
Updated by cob odo almost 7 years ago
- Related to 提案 #1241: Scoreの初期値をModelが提供できるようにする added
Updated by toshi_a 初音 almost 7 years ago
- Status changed from 分類待ち to 実装待ち
なるほど、確かに単一のText Noteだと、Scoreによる置き換えが発生しませんね。
#1227 を実装する時、実体参照解除機能をもたせたModelを検討したのですが、大げさになる為止めたということがありました。しかしよく考えれば、TextNoteを使うということは一切文字列を置換しないということを表明していることになるのですから、内容を変えているならTextNoteを使うべきではないですね。
Updated by cob odo almost 7 years ago
逆に、TextNoteを使わないということは他のプラグインによるScore置き換えを一切拒否するということになるので、それはそれで面白くないなーと思いました。
Updated by cob odo almost 7 years ago
- File 1233-dummynote.patch 1233-dummynote.patch added
サードパーティプラグインでのScore置き換えを許容し、かつ #1241 で提案した方法を除いて、Unescape済みTextNoteを採用させるには、先頭にdescriptionが空のNoteを置けばいいはずだ、ということで、例えば添付したパッチのような実装が考えられます。
Updated by toshi_a 初音 almost 7 years ago
これは思ってました。注記したときは勝手に再帰展開されるしこれでいけると思ってたんですが、再帰展開されるのは Diva::Model(:score_text) だけなので、確かに別の方法を考えないとですね。
#1233-11 のパッチはたしかに動きそうですが（Twitterの規約に従い、mikutter以外のCKを手放してしまってまたTwitterできなくなったため未確認です）、他にエレガントな方法があればそちらを採用したいですね。
他の解決策¶
実体参照を解除したdescriptionを持っているText Noteを仮に、UnescapedTextNoteと呼ぶことにします。
scoreルールがscoreを再帰的に問い合わせた結果を使う¶
私の当初の考えをブラッシュアップすると、Text Noteの代わりにUnescapedTextNoteを返すのではなく、UnescapedTextNoteを引数にscoreを得て、その内容を返すようにすれば全てのModelに適用できるScoreルールは適用されるようになります。
ただ、twitter_tweetではなくUnescapedTextNoteが対象となるため、twitter_tweetのみを対象にしたScoreルールが適用されません。
愚直な解決策としては、score_ofのオプション引数として、親となるModelとして別のものを渡せるようにすることです。しかしあまりにも解決方法が場当たり的すぎて、これも他に良い方法があれば避けたいところです。
TextNoteの要件を拡張¶
現在、score_ofがNoteを再帰展開するルールは、次のようなものです。
note.class.slug == :score_text
これが例えば次のようになっていれば、score_ofがUnescapedTextNoteを受け取った時に、Text Noteと同じように再帰展開が試みられます。
should_expand?(note)
もともと、Model slugを使うのはダックタイピングを阻害していて嫌だと思っていたので、副作用も少ないこの方法は私の中では今のところ有望です。
全てのNoteが再帰的に展開されるようにする¶
実はチケットにもしておらず、mikutter 3.8で実現しようと思っていた機能です。現在のScoreの仕組みでは、ハイパーリンクがNoteを持つことができないので、プレンーテキストにしかリンクを貼れないという問題があります。
具体的には、絵文字が含まれるURLは、twemojiプラグインで絵文字を展開できない（又は、twemojiプラグインのせいでURLが分断され、ハイパーリンクにならない）という問題があります。現状実装されているWorld系プラグインでこれが問題になることはまずないようですが。
この機能を実現するとしたら、全ての未知のNoteは必ず再帰展開を試みられることになるため、UnescapedTextNoteは特に工夫をしなくても #1233-10 の問題が発生しなくなります。
先延ばしにした理由は単純に解決できない問題があるからです。本筋から逸れるので程々にしますが、実装によってはScoreが単なるModelの配列ではなくなったり、そのせいでMiraclePainterがキャッシュしづらくなるという問題が起こると予想しています。
Updated by ncaq エヌユル almost 7 years ago
twitter.rbの295行のfilter_score_filterを弄ってdeadbeefのtext noteを返すようにしてみたのですが全く何も反映されませんでした…
プリントデバッグしてみたら呼び出されてはいるようなのですが
何に使われてるんでしょうこの処理…
ここのscore見てみるとちゃんとunescapeされてるんですよね
Updated by cob odo almost 7 years ago
- クラッシュする changed from No to Yes
ncaq net さんは書きました:
twitter.rbの295行のfilter_score_filterを弄ってdeadbeefのtext noteを返すようにしてみたのですが全く何も反映されませんでした…
プリントデバッグしてみたら呼び出されてはいるようなのですが
何に使われてるんでしょうこの処理…
ここのscore見てみるとちゃんとunescapeされてるんですよね
TextNoteのみで構成されるscoreは https://dev.mikutter.hachune.net/projects/mikutter/repository/revisions/acbbb1e2b5dd77f490da3b41ae8e41a92a60f846/entry/core/plugin/twitter/twitter.rb#L298 の条件で落ちます。
そこを書き換えても、 https://reference.mikutter.hachune.net/basis/2018/04/24/score.html の「Score選定アルゴリズム」により却下される場合があります。
score_filterがつくるscoreはあくまで提案で、複数提案されるscoreのどれが選ばれるかによって最終的な結果が変わるので、実行時にコードパスとして通っているからといって、必ずしもそれが結果に影響するわけではないんですね。
Updated by Izumi Tsutsui almost 7 years ago
master に対して #1233-11 の 1233-dummynote.patch を適用してみたところ、回避策としては問題ないようです。
toshi_a 初音 さんは書きました:
#1233-11 のパッチはたしかに動きそうですが（Twitterの規約に従い、mikutter以外のCKを手放してしまってまたTwitterできなくなったため未確認です）、他にエレガントな方法があればそちらを採用したいですね。
#1233-12 の本質対策に時間がかかるならば上記を先に適用しておくという選択肢もありかと思いますが
弊害はあるでしょうか。
ぼちぼち指摘が出ているようです。
https://twitter.com/ray45422/status/1020669432446660608
mikutterでhtmlでエスケープしないと表示できない文字がエスケープされたまま表示されてる
Updated by cob odo almost 7 years ago
弊害はあるでしょうか。
ない（つもり）です。
あるとすれば3.8向けのMiraclePainterリニューアル作業への影響がある、といった感じなんだと思います。
Updated by toshi_a 初音 almost 7 years ago
#1233-12 の本質対策に時間がかかるならば上記を先に適用しておくという選択肢もありかと思いますが
弊害はあるでしょうか。
#1233-11 の次善策は一見うまく動いていますが、このハックが同様の問題を抱えるサードパーティプラグインで横行すると、場合によっては将来のmikutterの実装が（ハックとの互換性を考えて）複雑なものになってしまうことを危惧していました。なので TextNoteの要件を拡張 が簡単に出来なかった場合に #1233-11 の適用をしようと考えていました。
（実はこの検証のために、OSC展示やテストのためにAkkiesoftから貰っていたTwitter APIキーを有効にしたところ標準プラグインのバグと思われるものを見つけたと思ったらPortalの問題だった、といったyak shavingをやっているところで、そうこうしているうちにTwitter APIに関する新しい発表があったりして少々焦っています）
Updated by cob odo over 6 years ago
- File 0001-html-entity.patch 0001-html-entity.patch added
別の問題を追っていたら、図らずもコレに対する修正ができたので、パッチをここに貼ってみます。
このパッチでは、以下の2つの修正をします。
Plugin::Twitter::Message#descriptionがHTML実体参照を解除したものを返すようにするPlugin[:twitter].score_by_entityがPlugin::Twitter::Message#descriptionではなくPlugin::Twitter::Message#bodyを使うようにする
この結果、以下の効果が得られます。
- 本文表示時にentityが無い場合にも実体参照が解除された状態になる（entityがある場合は #1227 で対処された方のコードを通るため、実体参照は解除されている）
- 「本文をコピー」時に実体参照が解除された状態になる
- extractプラグインによる
:popup_notify通知の本文パートで実体参照が解除された状態になる
Updated by cob odo over 6 years ago
:popup_notifyが使用するnotify-sendはむしろHTML実体参照でエスケープしなきゃいけないんですね。
http://www.galago-project.org/specs/notification/0.9/x161.html
さっきのパッチを使用するなら、送る直前に改めてエスケープするコードを追加する必要がありますね……
Updated by cob odo over 6 years ago
ちょっといくつか見てみましたが、メッセージモデルのdescriptionがHTML実体参照エスケープされているものを返すサードパーティプラグインは見当たりませんでした。
- Worldon： APIからHTMLで取得→descriptionはunescapeして返す
- Slack： APIから専用の記法で記述されたテキストで取得→Entity機構でユーザ名などを加工して表示するが、descriptionはそのまま。
- Haiku： APIからはてな記法で記述されたテキストで取得→そのまま返す
- RSS https://github.com/yukkurisinai/mikutter_rss ： リポジトリ消失？
- IRC https://bitbucket.org/Phenomer/mikutter-irc ： Divaより前の時代のプラグインですが、IRCなのでおそらくHTML実体参照は無い。
とすると、<>&などが含まれたメッセージの通知はTwitter以外ではそもそもマトモに動いてなかったんでしょうか？ Linuxデスクトップを使っていないのでわかりませんが……
動いていないとしたら別チケットを立てるべきですね。
Updated by cob odo over 6 years ago
DirectMessageで落ちたのでパッチに対策を入れました。
Updated by cob odo almost 6 years ago
- Status changed from 実装待ち to レビュー待ち
- Assignee set to Izumi Tsutsui
- Target version changed from 3.7 to 3.8
- プラグイン名 set to twitter
- ブランチ set to topic/1233-html-entities-in-twitter-description
レビューお願いします。
Updated by Izumi Tsutsui almost 6 years ago
- Status changed from レビュー待ち to マージ待ち
- Target version deleted (
3.8)
topic ブランチで検証しようとすると totori.dip.jp の gem でハマるという罠があるので
とりあえずで ffab1813 の差分を pkgsrc の mikutter にオレオレパッチで当ててテストしました。
show_tweet プラグインで
https://twitter.com/tsutsuii/status/995119161520013312
を表示させてみて、意図通り <>& が表示されました。
なので問題ないと思います。
Updated by Izumi Tsutsui almost 6 years ago
リロードしてもプルダウンメニューの選択が変わらない
（ので意図せず設定が戻ることがある）のは
Redmine の問題なのか Firefox の問題なのか……(´・ω・｀)