OpenPNE 3

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

opValidatorDate が 13月 や 32日 を弾くのでこれに任せてしまっていましたが、0月 や 0日 が指定できることが分かりました。その日付自体が不自然という問題もありますが、それ以上に 0年 を指定するとバリデート後に -1年 と評価され、ユーザ側で致命的エラーが生じる場合があることが分かりました。

管理画面からの入力時に日付の最小値を 0001/01/01 と指定することで対応します。なお、指定できる日付の範囲は、西暦1年1月1日から西暦9999年12月31日までという暗黙的な仕様に基づいて実装します。

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

日付型の仕様を厳密にしました。

YYYY/MM/DD の形式しか受け付けません。より厳密には次の正規表現にマッチする文字列です。
/^\s*\d{4}\/\d{1,2}\/\d{1,2}\s*$/

年が4桁でなかったり、年月日の順でないものは不正となります。また、0月0日を不正としました。（前回のテストで受理されてしまったのは opValidatorDate で実装されていたものが不完全だったことが原因です。）

管理画面側のバリデータやエラーについては十分対応できたと思いますが、ユーザ側のバリデータやエラーが若干不適切なままでした。別チケットを作って問題を分散させると参照が困難になると思われるのでこのチケットで併せて修正したいと思います。

(2) は、関連していますが、本チケットの内容と分離して考えることができるので、別チケットでの対応とすることにします。

管理画面側のプロフィール項目（数値、日付のmin,max問題）は修正したということで、テスト待ちにします。

バックポートの検証のために軽く動作を確認しましたが、疑問が発生したので海老原の方でレビューし直します。

修正内容に対する指摘等に入る前に、まず、このチケットに関する作業において日付の最大値、最小値として許容する値のパターンを狭めていますが、そのような選択をした理由がこのチケットからは読み取れなかったので説明をお願いしたいと思います。

note-22 での指摘に対する直接的な回答は、以下「▼ date_format(YYYY/MM/DD) 制限をした理由は次の通りである。」に示しています。関連する全ての情報を示します。

このチケットでの変更内容¶

• 1. editSuccess.php
  • 1-1. 文言が翻訳ファイルと異なっているのを修正(format 直後のドット)
    • 日付型に対する文言：『YYYY/MM/DD 形式で入力（例：2010/07/18, 0001/01/01）
      その他、 PHP の strtotime() 関数が解釈することのできる特殊な文字列が利用可能』
  • 1-2. 日付は年月日(date)情報しか持たないため、フォーマットからtimeを削除
  • 1-3. 具体例を '2009/01/01 23:59:21' から '2010/07/18, 0001/01/01' に変更した
    • この日付に意味はない：(4-1.)に併せて、年が4桁でない場合に0埋めが必要なことを示す例を含めた
• 2. messages.ja.xml
  • 2-1. 「最小値」「最大値」として入力できる数値に制限を指定するため、エラーメッセージを追加
    • これは「文字列長」と「数値」に対するメッセージ（「日付」は無関係）
• 3. ProfileForm.class.php
  • 3-1. 新L103,110: コーディング規約に準拠
  • 3-2. 新L111-115: 文字列長の場合は、(min, max)の下限を(0, 1)に設定
    • 文字列長として負値や(0, 0)を許さない
    • (0, 0)を許さない理由は #837 問題を考慮したため
  • 3-3. 新L125-126: datetime_output を設定
    • 設定しないとプロフィール編集画面で YYYY-MM-DD などとハイフン区切りになるためスラッシュ区切りにした
    • ※with_time, datetime 型が不適切であるため、後に date_output に書き換えている
  • 3-4. 新L139-142: 大小比較のバリデータを追加
    • ※setPostValidator() は誤りなので後に修正
  • 3-5. 新L147-164: 大小比較バリデートメソッドの定義
• 4. ProfileForm.class.php
  • 4-1. 新L125: 入力として受け付けられる日付のフォーマットを YYYY/MM/DD に制限（理由は後述）
  • 4-2. 新L126: (3-3.)を修正、date型に変更
  • 4-3. 新L127: 入力値が(4-1.)でない場合のエラーメッセージ中に示すフォーマットを記述
• 5. opValidatorDate
  • 5-1. L59,68: エラーメッセージ中のフォーマットがハードコーディングされていたので修正
    • 日付の入力が不正だった場合に、(4-3.)のフォーマットでの再入力を促す
  • 5-2: L94: 年月日が 0 な場合を弾く
    • ※この修正は不適切だったため後に元に戻した(参照7-1.)
