Twitter::Text のバグ取り風景

Twitter::Text のバグ取りの風景をお伝えします。このバグは 2020/11/21 17:46 時点の最新バージョンである 0.07 では修正されています。

やりたいこと

この CPAN Testers report の環境 (win32, Perl 5.16.0) で Twitter::Text のテストが通るようにします。普段の開発環境はLinux (Ubnutu)で、Perlのバージョンは5.30.0なので、そこそこ違いがあります。

やったこと

JSON モジュールの最低バージョンを2.90まで上げる (効果なし、取り下げた)

JSONモジュールは、バージョン 2.90 と 4.00 (3.99_01) で非互換変更を行っています*1。reportを見ると、JSONのバージョンが2.53 (1回目の非互換変更が行われるより前) だったので、これが原因か？ とアタリをつけて、最低バージョンを2.90まで上げました。

これは結論から言うと全く関係なかったです。JSON 2.90での非互換変更は、 JSON::{true,false} が JSON::PP::Boolean のインスタンスになっていること・それらが eq をoverrideしてないこと*2でした。JSONモジュールは、ツイートのバリデーションのルールを決める設定ファイルを読み込むためだけに使っているので、ここで非互換変更があっても関係ないです。そのため、JSONのバージョン制限はいったん導入した後にすぐ取り下げました。

JSONのバージョンを制限すると、先述したCPAN Testers reportでは Twitter::Text が使えなくなる、ということになります。できるだけ制限しない方向でなんとか解決できないものか、と思って次の作戦を考えました。

正規表現の /o 修飾子を外す (効果なし)

Twitter::Text で使う正規表現は、本家twitter-textのRuby実装の正規表現を参考にして、Perlの正規表現に書き直しています。

このとき、正規表現の修飾子をほとんどそのまま持ってきていましたが、そこには /o という修飾子が含まれていました。perldoc perlre には、 /o 修飾子について以下のように書いています。

o - pretend to optimize your code, but actually introduce bugs

バグを引き起す、と書いてあるのを見て、これが原因ではないかと思いました。Twitter::Text で使っている正規表現から /o を取り除いてみました。

この段階で、reportの環境を手元に再現できたのでテストを走らせてみましたが、どうやら /o 修飾子も関係ないようです。バグを引き起す、と書いてあるので取り除くのは悪くないかもしれませんが、問題は解決していません。

2020/11/23 追記

Rubyの正規表現についていた /o 修飾子の意味は、正規表現中での #{} による式展開を一度だけ行う、というものでした*3。

Net::IDN::Encode モジュールの最低バージョンを2.100まで上げる (効果あり)

そもそもどういうテストが落ちているのか、じっくり観察することにしました。落ちているのは、長いURLやドメインは23文字としてカウントしない*4、ということを確かめるテストでした。このことから、URLを抽出する関数 extract_urls_with_indices の実装に不備があるのではないか、とアタリをつけます。

warn によるデバッグを駆使することで、どうやら extract_urls_with_indices 内で呼び出している _is_valid_domain という関数の実装がおかしそうだ、ということが分かりました。 _is_valid_domain は、名前の通り、与えられたドメイン名がvalidかどうかの真偽値を返す関数です。この関数の中に、以下のような処理があります。

my $encoded_domain = eval { domain_to_ascii($domain) };

if ($@) {
    return 0;
}

度重なる warn デバッグの結果、落ちているテストでは、与えられるURLがinvalidなのでここで return 0 されるべきだが、 return 0 されずにvalidなURLとしてチェックをすり抜けている、ということが分かりました。いよいよ、どこにバグがあるのかが特定できました！

domain_to_ascii は Net::IDN::Encode モジュールがexportしている関数です。eval ブロックで囲んでいることからも分かるように、この関数は、与えられたドメイン名がinvalidであれば例外を送出することが期待されています。

reportにある環境では Net::IDN::Encode 2.005 がインストールされており、どうやらこのバージョンだとテストが落ちるようです。また、Net::IDN::Encode 2.500が入っている環境ではテストが通ることが分かっています。したがって、2.005より大きく2.500以下の範囲で、Twitter::Text のテストが通る Net::IDN::Encode の最低バージョンを特定できればよさそうです。

Net::IDN::Encode のバージョンに対して、テストが通るかどうかで二分探索を行えば最低バージョンを決定できそうです。が、その前に他のreportで、2.500より低いバージョンのNet::IDN::Encodeでテストが通ってないかを見ることにしました。すると、win32, Perl 5.16.3, Net::IDN::Encode 2.100 でテストが通っているreportを発見できました。落ちていたのは2.005だったので、バージョンを1つ上げて2.100以上にすることを要求すればよさそうです。幸いにも、手元のWindows環境のPerl 5.16.0でも Net::IDN::Encode 2.100 をインストールすることができました。

github.com

原因

さて、このように問題を解決することができましたが、原因は何だったのでしょうか？

またもや warn デバッグをすることで、label too long という croakが呼ばれるべきだが呼ばれていない、ということが分かりました。そして、この croak は、Net::IDN::Encode 2.100 で追加されたものでした。Net::IDN::Encode のcommit logを見ると、preserve case in pure-ASCII labels (bypass en-/decoding) · cfaerber/Net-IDN-Encode@9fed007 · GitHub というcommitで croak されるようになっています*5。

label (そもそもこの時点ではlabelが何かも分かってなかった) が63文字より長いと例外になるが、63文字とはどこから来た数字なのか？ 答えは、RFC 1034 でした。ドメイン名を . で区切ったときの各サブドメイン名を表す文字列のことを label と呼び、各 label は最大63文字、と規定されています。つまり、64文字以上の label を含む場合はドメイン名としてはinvalidである、ということです。

おわりに

CPAN Testers reportがなければバグに気づくことはなかったと思います。環境を再現するのがひと苦労でしたが、再現してしまえば手元でいくらでも調査できるので、素早く環境を作るのがよさそうです。

Net::IDN::Encode::domain_to_ascii が例外を送出することを期待しているけど、呼び出し元でlabelの長さを見てバリデーションする、という手も取れるかもしれませんが、依存モジュールの最低バージョンを指定して直ることが分かったのでそのようにしました。PODにも、不正な入力に対しては例外を送出する、と書いてあるのでまあよいのではないでしょうか。

ドメイン名に関する知識がないままバグに直面して調査した結果、学びがあった、という記事でした。

*1:https://metacpan.org/changes/distribution/JSON

*2:https://metacpan.org/source/ISHIGAKI/JSON-4.02/Changes#L76-83

*3:https://ruby-doc.org/core-2.7.2/Regexp.html#class-Regexp-label-Options

*4:Twitterでは、URLは一律23文字としてカウントします。Counting characters | Docs | Twitter Developer

*5:しかしテストケースにはlabelが長すぎるときのテストがないように見える