プロジェクト

全般

プロフィール

Backport(バックポート) #2416

管理画面でプロフィール項目の識別名に全角文字や角括弧が使えてしまう

Shingo Yamada12年以上前に追加. 12年以上前に更新.

ステータス:
Fixed(完了)
優先度:
High(高め)
担当者:
対象バージョン:
開始日:
2011-08-17
期日:
2011-09-29
進捗率:

100%


説明

概要

管理画面でプロフィール項目を作成、あるいは編集するときに、識別名に全角文字(※)が使える。全角文字が使えること自体は識別名がそのような仕様であれば問題ないが、それは想定した仕様ではないはずであり、識別名に全角文字を含めてプロフィール項目を作ってしまうと、携帯版でプロフィール編集が成功しなくなる(バリデータに弾かれる)という問題が起こる(PC版では弾かれない、ある文字列の場合はPC版も弾かれる:後述)。

※ここでは本来想定外の文字を「全角文字」と称することにする。実際はこれに全角でなくても弾くべき文字(例えば半角カナ等)が含まれるはずである。

「識別名に全角文字が含まれている場合に携帯版で、ある文字列の場合にPC版で、プロフィール編集ができなくなる」という問題と捉えることも可能かもしれないが、そもそも識別名に全角文字が使えることは想定していないため、「プロフィール項目の識別名に全角文字が使えてしまう」という問題として捉える。

この問題は #839 (3.5.1 向け) ですでに扱われているはずだが、 #839 は修正が不完全なまま完了とされている。このバックポートチケット #900 (3.4.4 向け) では、より適切な修正が行われており、 3.5 以降でこの問題が残ったままになってしまっている。

携帯版で弾かれる理由

識別名に全角文字が使われているとき、PC版ではプロフィール編集が問題なく行えるようだが、携帯版ではバリデータに弾かれてしまう。「正しくありません。(Invalid.)」という文言しか表示されないため何が原因で弾かれているのかすぐには分からないが、これは「識別名に使われている全角文字が、内部では UTF-8 エンコーディングで扱われるのに対し、ブラウザ側では Shift_JIS エンコーディングとして扱われるため、その差異により識別名のキーが不一致を起こす」ということによるものである。

キーの不一致を起こしてしまうことが問題ではなく、識別名に全角文字が使えてしまうことを問題の本質と捉えるべきだろう。

PC版で弾かれるケース

PC版では、(偶然ではあるが)内部でのエンコーディングもブラウザ側でも UTF-8 エンコーディングとしてキー値が扱われるため、例えば識別名が「あいうえお」であってもプロフィールの編集は問題なく行える。この場合、 input 要素の name 属性値は "profile[あいうえお][value]" のようになる(プロフィール項目の種類によって若干異なるが)。