• 6. ProfileForm.class.php
  • 6-1. (3-4.)を修正
    • 同ファイル56行目 のバリデートを打ち消していたため修正
• 7. opValidatorDate.class.php
  • 7-1. 条件式を書き換えた修正(5-2.)を元に戻した(revert)
• 8. messages.ja.xml, ProfileForm.class.php
  • 8-1. 「最大値：最小値以下でなければなりません。」と(2-1.)で逆のことを言っていたので修正

変更箇所に関する考察¶

修正内容にチケットタイトル以上の修正が含まれていることや修正の流れが不自然に見えるかもしれません。これは、この問題が #837 や #1595 など他にも関連する問題があったことと、修正実施者が適切な修正の全容を認識出来ていない状態で修正を行っていたためです。

• 3-2. (min, max) に関して (0, 1) を下限にした理由
  • 2系から3系へのコンバートを行うと、2系の仕様により (0, 0) という制限値が「制限なし」を意味してしまっていたため、これを考慮して、文字列長 1 以上という前提の下限値として (0, 1) を設定した
  • しかしながら、この問題は #1478 で解消されている
• 3-3. datetime型にしていた理由
  • 修正前が datetime 型であり、この修正時点で date 型にすべきだということに気づいていなかった
• 4-1. YYYY/MM/DD (Y{4}/M{1,2}/D{1,2}) に制限した理由
  • 1: このチケットの修正を始めた時点で、"today" などの「YYYY/MM/DDといった形式以外の入力値」が「YYYY/MM/DD形式」に変換されてからDBに保存されるという（編集時には"today"ではなくなっている）実装となっていて、これを仕様だと思っていた（後に作成された #1595 ではバグ扱いされている）。
    • "today" といった入力値が "YYYY/MM/DD" に変換されてから保存されるのが仕様だと思った理由
      • "today" や "tomorrow" などと相対値を指定し、相対値がDBに保存されるのが正しいとしたら、それはいったい min, max としていつの日付を指すのか特定できないのではないかと思った。
      • 例えば、2010年10月10日に (min,max)=("yesterday", "tomorrow") と管理画面で登録したら、その日付型プロフィールには 2010/10/09 <= date <= 2010/10/11 となる date しか正しくないと判断されるべきではないのか。
      • メンバーがプロフィールを入力する時に yesterday, tomorrow を日付に変換するとなると、入力日の前日から翌日までの入力しか正しくないという判断をすることになり、これは意図する制限値として採用されるべきではないと思った。
  • 2: このチケットの本題であるmin-max比較に関して、(3-4.)(3-5.)の修正を行っていたとき、始めに setPost-(mergePost-) メソッドではなく setPre- メソッドで試していた。Preの場合は min,max の値として（月や日が1桁のような場合でも） "YYYY/MM/DD" という形式の固定10文字長ではなく、入力された値そのもの("today", "2010/1/10" など)であったため比較が難しいと思い(※) YYYY/MM/DD という形式のみを許可するようにしようと思った。
  • 3(※2): strtotime() に入力値を渡せば、 "today" や月部分などが1桁の "2010/1/10" のような値でも比較できるようになるが、そもそもこのフォームの日付として入力できる下限・上限に制限があるのかというところを考え、 strtotime() が扱える日付の範囲に依存しないようにしたいと思い、文字列比較による辞書順での大小比較をするという考えに辿りついた（※）。
  • 4(※3): 文字列比較の場合 ($min = "2010/01/10", $max = "2010/01/02") のように文字列長が同じ（で区切り文字も同じ）ならうまくいくはずだが、文字列長が異なるとうまくいかない気もする。
    • このあたりまで考えて setPre- ではなく setPost-(mergePost-) にすべきだということに気づいた。*Post- であれば日付がバリデート（クリーン）された後なので "2010/1/9" という入力だったとしても "2010/01/09" となり日付型 min,max の文字列比較がうまくできる。
  • 5: (2:)(3:)(4:)の流れから、考察がしっかりしていないまま YYYY/MM/DD という形式に制限したままにしてしまっていた。また、(1:)があるためこれが不自然だとは思っていなかった。
