プロジェクト

全般

プロフィール

操作
リンクをコピー

バグ #1259

完了

特定の URI 文字列を含むツイートを表示すると落ちる

Izumi Tsutsui さんが7年以上前に追加. 7年以上前に更新.

ステータス:
終了
優先度:
通常
担当者:
cob odo
対象バージョン:
3.7
プラグイン名:
ブランチ:
topic/1259-check-entity-url
クラッシュする:
はい

説明

以下のツイート
https://twitter.com/neta__/status/1003680831829766150
を mikutter で表示すると以下の現象になるようです。
  • ツイート表示が error 画像になる
  • その error 表示のツイート部分をクリックすると mikutter がクラッシュする or ハングする

クラレポは以下

Addressable::URI::InvalidURIError Invalid character in host: 'www\.apple'
/usr/pkg/lib/ruby/gems/2.4.0/gems/addressable-2.5.2/lib/addressable/uri.rb:2438:in `validate'
/usr/pkg/lib/ruby/gems/2.4.0/gems/addressable-2.5.2/lib/addressable/uri.rb:2357:in `defer_validation'
/usr/pkg/lib/ruby/gems/2.4.0/gems/addressable-2.5.2/lib/addressable/uri.rb:796:in `initialize'
/usr/pkg/lib/ruby/gems/2.4.0/gems/addressable-2.5.2/lib/addressable/uri.rb:136:in `new'
/usr/pkg/lib/ruby/gems/2.4.0/gems/addressable-2.5.2/lib/addressable/uri.rb:136:in `parse'
/usr/pkg/lib/ruby/gems/2.4.0/gems/diva-0.3.2/lib/diva/uri.rb:123:in `rescue in generate_uri_by_string'
/usr/pkg/lib/ruby/gems/2.4.0/gems/diva-0.3.2/lib/diva/uri.rb:120:in `generate_uri_by_string'
/usr/pkg/lib/ruby/gems/2.4.0/gems/diva-0.3.2/lib/diva/uri.rb:113:in `generate_uri'
/usr/pkg/lib/ruby/gems/2.4.0/gems/diva-0.3.2/lib/diva/uri.rb:75:in `to_uri'
/usr/pkg/lib/ruby/gems/2.4.0/gems/diva-0.3.2/lib/diva/uri.rb:104:in `method_missing'
{MIKUTTER_DIR}/core/plugin/search/model/search.rb:13:in `block in <class:Search>'
{MIKUTTER_DIR}/core/lib/diva_hacks/model.rb:100:in `block (2 levels) in handle'
/usr/pkg/lib/ruby/gems/2.4.0/gems/pluggaloid-1.1.1/lib/pluggaloid/filter.rb:28:in `filtering'
/usr/pkg/lib/ruby/gems/2.4.0/gems/pluggaloid-1.1.1/lib/pluggaloid/event.rb:59:in `block (2 levels) in filtering'
/usr/pkg/lib/ruby/gems/2.4.0/gems/pluggaloid-1.1.1/lib/pluggaloid/event.rb:58:in `each'
/usr/pkg/lib/ruby/gems/2.4.0/gems/pluggaloid-1.1.1/lib/pluggaloid/event.rb:58:in `reduce'
/usr/pkg/lib/ruby/gems/2.4.0/gems/pluggaloid-1.1.1/lib/pluggaloid/event.rb:58:in `block in filtering'
/usr/pkg/lib/ruby/gems/2.4.0/gems/pluggaloid-1.1.1/lib/pluggaloid/event.rb:57:in `catch'
/usr/pkg/lib/ruby/gems/2.4.0/gems/pluggaloid-1.1.1/lib/pluggaloid/event.rb:57:in `filtering'
/usr/pkg/lib/ruby/gems/2.4.0/gems/pluggaloid-1.1.1/lib/pluggaloid/plugin.rb:63:in `filtering'
{MIKUTTER_DIR}/core/mui/cairo_sub_parts_quote.rb:35:in `block (2 levels) in initialize'
{MIKUTTER_DIR}/core/mui/cairo_sub_parts_quote.rb:38:in `each'
{MIKUTTER_DIR}/core/mui/cairo_sub_parts_quote.rb:38:in `each'
{MIKUTTER_DIR}/core/mui/cairo_sub_parts_quote.rb:38:in `each'
{MIKUTTER_DIR}/core/mui/cairo_sub_parts_quote.rb:38:in `each'
{MIKUTTER_DIR}/core/mui/cairo_sub_parts_quote.rb:38:in `each'
{MIKUTTER_DIR}/core/mui/cairo_sub_parts_quote.rb:38:in `find'
{MIKUTTER_DIR}/core/mui/cairo_sub_parts_quote.rb:38:in `block in initialize'
{MIKUTTER_DIR}/core/mui/cairo_sub_parts_quote.rb:33:in `map'
{MIKUTTER_DIR}/core/mui/cairo_sub_parts_quote.rb:33:in `initialize'
{MIKUTTER_DIR}/core/mui/cairo_sub_parts_helper.rb:21:in `new'
{MIKUTTER_DIR}/core/mui/cairo_sub_parts_helper.rb:21:in `block in subparts'
{MIKUTTER_DIR}/core/mui/cairo_sub_parts_helper.rb:21:in `map'
{MIKUTTER_DIR}/core/mui/cairo_sub_parts_helper.rb:21:in `subparts'
{MIKUTTER_DIR}/core/lib/uithreadonly.rb:22:in `block (2 levels) in singleton class'
{MIKUTTER_DIR}/core/mui/cairo_sub_parts_helper.rb:41:in `_subparts_height'
{MIKUTTER_DIR}/core/mui/cairo_sub_parts_helper.rb:34:in `subparts_height'
{MIKUTTER_DIR}/core/lib/uithreadonly.rb:22:in `block (2 levels) in singleton class'
{MIKUTTER_DIR}/core/mui/cairo_coordinate_module.rb:48:in `height'
{MIKUTTER_DIR}/core/lib/uithreadonly.rb:22:in `block (2 levels) in singleton class'
{MIKUTTER_DIR}/core/mui/cairo_coordinate_module.rb:53:in `mainpart_height'
{MIKUTTER_DIR}/core/lib/uithreadonly.rb:22:in `block (2 levels) in singleton class'
{MIKUTTER_DIR}/core/mui/cairo_sub_parts_voter.rb:18:in `block in initialize'
{MIKUTTER_DIR}/core/mui/gtk_extension.rb:35:in `block in safety_signal_connect'
{MIKUTTER_DIR}/core/lib/uithreadonly.rb:22:in `signal_emit'
{MIKUTTER_DIR}/core/lib/uithreadonly.rb:22:in `block (2 levels) in singleton class'
{MIKUTTER_DIR}/core/mui/cairo_miracle_painter.rb:152:in `clicked'
{MIKUTTER_DIR}/core/lib/uithreadonly.rb:22:in `block (2 levels) in singleton class'
{MIKUTTER_DIR}/core/mui/cairo_cell_renderer_message.rb:160:in `block in event_hooks'
{MIKUTTER_DIR}/core/mui/gtk_extension.rb:35:in `block in safety_signal_connect'
{MIKUTTER_DIR}/core/mui/cairo_cell_renderer_message.rb:97:in `signal_emit'
{MIKUTTER_DIR}/core/mui/cairo_cell_renderer_message.rb:97:in `block in tree='
{MIKUTTER_DIR}/core/mui/gtk_extension.rb:35:in `block in safety_signal_connect'
{MIKUTTER_DIR}/core/plugin/gtk/mainloop.rb:10:in `main'
{MIKUTTER_DIR}/core/plugin/gtk/mainloop.rb:10:in `mainloop'
{MIKUTTER_DIR}/mikutter.rb:66:in `boot!'
{MIKUTTER_DIR}/mikutter.rb:102:in `<main>'

apple\.com という文字列が addressable の秘孔を突いている感じ?
addressable のバージョンは (pkgsrc 環境の) 2.5.2 です。

ファイル

neta__-1003680831829766150.png (33.5 KB) neta__-1003680831829766150.png Izumi Tsutsui, 2018-06-08 00:25
操作
リンクをコピー
 #1

cob odo さんが7年以上前に更新

  • ステータス を 分類待ち から 実装待ち に変更

クラッシュに至るまでの流れは以下の通り。

  1. 描画時、まず最初にscoreをつくる。Plugin[:twitter].score_by_entityが、tweetのjsonに含まれるentityを使ってscore（候補のひとつ）を構成する。
    • tweet[:entities][:urls]に「twitterがURLだと認識した」ものが入っている
    • URLを取り出してtweetテキストを置き換える範囲（Range）とHyperLinkNoteの組を作る
      • このとき、HyperLinkNoteのコンストラクタでURL文字列をDiva::URIオブジェクトにするが、この時点ではURLのパースは走らない。
    • ↑の組を使ってテキストを置き換え、scoreを構成していく。
  2. sub_parts_quoteが引用の処理をする。
    • scoreからHyperLinkNoteをすべて取り出してURLにする
    • URLに対応するカスタムModelがあるかmodel_of_uriフィルタで問い合わせる
    • searchプラグインのPlugin::Search::Searchモデルのhandleの判定文が走る
      • ↑の中でuri.hostの呼び出し
      • Diva::URI#method_missingによりDiva::URI#to_uriが呼ばれ、URI.parseが呼ばれて失敗し、Addressable::URI.newが呼ばれて失敗する
  3. Addressable::URI.newが投げた例外を捕捉できずクラッシュ

トレースはこの2～3の流れを示していますが、searchモデル以外のhandleでももちろん発生しますし、sub_parts_quote以外の処理（例えばクリック時のintent処理）でも発生するため、1の段階でチェックして対処するのがよさそうです。

Diva::URI.new()して.freezeすればその時点でパースさせられるので、エラーを検出したら（それは妥当なURLではないので）HyperLinkNoteを作るのをやめてTextNoteを作るようにすれば、クラッシュはしなくなります。

この例のtweet https://twitter.com/neta__/status/1003680831829766150 の場合、そういうコードに書き換えれば確かにクラッシュはしなくなるものの、

www\.apple
の部分がt.coで短縮されたものとして表示されることになります。これは、このリンク以外にentityが含まれておらず、scoreの生成時にTextNote3つで構成したものが却下されてしまうからです。つまり #1233 と同様です。

このチケットでは単にクラッシュを防ぐにとどめ、scoreの優先順位の問題は #1233 や #1241 で議論するのがいいかなと思います。
今夜あたりにパッチを書くつもりです。

操作
リンクをコピー
 #2

cob odo さんが7年以上前に更新

  • 担当者 を cob odo にセット
操作
リンクをコピー
 #3

toshi_a 初音 さんが7年以上前に更新

TwitterがURLでないものを送ってくる可能性について失念していました。t.coがあって本当に良かった。
修正はこの方針で良いと思います。

Diva::URIがURIの作成を遅延する理由は
  • Diva::Model#uri をModelごとにいちいち作っているとmikutterの動作がかなり遅くなった
  • ほとんどの場合、 Diva::Model#uri は同一性の判定に使われるのみなので、Stringの比較で良い
    • が、URIのフォーマットに従っている必要がある

と、どちらも Diva::Model#uri に使われるときのための配慮です。HyperLinkNote程度であれば、正格評価されても特に問題なさそうです。

これは、このリンク以外にentityが含まれておらず、scoreの生成時にTextNote3つで構成したものが却下されてしまうからです。つまり #1233 と同様です。
このチケットでは単にクラッシュを防ぐにとどめ、scoreの優先順位の問題は #1233 や #1241 で議論するのがいいかなと思います。

そうしましょう。

操作
リンクをコピー
 #4

cob odo さんが7年以上前に更新

  • ステータス を 実装待ち から レビュー待ち に変更
  • 担当者 を cob odo から Izumi Tsutsui に変更
  • ブランチ を topic/1259-check-entity-url にセット

HyperLinkNote程度であれば、正格評価されても特に問題なさそうです。

パフォーマンスの問題は失念していましたが、問題なさそうということでよかったです。実際動かしてみた限りでは問題なさそうでした。

topic/1259-check-entity-url ブランチに修正をpushしたので、試してもらえますか？＞つついさん

操作
リンクをコピー
 #5

Izumi Tsutsui さんが7年以上前に更新

テストしてみて、クラッシュしなくなるという点ではOKっぽいです。ありがとうございます。

操作
リンクをコピー
 #6

cob odo さんが7年以上前に更新

  • ステータス を マージ待ち から 終了 に変更

確認ありがとうございます。masterにmergeしたのでこのチケットは終了にします。

操作
リンクをコピー

他の形式にエクスポート: Atom PDF