プロジェクト

全般

プロフィール

Bug(バグ) #2882

opValidatorDate で「年」に 0 を入力した場合に空値として認識される

Youichi Kimura約12年前に追加. ほぼ7年前に更新.

ステータス:
Won't fix(対応せず)
優先度:
Normal(通常)
担当者:
対象バージョン:
開始日:
2012-03-13
期日:
進捗率:

0%

3.6 で発生するか:
Yes (はい)
3.8 で発生するか:
Yes (はい)

説明

現象

opValidatorDate を使用しているフォーム(コミュニティイベントの作成画面など)で、日付を入力する項目の「年」に 0 を入力すると空値として扱われる。

  • コミュニティイベントの作成画面の場合、「募集期日」の「年」に 0 と入力された状態(「月」「日」は入力しない)で送信しても「正しくありません。」と表示されずにイベントの作成が完了する。

原因

lib/validator/opValidatorDate.class.php の 100 行目付近

    // if one date value is empty, all others must be empty too
    $empties =
      (!isset($value['year']) || !$value['year'] ? 1 : 0) +
      (!isset($value['month']) || !$value['month'] ? 1 : 0) +
      (!isset($value['day']) || !$value['day'] ? 1 : 0);

この判定で、「年」($value['year']) に "0" が入力されると !isset($value['year']) || !$value['year']true となり空値と判定されることが原因。

参照: PHP: PHP 型の比較表 - Manual

修正内容

修正内容を記入


関連するチケット

関連している OpenPNE 3 - Bug(バグ) #3270: sfValidatorDateで「年」に 0 を入力した場合に空値として認識される New(新規) 2012-12-05
関連している OpenPNE 3 - Backport(バックポート) #3281: opValidatorDate で「年」に 0 を入力した場合に空値として認識される Fixed(完了) 2012-12-17
関連している OpenPNE 3 - Backport(バックポート) #3288: opValidatorDate で「年」に 0 を入力した場合に空値として認識される Fixed(完了) 2012-03-13
関連している OpenPNE 3 - Bug(バグ) #3291: opValidatorDate がタイムゾーンを考慮していない可能性がある. New(新規) 2013-01-10

関係しているリビジョン

リビジョン cc287b7b (差分)
Yuya Watanabe約11年前に追加

