If you compare two numerical strings, they are compared as integers

  • 元タイトル: recent プラグインを利用した時の挙動について
  • ページ: BugTrack2
  • 投稿者: 名無しさん
  • 優先順位: 低
  • 状態: 完了
  • カテゴリー: 本体バグ
  • 投稿日: 2007-05-17 (木) 19:58:23
  • バージョン: -1.4.7

概要

"1", "01", "001" のように、数値の羅列に見えるページ名で、数値としては同一であるように見えるページが複数あった場合、これらが全て同一であるとみなされてしまっています。

これはPHPの文字列の比較に関する仕様を考慮できていないのが原因です。two numerical strings (数字の羅列に見える文字列型の値二つ) を比較する時は、 == ではなく === 演算子を使って比較しなければ(意図した結果にはならず) それぞれが 数値として 比較されてしまいます。

関連

メッセージ

メニューバーでrecent プラグインを利用した時、今見ているページ以外のページまでリンクされなくなってしまいます。
この現象が発生するのは、整数のみで構成されたページ名で以下のような場合です。

  • 1、01、001、など、頭の0を省けば同じ数字になってしまうページ名が、recent プラグインで同時に表示されている時。下は適当な(お試しサイトで実験した)イメージ。
    最新の20件
    2007-05-17 
     001    <= 例えばこれを表示しているとき
     020 
     20 
     01     <= これと
     1      <= これもリンクされなくなってしまう
     00 
     0 
    この例で、「1」というページを今見ているとすると、「01」と、「001」というページもリンク対象から外れてしまいます。