• YYYY/MM/DD に制限している状況に対する考察
  • 仮に "today" や "YYYY-MM-DD" などの（YYYY/MM/DD以外の）文字列を受け付けず、YYYY/MM/DD のみを受け付けるという仕様でよいとした場合、冒頭でも示した次の注意書きが不適切である。
    • 日付型に対する文言：『YYYY/MM/DD 形式で入力（例：2010/07/18, 0001/01/01）
      その他、 PHP の strtotime() 関数が解釈することのできる特殊な文字列が利用可能』
  • このチケットコメントで指摘されているとおり、YYYY/MM/DD に制限すること自体が（これまで可能だったのに変更するという点でも）不自然であるため、この制限は付けるべきではないと現時点では思っている。が、制限を外すべきだという理由も少し弱いため検討している。
  • ▼ date_format(YYYY/MM/DD) 制限をした理由は次の通りである。
    • "today" などの（相対的な）入力があれば、その入力を（絶対的な） "2010/11/24" などに変換してから保存すべきだと思っていた（理由は上記(4-1.1:項)参照）。
    • 日付が自動的に計算されることにより次に示すような意図しない日付指定ができてしまう。以下(C)は(Case)の略。
      • (C1) "2010/02/31" が受け付けられ "2010/03/03" となる。（2010年02月は28日までなので、2月29日が3月1日と計算される）
      • (C2) "2010/02/00" が受け付けられ "2010/01/31" となる。（2月1日の前日と計算され、1月31日になる）
      • (C3) "2010/01/00" が受け付けられ "2009/12/31" となる。(C2)と同じ原理。
      • (C4) "2010/00/01" が受け付けられ "2009/12/01" となる。(C2)と同じ原理。月の場合も同様。
      • (C5) "0000/01/01" が受け付けられる。日付はこのまま保存される。
      • (C6) "0000/00/01" が受け付けられ "-0001/12/01" となる。この結果は異常であり、min-max比較を文字列比較でやろうと思った場合に予期しない結果となりうる。
    • YYYY/MM/DD 形式以外の入力を想定していないと思っていた。
      • "today" 云々の話もあるが、"2010-01-01" のようにハイフン区切りを許しているのも不自然に感じた。
      • こうした経緯があり、 YYYY/MM/DD に限定してしまってもよいと判断した（この考察・判断が正しいかは自分自身でも疑問であり検討したい）。
  • (4-1.)の(1:)項で考察している通りだが "today" などをそのまま保持するという点について認識ができていない（この問題は #1595 を参照）
• 5-2. "!empty($value[$key])" を "0 !== (int)$value[$key]" と変更した理由
  • 1: 上記(4-1.)対応過程で、0年(0000/MM/DD) など年月日のいずれかに 0 が設定できてしまったことに対して、それを意図的に対処しようとした。
  • 2: しかしこの(1:)の修正は、その意図に対して適切なものではなく、取り消されるべきであると指摘された（対応している）。
  • 3: この「0年問題」に対しては date_format 制限(YYYY/MM/DD)によって制約が掛けられている

関連チケットの状況と関係について¶

• #837 : 上記の対応前から認識しており並行して修正した -- 「(min,max)=(0,0) という設定ができてしまい0文字制限されてしまうのは不自然」
  • 2系では (min,max)=(null,null) のような意味を (0,0) として表現していた。この問題はコンバート時に制限なしの項目が0文字制限されてしまうという問題を含んでいる。
    • #837 では、メンバー画面でプロフィール入力時に、既にDBに保存されている(min,max)がより適切な制限値として振舞うように改善している
    • #940 では、管理画面から(min,max)を設定する際のバリデーションを改善している
    • #1449 では、#837 での修正をより強化している（ #837 での対応漏れを対応）
    • #1478 では、コンバート時の(min,max)値を想定した値として扱えるように改善している
• #940 : このチケットです -- 「min > max のまま設定できてしまう」
• #1053 : 上記の対応前からあったようだがチケットの存在を認識していなかった -- 「文字列長の制限値なのに自然数でない負値などが入力できてしまう」
• #1449 : (#837)対応中に派生して作られた -- 「(0)を制限なしとしたため(min,max)=(0,1)の場合でも(-1)などが通ってしまう」
• #1478 : #837 の本質的な対策として作られた -- 「2系は(0,0)が制限なしだが、3系は(0,0)が0制限を意味するので意図しない結果となる」
• #1595 : #1449 の修正チェック時に発覚して作られた -- 「"today" などを入力しても "YYYY/MM/DD" の形式の具体的な日付に変換されてから入力値が保存されている」
  • #930 の修正が #1595 に関係しているらしい
• #1630 : #940 で管理側の値の表記修正を入れているが、それに対するメンバー側の修正として作られた -- 「メンバー側のプロフィール入力時のエラーメッセージのフォーマットが適切でない」

以上、 #940 で多くの修正を行った私が認識している状況についてまとめました。私の修正にも不適切な箇所がある可能性が高いため、在るべき仕様を検討した上で、どこをどう修正すべきか指摘があればお願いします。

