Project

General

Profile

Bug(バグ) #2882

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

Added by Youichi Kimura over 7 years ago. Updated over 2 years ago.

Status:
Won't fix(対応せず)
Priority:
Normal(通常)
Assignee:
Target version:
Start date:
2012-03-13
Due date:
% Done:

0%

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

Description

現象

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

修正内容

修正内容を記入


Related issues

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

Associated revisions

Revision cc287b7b (diff)
Added by Yuya Watanabe over 6 years ago

(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)

Revision f4b62140 (diff)
Added by Yuya Watanabe over 6 years ago

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

Revision 05d1c37d (diff)
Added by Yuya Watanabe over 6 years ago

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

Revision d78536ac (diff)
Added by Yuya Watanabe over 6 years ago

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

Revision 54730f08
Added by Yuya Watanabe over 6 years ago

Merge branch 't2882' (fixes #2882)

History

#1 Updated by Youichi Kimura over 7 years ago

  • Target version changed from OpenPNE 3.7.0 to 261

#2 Updated by Shouta Kashiwagi over 7 years ago

  • Target version changed from 261 to OpenPNE 3.8.x

#3 Updated by kaoru n over 6 years ago

  • Status changed from New(新規) to Accepted(着手)
  • Assignee set to kaoru n
  • 3.8 で発生するか set to Unknown (未調査)

#4 Updated by kaoru n over 6 years ago

原因は、上記記述内容ではなく(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 Updated by kaoru n over 6 years ago

  • 3.6 で発生するか changed from Unknown (未調査) to Yes (はい)
  • 3.8 で発生するか changed from Unknown (未調査) to Yes (はい)

#6 Updated by kaoru n over 6 years ago

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

#7 Updated by kaoru n over 6 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)
  • Assignee deleted (kaoru n)
  • % Done changed from 0 to 50

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

#8 Updated by Mutsumi Imamura over 6 years ago

  • Target version changed from OpenPNE 3.8.x to OpenPNE 3.8.4

#9 Updated by Mutsumi Imamura over 6 years ago

  • Target version changed from OpenPNE 3.8.4 to OpenPNE 3.9.0-old

#10 Updated by Mutsumi Imamura over 6 years ago

  • Assignee set to Yuya Watanabe

#11 Updated by Yuya Watanabe over 6 years ago

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 Updated by Yuya Watanabe over 6 years ago

  • Status changed from Pending Review(レビュー待ち) to Accepted(着手)
  • % Done changed from 50 to 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 Updated by Yuya Watanabe over 6 years ago

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

  • (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 Updated by Yuya Watanabe over 6 years ago

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

$ 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 Updated by Yuya Watanabe over 6 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)
  • % Done changed from 0 to 50

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

#16 Updated by Kousuke Ebihara over 6 years ago

  • Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
  • % Done changed from 50 to 70

#18 Updated by isao sano over 2 years ago

  • Status changed from Pending Testing(テスト待ち) to Won't fix(対応せず)
  • % Done changed from 70 to 0

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

Also available in: Atom PDF