(refs #2882) add sfValidatorDateTest +38 case

about test case
empty values: ('', false, array(), new SimpleXmlElement('<hoge />'))
not empty values: (0, '0', 0.0)

required:         (true, false)
date array: (Yms, YmsH)

リビジョン f4b62140 (差分)
Yuya Watanabe約11年前に追加

(refs #2882) fixed not to evaluate empty array in preg_match

リビジョン 05d1c37d (差分)
Yuya Watanabe約11年前に追加

(refs #2882) fixed not to be empty for zero number or string

リビジョン d78536ac (差分)
Yuya Watanabe約11年前に追加

(refs #2882) fixed not to be empty day values for zero number or string

リビジョン 54730f08
Yuya Watanabe約11年前に追加

Merge branch 't2882' (fixes #2882)

履歴

#1 Youichi Kimura約12年前に更新

  • 対象バージョンOpenPNE 3.7.0 から 261 に変更

#2 Shouta Kashiwagi約12年前に更新

  • 対象バージョン261 から OpenPNE 3.8.x に変更

#3 kaoru n11年以上前に更新

  • ステータスNew(新規) から Accepted(着手) に変更
  • 担当者kaoru n にセット
  • 3.8 で発生するかUnknown (未調査) にセット

#4 kaoru n11年以上前に更新

原因は、上記記述内容ではなく(opValidatorDateは使用していない)、以下の記述内容にありました。

入力チェックに使用しているクラスsfValidatorDate内で入力値のEmptyチェックを行っています(sfValidatorDateのisEmpty)が、与えられた引数がarrayの場合に「array_filter」という関数を使用しています。
この関数は(http://php.net/manual/ja/function.array-filter.phpより)

コールバック関数が与えられなかった場合、 input のエントリの中で FALSE と等しいもの (boolean への変換 を参照ください) がすべて削除されます。 

ということで、年に入力された0がfalse扱いされ削除されます。
その後Emptyチェックを行うため、年に0のみが入力された場合、Emptyと判定され必須項目ではないためエラーチェックに引っかかりません。

#5 kaoru n11年以上前に更新

  • 3.6 で発生するかUnknown (未調査) から Yes (はい) に変更
  • 3.8 で発生するかUnknown (未調査) から Yes (はい) に変更

#6 kaoru n11年以上前に更新

opValidatorDateのバグはバグとして存在していて、コミュニティイベントの作成時にsfValidatorDateで現れる現象としては同じですが、起こっているバグは別問題でした。
ですので、このチケットでは、opValidatorDateのバグを扱い、コミュニティイベントの作成時の件は別チケットを起票します。

#7 kaoru n11年以上前に更新

  • ステータスAccepted(着手) から Pending Review(レビュー待ち) に変更
  • 担当者 を削除 (kaoru n)
  • 進捗率0 から 50 に変更

https://github.com/openpne/OpenPNE3/pull/62
プルリクエストしました。

#8 Mutsumi Imamura11年以上前に更新

  • 対象バージョンOpenPNE 3.8.x から OpenPNE 3.8.4 に変更

#9 Mutsumi Imamura11年以上前に更新

  • 対象バージョンOpenPNE 3.8.4 から OpenPNE 3.9.0-old に変更

#10 Mutsumi Imamura11年以上前に更新

  • 担当者Yuya Watanabe にセット

#11 Yuya Watanabe約11年前に更新

sfValidatorBase.class.php では値が下記の 3 つのいずれかになる場合に isEmpty() が true となる.

    特別な値 NULL (値がセットされていない変数を含む)
    空の文字列
    要素の数がゼロである 配列

sfValidatorDate.class.php では上記に加えて,配列の中の物が下記のものも isEmpty() が true となる値である.

    boolean の FALSE
    integer の 0 (ゼロ)
    float の 0.0 (ゼロ)
    文字列の "0" 
    空のタグから作成された SimpleXML オブジェクト

PHP: 論理型 (boolean) - Manual
http://php.net/manual/ja/language.types.boolean.php#language.types.boolean.casting

わざわざ isEmpty() をオーバーライドしているのは, sfValidatorBase#isEmpty() が単項目のみを考慮しているため.
下記コミットで配列で渡された場合を考慮している.
http://trac.symfony-project.org/changeset/7081

ただし,上記コミットのテストコードを見て分かる通り(下記に記述),それぞれの中身が null, '', array() 以外のものであることを考慮していない.つまり,動作としては正しいが過剰にフィルタしているという推測ができる.このコミットが5年前(2013/01/08 現在)であり,それ以降 isEmpty() については修正がされていないため sfValidatorDate のバグとして Symfony 側を修正するという手もあるが,ここでは議論しないこととする.

158 // required
159 $v = new sfValidatorDate();
160 foreach (array(
161   array('year' => '', 'month' => '', 'day' => ''),
162   array('year' => null, 'month' => null, 'day' => null),
163   '',
164   null,
165 ) as $input)
166 {
167   try
168   {
169     $v->clean($input);
170     $t->fail('->clean() throws an exception if the date is empty and required is true');
171   }
172   catch (sfValidatorError $e)
173   {
174     $t->pass('->clean() throws an exception if the date is empty and required is true');
175   }
176 }

sfValidatorDate#isEmpty() が 0 をどう扱うかについて言明していないことを考慮すると, opValidatorDate#isEmpty() によって入力がないかどうかを調べる際に 0 を空入力としないようにする修正は妥当であると思われる.ただし, プルリクエストのように sfValidatorBase#isEmpty() をそのまま使ってしまう場合, $value の値が配列である場合にそれぞれの値が空入力であるかどうかという点について考慮されなくなってしまうため間違いである.

また, year のみを考慮しているが現実としてはこの値を西暦とすると紀元前など正の整数以外の年が存在することを鑑みた上で 0 より大きい値のみを受け付けるという仕様として認識できるが,月および日については 0 月 8 日や 1 月 0 日などは存在しないため、むしろ月および日について修正を行う必要があるのではないかという考え方もできる.

別件

下記のようなコードが合った場合の出力として, isEmpty() が true か false かによって返ってくる型が違う.これは sfValidatorBase の中で isEmpty() が true の場合は getEmptyValue() によって返ってくる値をそのまま返し, false の場合は sfValidatorDate の処理を通って返ってくるという違いが発生するため.仕様といえば仕様だが直感的ではなさそう.

$validator = new opValidatorDate();
$validator->setOption('empty_value', new DateTime('1989-01-08')); 
var_dump($validator->clean(array('year' => '', 'month' => '', 'day' => '')));
var_dump($validator->clean(array('year' => '', 'month' => '', 'day' => '', 'hour' => '10')));

出力

object(DateTime)#23 (3) {
  ["date"]=>  string(19) "1989-01-08 00:00:00" 
  ["timezone_type"]=>  int(3)
  ["timezone"]=>  string(10) "Asia/Tokyo" 
}
string(10) "1989-01-08" 

別件2

lib/vendor/symfony/test/unit/validator/sfValidatorDateTest.php より,timezone についてのテストコードを拝借してきて opValidatorDate に適用してみたときにテストがパスしない.sfValidatorDate に変更すると通る.

  1 <?php
  2 
  3 include_once dirname(__FILE__) . '/../../bootstrap/unit.php';
  4 
  5 $t = new lime_test(1, new lime_output_color());
  6 
  7 $v = new opValidatorDate();
  8 
  9 $t->diag('opValidatorDate');
 10 
 11 // timezones
 12 $defaultTimezone = new DateTimeZone(date_default_timezone_get());
 13 $otherTimezone = new DateTimeZone('US/Pacific');
 14 if ($defaultTimezone->getOffset(new DateTime()) == $otherTimezone->getOffset(new DateTime()))
 15 {
 16   $otherTimezone = new DateTimeZone('US/Eastern');
 17 }
 18 
 19 $date = new DateTime('2000-01-01T00:00:00-00:00');
 20 $date->setTimezone($otherTimezone);
 21 $v->setOption('with_time', true);
 22 $clean = $v->clean($date->format(DATE_ATOM));
 23 
 24 $date->setTimezone($defaultTimezone);
 25 $t->is($clean, $date->format('Y-m-d H:i:s'), '->clean() respects incoming and default timezones');

$ php opValidatorDateTestForTimezone.php
not ok 1 - ->clean() respects incoming and default timezones
#     Failed test (./opValidatorDateTestForTimezone.php at line 25)
#            got: '1999-12-31 16:00:00'
#       expected: '2000-01-01 09:00:00'

#12 Yuya Watanabe約11年前に更新

  • ステータスPending Review(レビュー待ち) から Accepted(着手) に変更
  • 進捗率50 から 0 に変更

コードからみる仕様

付録 B - バリデーター (1_2) - Symfony
http://symfony.com/legacy/doc/forms/1_2/ja/B-Validators#chapter_b_sub_sfvalidatordate

  • 次のキーで構成される配列: year、month、day、hour、 minute と second
    • すべて数字
    • year は 4 桁以内の数字
    • year, month, day は all or nothing
    • checkdate() が true になるような (year, month, date) の組み合わせがセットされている.
    • with_time が指定されている場合は (hour), (hour, minute), (hour, minute, second) の組み合わせがセットされている.
  • 提供されるのであれば date_format 正規表現にマッチする文字列
  • PHP の strtotime() 関数によって解析できる文字列
  • タイムスタンプを表す整数
  • required が false かつ isEmpty() が false である場合は empty_value は strtotime() 関数によって解析できる文字列か date('YmdHis', $value) で解析できる数字列でなければならない.

opValidaterDate

  • 次のキーで構成される配列: year、month、day、hour、 minute と second
    • すべて数字
    • year は 4 桁以内の数字
    • year, month, day は all or nothing
    • checkdate() が true になるような (year, month, date) の組み合わせがセットされている.
    • with_time が指定されている場合は (hour), (hour, minute), (hour, minute, second) の組み合わせがセットされている.
  • 提供されるのであれば date_format 正規表現にマッチする文字列
  • PHP の strtotime() 関数によって解析できる文字列
  • required が false かつ isEmpty() が false である場合は empty_value は DateTime オブジェクトでなければならない
sfValidatorDate と opValidatorDate を外側から見た場合の違いとしては,下記の2つがある.
  • opValidaterDate は入力にタイムスタンプを許容しない
  • opValidaterDate は empty_value に DateTime を用いる必要がある

isEmpty について

意図としては任意の値が空ではない(0 は空でないとする)状態を考えたいので下記のようなものが考えられる.

function isEmpty($value) {
  if (!is_array($value)) {
    return parent::isEmpty($value);
  }

  foreach ($value as $v) {
    if (!in_array($v, array(null, '', array()), true)) {
      return false;
    }
  }

  return true;
}

setTimezone の挙動について

sfValidatorDate と opValidatorDate の挙動に差があるが,今回は考慮しないこととする.

修正方針

上記 isEmpty のコードを用いて空判定を行う. つまり, 0 は空文字列ではないという扱い.
また,タイムスタンプを許容しないことおよび empty_value が DateTime であるべきということをコメントで明記しておく.

#13 Yuya Watanabe約11年前に更新

下記観点のテストケースを追加した結果を示しておく.

  • (boolean) として扱う場合に false となるもので 空として扱うものと扱わないもの
  • required が true の場合と false の場合
  • isEmpty が true の場合と false の場合
空入力として扱う値
  • '',
  • null,
  • false,
  • array()
  • new SimpleXMLElement('<hoge />')
空入力として扱わない値
  • 0
  • '0'
$ prove --exec php opValidatorDateTest.php
opValidatorDateTest.php .. Failed 10/80 subtests 

Test Summary Report
-------------------
opValidatorDateTest.php (Wstat: 0 Tests: 76 Failed: 6)
  Failed tests:  58, 60, 68-69, 75-76
  Parse errors: Bad plan.  You planned 80 tests but ran 76.
Files=1, Tests=76,  0 wallclock secs ( 0.04 usr  0.01 sys +  0.43 cusr  0.06 csys =  0.54 CPU)
Result: FAIL

#14 Yuya Watanabe約11年前に更新

浮動小数点を空入力として扱わないとしてテストを追加.
手元の環境ですべてのテストにパスすることを確認.

$ prove --exec php opValidatorDateTest.php
opValidatorDateTest.php .. ok     
All tests successful.
Files=1, Tests=84,  1 wallclock secs ( 0.04 usr  0.01 sys +  0.43 cusr  0.06 csys =  0.54 CPU)
Result: PASS

#15 Yuya Watanabe約11年前に更新

  • ステータスAccepted(着手) から Pending Review(レビュー待ち) に変更
  • 進捗率0 から 50 に変更

更新履歴 54730f08546bf98f8886cc0941b76ebfab24e086 で適用されました。

#16 Kousuke Ebihara約11年前に更新

  • ステータスPending Review(レビュー待ち) から Pending Testing(テスト待ち) に変更
  • 進捗率50 から 70 に変更

#18 isao sanoほぼ7年前に更新

  • ステータスPending Testing(テスト待ち) から Won't fix(対応せず) に変更
  • 進捗率70 から 0 に変更

OpenPNE 3.8.4 にて対応済みであったため、対応せずとします。

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