このチケットは「プロフィール項目の最小値(min)・最大値(max)の大小比較が行なわれていない」という内容でしたが、その内容を超える修正を同時に行ってしまっています。この点で修正内容が追いにくくなってしまっているかもしれません。

このチケットでの変更内容 に経緯を示していますが、より簡単に現在の状況についてまとめておきます。

• Y{4}/M{1,2}/D{1,2} のような正規表現で YYYY/MM/DD 形式の文字列しか受け付けないようにした
  • 0年や0月や0日を指定できなくしている
  • "today" や "YYYY-MM-DD" などを指定できなくしている
• 文字列長としての (min, max) に対し、下限を (0, 1) に設定した
• min, max の大小を（文字列）比較し、min <= max でなければエラーを返すようにした
• 日付入力時の注釈が翻訳されていなかったのを修正した
• 保存時のフォーマットを date_output' => 'Y/m/d' とスラッシュ区切りになるように指定した

note-23 が長文なので簡単に対応内容を整理しました。

現在の修正内容に対して差し戻しなどがあれば指摘お願いします。レビュー待ちにステータスを変更します。

説明ありがとうございます（7 ヶ月間もかかってしまいすいません……）。日付型プロフィールのバリデーションに関して差し戻しますので、以下 2 点について対応をお願いします。

• #930 により混入した問題（プロフィール項目登録時に strtotime() によって求めた値が登録される問題）に基づく書式の決定はそもそも前提が誤りなので取り消してください
• その上で、以下の「2. ではどうするべきか」で提示した実装方法について検討してください

#930 で混入した問題については既に作られているバグチケットである #1595 で対応してもよいですし、それが紛らわしくなるのであればこのチケットで対応しても構わないと思います。

1. 現状判明している問題点について¶

#1595 で指摘しているとおり、プロフィール項目登録時に strtotime() によって求めた値を DB に格納している問題は、後方互換性を壊しているという点で重大です。

この変更が具体的に影響を受ける例として、誕生日があります。誕生日は min の値として "-100years" が、 max の値として "now" が指定されています。最大の年齢にある程度の制約を設けるのはまあそれなりに妥当ですし、誕生日として未来の日付を指定できないようにするのは仕様として適切です。この挙動が、 #930 によって、プロフィール登録時の -100years と now に固定されてしまったため、プロフィール登録時以降の日付で登録がおこなえなくなってしまいました。

プリセットプロフィール導入前にはこの値が変更できたため、たとえば 18 歳未満の登録を制限する目的でこの min の値を変更していた場合、満 18 歳である（が、プロフィール項目登録時には満 17 歳だった）ユーザが登録できなくなる可能性が出てきます。

プリセットプロフィールは今のところ min や max の値を変更することはできませんが、今後の機能改善によって変更がおこなえるようになった場合に影響を受けやすくなるかもしれません。

もちろん、誕生日以外の独自のプロフィール項目で、 min や max を設定している場合にも、影響を受けます。普通に考えれば日付を入力させようと思った場合に、未来の日付を入力させてよい場合というのは少ないはずですので、このような場合に max に "today" や "-10days" などの値を入れて入力値を制限したいというニーズは当然考えられます。現在の OpenPNE は #930 の対応以降、このような場合に対処できない実装になっています。

ここまでで述べたことは、すべて、 #930 により混入した問題に関することで、本チケットでおこなわれた修正により発生したものではありませんが、 このチケットでは #930 で誤って混入した実装に従って、誤った入力書式の決定がおこなわれてしまいました。この変更は改められるべきです。

// 余談ですが、 ISO-8601 に従うとすれば、 00 月や 00 日は異常な値（MySQL などのシステムでは未入力値として扱っているが）ですが、 0000 年は異常な値ではなく、紀元前 1 年として扱われるのが通常です。さらにさらに余談ですが、 PHP 5.3.x 以降における strtotime() の仕様（http://jp2.php.net/manual/ja/datetime.formats.php）でも年については ISO-8601 に従い 0000 年を紀元前 1 年として扱っているようですが、月や日におけるアンダーフローを許していたり、うるう秒を受け入れていない（？）などの細やかな違いがあるようです。
// まあ、しかし、プロフィール項目で紀元前の日付が必要になる状況はさすがに考えにくいので、バリデーションの際に支障が出るようならこのまま制限してしまってもよいかもしれません（が、書式チェックのことを考えなければ大小の比較はおこなえるようになるため、一応紀元前の日付やアンダーフローした月日も受け入れられるようにはなります）。

2. ではどうするべきか¶