BugTrack2/235 の修正をする前の環境(1.4.7_notb)で試しても同じ結果になったので、原因は別の場所にあるようです。


  • すいません、原因はPHP マニュアルの「[[ここ>http://www.php.net/manual/ja/language.operators.comparison.php]]」に書いてありました。下は該当部分の抜粋。
    整数値を文字列と比較する際、文字列が 数値に変換されます。
    数値形式の文字列を比較する場合、それは整数として比較されます。
    後半部分が、今回recent プラグインで該当しているようです。
    // $Id: recent.inc.php,v 1.24 2006/09/30 02:18:23 henoheno Exp $
    (中略)
    function plugin_recent_convert()
    {
    (中略)
    		if($page == $vars['page']) {
    			// No need to link to the page you just read, or notify where you just read
    			$items .= ' <li>' . $s_page . '</li>' . "\n";
    		} else {
    			$r_page = rawurlencode($page);
    			$passage = $show_passage ? ' ' . get_passage($time) : '';
    			$items .= ' <li><a href="' . $script . '?' . $r_page . '"' . 
    				' title="' . $s_page . $passage . '">' . $s_page . '</a></li>' . "\n";
    		}
    (以降、省略)
    上で、$page がリストに載っているページ名、$vars['page'] は今見ているページ名です。
    if で比較する時に、「1」も「01」も「1」であると解釈されるので、今回のような問題が起こるようです。
    もしこれを回避するのならば、if 文の条件をpreg_match で書く必要があると思います。 -- 2007-05-17 (木) 21:36:06
  • 比較演算子を==ではなく===にしたらどうでしょうか。 -- ぃぉぃぉ 2007-05-17 (木) 21:48:03
  • お知らせありがとうございます。ご指摘の通りで、ぃぉぃぉさんの対処が正解のようです。これは私も意識していませんでした。同様のケースは他にもありそうですね・・・ (^^; マニュアルにあるコメントにも記載があります。BugTrack2/235とは完全な別件という事になります。 -- henoheno 2007-05-17 (木) 22:25:34
    "If you compare two numerical strings, they are compared as integers."
  • 上のバージョン(1.26)で、正しくリンクする事を確認しました。
    前述の比較演算子「===」、自分も試そうとしてたはずなんですが・・・、書き換え後に保存してなかったのか失敗に。
    その結果は最初のコメントを見ればわかるように、やや的外れな記述に(泣)。
    ひとまずステータスを「完了」にします。他で同様のものを見つけた場合は、ここを再利用させてもらいます。
    ありがとうございました。 -- 2007-05-17 (木) 23:38:09


  • 2007-05-18 00:00 時点のCVS 版ファイルで、「$page, $_page, $refer, $vars['page'], $var['refer'] 」を、「==」か「!=」で比較しているものをリストアップしてみました。
    lib/file.php(92):	    if ($autoalias && $page == $aliaspage) {
    lib/file.php(285):    if (PKWK_READONLY || $limit == 0 || $page == '' || $recentpage == '' ||
    lib/file.php(422):    if ($page != $whatsnew && ! check_non_list($page))
    lib/file.php(723):    $links[$page] = ($page == $vars['page']) ? $related : array();
    
    lib/html.php(85):	    $is_page = (is_pagename($_page) && ! arg_check('backup') && $_page != $whatsnew);
    lib/html.php(184):    if ($_page == $whatsnew || check_non_list($_page))
    lib/html.php(201):    if (isset($vars['refer']) && $vars['refer'] != '')
    
    lib/link.php(70):	    $_obj->name == $page || $_obj->name == '')
    lib/link.php(117):    if ($_page != $page)
    lib/link.php(152):    if ($page == $whatsnew) continue;
    lib/link.php(158):    $_obj->name == $page || $_obj->name == '')
    lib/link.php(208):    if ($ref_page != $page) $ref .= $line;
    lib/link.php(235):    if ($ref_page != $page) {
    
    lib/make_link.php(21):    return $clone->convert($string, ($page != '') ? $page : $vars['page']);
    lib/make_link.php(782):    if ($page == '') return '<a href="' . $anchor . '">' . $s_alias . '</a>';
    lib/make_link.php(785):    $r_refer = ($refer == '') ? '' : '&amp;refer=' . rawurlencode($refer);
    lib/make_link.php(787):    if (! isset($related[$page]) && $page != $vars['page'] && is_page($page))
    
    plugin/attach.inc.php(98):	    if ($refer != '' && is_pagename($refer)) {
    plugin/attach.inc.php(127):    if ($page == '' || ! is_page($page)) {
    plugin/attach.inc.php(326):    $msg = $_attach_messages[($refer == '') ? 'msg_listall' : 'msg_listpage'];
    plugin/attach.inc.php(327):    $body = ($refer == '' || isset($obj->pages[$refer])) ?
    plugin/attach.inc.php(810):    $page_pattern = ($page == '') ? '(?:[0-9A-F]{2})+' : preg_quote(encode($page), '/');
    plugin/attach.inc.php(833):    if ($page != '') {
    
    plugin/backup.inc.php(25):	    if ($page == '') return array('msg'=>$_title_backuplist, 'body'=>plugin_backup_get_list_all());
    
    plugin/calendar.inc.php(34):    if ($page == '') {
    
    plugin/calendar_viewer.inc.php(310):    if ($vars['page'] != '') $return_vars_array['msg'] .= '/';
    
    plugin/edit.inc.php(96):    if ($page == NULL) $page = '';
    
    plugin/include.inc.php(111):    if ($page == $menubar) {
    
    plugin/lastmod.inc.php(15):    if ($page == ''){
    
    plugin/lookup.inc.php(51):	    if ($page == '') return FALSE; // Do nothing
    
    plugin/map.inc.php(23):    if ($refer == '' || ! is_page($refer)) {
    
    plugin/menu.inc.php(50):    } else if ($vars['page'] == $page) {
    
    plugin/navi.inc.php(103):    if ($page == $current) break;
    plugin/navi.inc.php(130):    if ($_page != '') {
    plugin/navi.inc.php(157):    if ($page != $home)
    
    plugin/newpage.inc.php(42):    if ($vars['page'] == '') {
    
    plugin/pcomment.inc.php(263):    if ($refer != '') pkwk_touch_file(get_filename($refer));
    
    plugin/popular.inc.php(47):    $page == $whatsnew || check_non_list($page) ||
    plugin/popular.inc.php(80):    if ($page == $vars['page']) {
    
    plugin/random.inc.php(50):    if ($page != '') $vars['refer'] = $page;
    
    plugin/related.inc.php(22):    if ($_page == '') $_page = $defaultpage;
    plugin/related.inc.php(29):    if ($page == $whatsnew ||
    
    plugin/rename.inc.php(42):    if ($refer == '') {
    plugin/rename.inc.php(48):    } else if ($refer == $whatsnew) {
    plugin/rename.inc.php(51):    } else if ($page == '' || $page == $refer) {
    plugin/rename.inc.php(83):	    if ($page != '') $body = sprintf($body, htmlspecialchars($page));
    plugin/rename.inc.php(139):    if ($page == '') $page = $refer;
    plugin/rename.inc.php(384):    if ($page == '') $page = PLUGIN_RENAME_LOGPAGE;
    plugin/rename.inc.php(397):    if ($name == $page) continue;
    plugin/rename.inc.php(409):    if ($_page == $whatsnew) continue;
    plugin/rename.inc.php(411):    $selected = ($_page == $page) ? ' selected' : '';
    
    plugin/template.inc.php(60):    } else if ($page != '' && ! $is_pagename) {
    plugin/template.inc.php(65):    $s_page  = ($page == '') ? str_replace('$1', $s_refer, $_msg_template_page) : $_page;
    
    plugin/topicpath.inc.php(34):    if ($page == '' || $page == $defaultpage) return '';
    
    skin/tdiary.skin.php(693):	    if ($_page != '') {
    skin/tdiary.skin.php(699):	    if ($page != '') {
    多分、変更しなくても問題のない、空文字列('')との比較も一応載せてあります。
    • $defaultpage や、$whatsnew 、$menubar との比較は、$defaultpage などの設定を「半角数字のみのページ名」に変更しない限りは問題ないと思いますが、 念のために変更しておいたほうがいいと思います。(一応、下とは別物扱い)
    • $page と$vars['page'] の比較などの、ページ名同士の比較は修正したほうがいいと思います。
      popular プラグインは、recent プラグインの仲間なので、同じ問題が発生するでしょうし、
      rename プラグインでは、例えばページ名を「1」から「001」のように変更しようとしても、 変更前と変更後のページ名が同じだと判断されて、変更後のページ名を入力する場面に戻ってしまいます。

変更すべきか、などの優先順位をこちらで付けられない*1ので、確認をお願いします*2
ステータスを「完了」ではなく、調査開始という意味で「着手」にすればよかった、とちょっと後悔してたり。 -- 2007-05-19 (土) 14:32:38

  • まとめ & 追記ありがとうございます。ステータスなどはいつでも調整できるので気にしないで下さい。この問題(バッドノウハウ?)、あらゆる文字列の比較に === ないし !== を使うようにする癖でも付けない限り解放されなそうですね (^^; -- henoheno 2007-05-20 (日) 22:55:50
  • こちらの件、遅くなりましたが一通り反映しました。確かに空文字と比較している部分についてはこの点の考慮は不要であるとの認識です。 -- henoheno 2007-10-06 (土) 22:22:05
  • 確認しました。$whatsnew に関しては、is_cantedit() の導入で仕組みが変わったので、修正の対象外となったと認識しております。
    あと、上とのファイルのCopyright で気になったところをいくつか。 -- 2007-10-07 (日) 19:14:32
    • lib/link.php (1.17)
      // Copyright (C) 2003-2006, 2007 PukiWiki Developers Team
      • 2006と2007の間を区切っているのが意図的なのかな?、と少し不思議に思っています。
        BugTrack2/61に記載の例に合わせると、2003-2007 になりそうなんですが。
    • lib/make_link.php (1.36)
      //   2003-2005, 2007 PukiWiki Developers Team
      • cvs:lib/make_link.php を見ていると、Rev. 1.31-1.35 は2006年に更新したとなっているので、
        2003-2007 か、上と同じ2003-2006, 2007の方が正しいのではないでしょうか。
    • plugin/comment.inc.php (1.38)
      //   2002-2005, 2007 PukiWiki Developers Team
      • cvs:plugin/comment.inc.php を見ていると、Rev. 1.36 は2006年に更新したとなっているので、…(上と同じなので省略)。
    • 地味なうえに本筋と関係ない指摘ばかりですが、確認お願いします。
    • Copyrightのコメントありがとうございます (^^; その様子だとViewCVSが自由に閲覧できていらっしゃるみたいですから、plugin以下にある、まだこれらのヘッダが無いファイルの追跡などもいけそうですね*3。 -- henoheno 2007-10-07 (日) 22:14:36
    • 修正確認しました。「ViewCVSが自由に閲覧~」はできているのかあまり自信が無いのですが、暇があれば追跡してみて、その結果をBugTrack2/61に挙げたほうがいいのかなと思いはじめてます(お忙しいようですし)。 -- 2007-10-07 (日) 23:55:19

MD5 hash 関係

  • 気がついた分だけですが、修正漏れをいくつか列挙しておきます。 -- 2009-04-18 (土) 06:43:24
    1. plugin/attach.inc.php (1.89) の620行目と672行目、パスワードのMD5 hash の比較
    2. plugin/pcomment.inc.php (1.46) の232行目、リプライ用のMD5 hash の比較
  • 仰る通りかと思います :) -- henoheno 2009-04-18 (土) 09:42:52
  • あれ? この件は根拠が成り立たなくはないですか? なぜなら、hash値は桁数が全て同じであるので、たとえ互いに数字列になったとしても "01" や "001" のようなケースが成立しないからです。(また後日、頭を冷やして良く考えるべき) -- henoheno 2009-05-07 (木) 22:44:56
    • では、実験。'1e0' と'001' を== で比較したらどうなるんでしたっけ? -- 2009-05-07 (木) 23:22:58

返り値としてFALSE だけでなく、FALSE として評価される 0 や "" といった値を返す可能性もある関数

readdir 関数


*1 処理の流れを全部把握してないから、特にlib フォルダ以下のファイル
*2 すでに始めていたら、すみません。修正告知と内容がかぶってしまう(であろう)このコメントを消してください。
*3 ソースの断片まで追おうとすると大変ですが、commit log が追えるならその手前までいけます

トップ   編集 凍結 差分 バックアップ 添付 複製 名前変更 リロード   新規 一覧 検索 最終更新   ヘルプ   最終更新のRSS
Last-modified: 2010-03-08 (月) 13:09:28
Site admin: PukiWiki Development Team

PukiWiki 1.5.2+ © 2001-2019 PukiWiki Development Team. Powered by PHP 5.6.40-0+deb8u7. HTML convert time: 0.543 sec.

OSDN