Project

General

Profile

Actions

Backport(バックポート) #2416

closed

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

Added by Shingo Yamada about 13 years ago. Updated about 13 years ago.

Status:
Fixed(完了)
Priority:
High(高め)
Assignee:
Target version:
Start date:
2011-08-17
Due date:
2011-09-29
% Done:

100%

Estimated time:

Description

概要

管理画面でプロフィール項目を作成、あるいは編集するときに、識別名に全角文字(※)が使える。全角文字が使えること自体は識別名がそのような仕様であれば問題ないが、それは想定した仕様ではないはずであり、識別名に全角文字を含めてプロフィール項目を作ってしまうと、携帯版でプロフィール編集が成功しなくなる(バリデータに弾かれる)という問題が起こる(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 の内容)についてはバックポート対応が可能ではある。


Related issues 1 (0 open1 closed)

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

Actions
Actions #1

Updated by Minoru Takai about 13 years ago

  • Assignee deleted (Minoru Takai)

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

Actions #2

Updated by Shingo Yamada about 13 years ago

  • Due date set to 2011-09-29
  • Assignee set to Minoru Takai
  • Target version changed from OpenPNE 3.6RC1 to OpenPNE 3.6.0
Actions #3

Updated by Minoru Takai about 13 years ago

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

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

Actions #4

Updated by Fumie Toyooka about 13 years ago

  • Target version changed from OpenPNE 3.6.0 to OpenPNE 3.6RC2
Actions #5

Updated by Minoru Takai about 13 years ago

  • Status changed from New(新規) to Pending Review(レビュー待ち)
  • % Done changed from 0 to 50

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

Actions #6

Updated by Minoru Takai about 13 years ago

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

Actions #7

Updated by Minoru Takai about 13 years ago

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

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

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

Actions #8

Updated by Fumie Toyooka about 13 years ago

  • Status changed from Pending Testing(テスト待ち) to Fixed(完了)
  • % Done changed from 70 to 100

テストOKです.

Actions

Also available in: Atom PDF