上にあげた問題を回避したうえでこのチケットへの対応をおこなう方法として、以下のようなものが考えられます。

• 1988/04/23 に min に today を、 max に 1988/04/24 を指定してプロフィール項目を登録した場合、翌日にはどの値も受け入れなくなることになるため
• 独自のルールによる比較ではなく、入力値をその時点での strtotime() に通した結果同士で比較をおこなうことで min と max の値が矛盾していないかを検証してもよいが、入力値によっては、時間の経過によって矛盾する結果になるものがありうるかもしれない
• プロフィール項目として許容しうる相対日指定のパターンはある程度限定できると思うが、それ以外の書式を既に使用しているユーザがいるかもしれないことを考えると踏み切りにくい（開発版でのみ種類を限定するというのはアリ）
• ただ、そもそも、絶対日指定か相対日指定かを判別するのは困難である。前述のマニュアルや、 http://lxr.php.net/xref/PHP_5_3/ext/date/lib/parse_date.re を参考に絶対日指定であるかどうかを判別する処理を書くことは、やってやれないことはないとは思うが……
  • MM/DD/YYYY は受け入れないのかとかも考えないといけない

2. 1. とほぼ同様だが、入力できる値の種類の制限は設けない

• 元々の実装をほとんど維持する形になる
• これが一番現実的かも
• 心もとないなら、注意書きを充実させるなり、大雑把に strtotime() の結果どうしで比較してマズそうなら登録はするものの警告出すとか

3. 安定版には取り込まれるべきか¶

このチケットのバックポートチケットである #1265 にて安定版における取り込みについて述べられていたのでこの点についても言及します。

バグチケットでの対応方針が明確にはなっておらず、安定版での取り込みは難しいので対応しない方針とします

とありますが、バックポートチケットへの対応を検討した時点で対応方針が明確になっていないからといって安定版での取り込みが難しいというのは論理が飛躍している、というより意味がわかりません（別チケット対応にするなどということならわかりますが……）。この問題は明らかにバグで、しかもこの問題によって長期間後方互換性が壊れています。安定版で対応しない理由がありません。安定版の安定性を損なわない形で変更を取り込んでください。

http://redmine.openpne.jp/issues/940#note-26 でのコメントありがとうございます。以前着手してから時間が経ってしまっていましたが、近々この問題を解消あるいは整理したいと考えていました。

#930 で混入した問題と、想定的な日付指定に対する仕様について認識を改めたので、 note-26 を参考に後方互換性等の問題を解消することを前提に、より好ましい仕様を検討して修正を行いたいと思います。

少なくとも、このチケットで私が既にコミットした内容は note-26 で指摘されている通り、あるべき仕様を誤解した状態での修正であるため、少なくともこれらの不適切な修正を取り消すことを対応します。更に適切な修正が可能そうであれば修正方針を示した上でコミットしたいと思います。

現時点で、このチケットで扱おうとしていた内容を示します。

• （このチケットに関連付けされている多くのチケットなど）関連する問題はいくつか修正完了している
• このチケットでは次の問題のいくつか（あるいは全て）を修正し、必要があれば残りの問題の別チケットを作る
  • プロフィール項目の最小値と最大値の比較をしていない（文字数の制限、数値の制限に関して）
  • 非負整数のみが受け付けられるべき箇所で負値が入力できる（別チケットがあったかもしれないが、ここで扱ってもよいと思った）
  • プロフィール管理画面（編集画面ではなく、一覧画面）で、各プロフィールの制限値を表示するようにする
    • これは最小値と最大値の比較について、日付型の制限のバリデートを書けない（相対的日付と絶対的日付の比較は、その観測時点によって妥当性が変わる）ので、プロフィール管理画面を開いた時点でバリデートし、日付に問題があれば管理者に「おかしい」ことを分かるようにする
    • 別のチケットで「矛盾する最小値、最大値による制限」がある場合に、その制限がなかったことにするような修正が入っている（可能性がある）ので、バリデートが通らない（本チケットに基づく最小値最大値の妥当性の矛盾がある）場合に管理者に気づかせる必要がある。これは note-26 の「心もとないなら、注意書きを充実させるなり、大雑把に strtotime() の結果どうしで比較してマズそうなら登録はするものの警告出すとか」のことである。

なお、安定版 3.6 向けにこの問題を修正する必要性があるかについては、この問題は管理者にしか関係せず、管理者にとって不便を強いる不具合ではない（このチケットの指摘通り直す必要性はあまりない）と考えられるので、対応の必要性は弱いだろうということを示しておきます。