識別名に「abc"def」を使った場合は、 htmlspecialchars() 相当の処理でエスケープ(文字参照への変換)が行われるため name 属性値は "profile[abc"def]" となる。

ここで問題となるのは識別名に「aaa[bbb」や「xxx]yyy」のように角括弧を含んでいる場合で、角括弧はエスケープの対象ではないため name 属性値が "profile[xxx]yyy][value]" となってしまう。これでは内部とブラウザ側で、識別子のキーが一致するものの、角括弧によって表現されている配列の構造が崩れてしまうためキーの取得ができず、「正しくありません。」とバリデータに弾かれてしまう。これは原理的に明らかだが、PC版と携帯版共通の問題である。

修正方針

#900 での検討内容をもとに、識別名に対して同様のバリデートを行うようにする。

#900 ではバリデートは行われているが、入力可能な文字の説明や、エラーメッセージが適切ではないため、メッセージの追加も併せて行うことにする。

修正前と修正後の動作

======== 修正前

==== バリデートでエラーとなる(登録できない)
 1. 【適切(*1)】 "op_preset_hoge" 先頭が op_preset_ で始まる名前を付けようとする(3系独自のバリデート:正しくありません)
 2. 【適切】 "" 空文字列(必須項目です)
 3. 【仕様による(*2)】 " " 半角スペースのみ(trim が有効:必須項目です)
 4. 【仕様による(*2)】 "  " 全角スペースのみ(opValidatorString の trim が有効:必須項目です)

==== バリデートでエラーとならない(登録できる)
 5. 【適切】 "foo" 妥当な文字列
 6. 【仕様による(*2)】 " bar " 前後に半角スペースを含む妥当な文字列(trim が有効)
 7. 【仕様による(*2)】 " buzz " 前後に全角スペースを含む妥当な文字列(opValidatorString の trim が有効)
 8. 【不適切】 "あいうえお" 全角文字を含む文字列
 9. 【不適切】 "abc]def" 角括弧(開き、閉じの少なくとも一方)を含む文字列
10. 【仕様による(*3)】 "hoge#fuga=piyo?" いろいろな記号を含む文字列
11. 【仕様による(*3)】 "12345" 半角数字のみの文字列
12. 【仕様による(*3)】 "____" アンダースコアのみの文字列
13. 【仕様による(*3)】 "123abd" 数字から始まる妥当そうな文字列

======== 修正後

==== 識別名に関する注釈を追加 (*0)
「アンダースコアと半角英数字のみが使えます。識別名にはアルファベットが含まれていなければなりません。」

==== バリデートでエラーとなる(登録できない)
 1. 【適切(*1)】 "op_preset_hoge" 先頭が op_preset_ で始まる名前を付けようとする(3系独自のバリデート:適切なエラーメッセージ)
 2. 【適切】 "" 空文字列(必須項目です)
 x. 【適切】上記 3, 4, 6, 7, 8, 9, 10, 11, 12 (今回の修正:形式が不正)

==== バリデートでエラーとならない(登録できる)
 5. 【適切】 "foo" 妥当な文字列
13. 【適切】 "123abd" 数字から始まる妥当そうな文字列
14. 【適切】 妥当な文字列はこの他、アンダースコアと半角英数字のみからなり、少なくともアルファベットを1文字含む文字列

変更点について:

  • (*0) 識別名に対する注釈
    • 識別名の入力フォームのそばに、識別名として受け付けられる文字列についての説明を加えた。
  • (*1) op_preset_ で始まる名前の場合
    • 修正前もエラーとなっていたが、修正前には適切なエラーメッセージが表示されていなかったので修正した。
  • (*2) 入力文字列の前後に空白がある場合
    • trim は行わないままバリデートすることにした。つまり、修正前は受け付けられていた 6 や 7 のケースを両方ともエラーとなるようにした。
  • (*3) 識別名に使える文字列と、識別名の制約について
    • #900 での検討内容を妥当だと判断したため、 10, 11, 12 のケースはエラーとなるようにした。
    • 13 のケース(数字から始まる識別名)は一般的には受け付けない印象があるが、プロフィール項目の識別名としては受け付けても特に問題がなく、 2 系時代でも受け付けていたため、 3.4 系の修正と合わせ、これは許容することにした。 12 のケース(アンダースコアのみ)を弾いたのも 3.4 系と合わせるためであり、 3.6 系以降で 12 のケースを弾いた特別な理由はない( 12 のケースを弾かないという選択肢もあり得た)。

(*2) について、 trim を行う必要があるかどうかについてだが、 sfValidatorString の trim (一般的な空白類のみ)にしても opValidatorString の trim (全角スペースを含む空白類)にしても、識別名において trim を行う必要はないと判断した。そもそも trim は、空白類の入力も受け付けるテキスト入力エリアにおいて、その入力値の前後に付く連続した空白類を取り除くという意図が強いように思う。もともと空白類の入力を受け付けないテキスト入力エリアでは入力値が trim される必要はないと判断した。

対象バージョン

master(3.7), 3.6.x で修正が必要。

3.4.x では #900 の修正が行われているため対象外だが、メッセージの追加(上記 *0 と *1 の内容)についてはバックポート対応が可能ではある。


関連するチケット

関連している OpenPNE 3 - Bug(バグ) #2356: 管理画面でプロフィール項目の識別名に全角文字や角括弧が使えてしまう Fixed(完了) 2011-08-17

関係しているリビジョン

リビジョン 0ccf65af (差分)
Minoru Takai12年以上前に追加

(fixes #2416, BP from #2356) add validation for the profile identification name on backend.

リビジョン 3314d916 (差分)
Minoru Takai12年以上前に追加

(fixes #2416, BP from #2356) set error message for a case that inputted "op_prefix_" as a prefix.

履歴

#1 Minoru Takai12年以上前に更新

  • 担当者 を削除 (Minoru Takai)

OpenPNE-3.6RC1 に含めるかを先に検討します。

#2 Shingo Yamada12年以上前に更新

  • 期日2011-09-29 にセット
  • 担当者Minoru Takai にセット
  • 対象バージョンOpenPNE 3.6RC1 から OpenPNE 3.6.0 に変更

#3 Minoru Takai12年以上前に更新

OpenPNE-3.6.0 対象のチケットは、OpenPNE-3.6RC1 がリリースされた後で stable-3.6.x ブランチに修正を取り込みます。

OpenPNE-3.6RC1 のリリース前に stable-3.6.x ブランチにコミットをしないでください。

#4 Fumie Toyooka12年以上前に更新

  • 対象バージョンOpenPNE 3.6.0 から OpenPNE 3.6RC2 に変更

#5 Minoru Takai12年以上前に更新

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

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

#6 Minoru Takai12年以上前に更新

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

#7 Minoru Takai12年以上前に更新

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

親チケットでの修正が stable-3.6.x ブランチに取り込まれていることを確認しました。

また、実装者テストを簡単に行い、修正が反映できていることを確認しています。レビューOKとします。

#8 Fumie Toyooka12年以上前に更新

  • ステータスPending Testing(テスト待ち) から Fixed(完了) に変更
  • 進捗率70 から 100 に変更

テストOKです.

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