Ticket #488 (closed バグ指摘: 無効)

Opened 11 years ago

Last modified 8 years ago

POSTされる値とDBの文字コードが一致しない場合、DBエラーとなる

Reported by: kishida Owned by: somobody
Priority: Milestone: EC-CUBE2.11.0
Component: フロント Version: 2.4.0
Keywords: Cc: Seasoft
修正済み: yes

Description (last modified by nanasess) (diff)

POSTする時にブラウザの文字コードを、 ECCUBEで指定した文字コード以外を設定しPOSTした場合、 文字コードのチェックを行っていない為、 DBで下記のようなエラーが起こる場合がある。 (※POSTされる値によってエラーとなる場合がある)

[nativecode=ERROR:  invalid byte sequence for encoding "EUC_JP": 0x8250
HINT:  This error can also happen if the byte sequence does not match the encoding expected by the server, which is controlled by "client_encoding".]

解決策

// 検索語がUTF-8じゃなかったら、強制的にUTF-8にする
$encode = mb_detect_encoding($_POST['name']);
if( CHAR_CODE != $encode ){
    $name = mb_convert_encoding($_POST['name'], "UTF-8", "auto");
}

Attachments

448-CHAR_CODEをSJISに.gif Download (27.1 KB) - added by Seasoft 11 years ago.

Change History

comment:1 Changed 11 years ago by kajiwara

  • Milestone changed from EC-CUBE2.4.1 to EC-CUBE2.4.2

影響範囲と修正点が少し曖昧であるため、一旦マイルストーンを2.4.2に変更し、
引き続きの対応とさせていただきたいと思います。

comment:2 Changed 11 years ago by kajiwara

  • Owner changed from somebody to kajiwara
  • Status changed from new to assigned

comment:3 Changed 11 years ago by kajiwara

  • Owner kajiwara deleted
  • Status changed from assigned to new

comment:4 Changed 11 years ago by nanasess

  • Owner set to nanasess
  • Status changed from new to assigned

comment:5 Changed 11 years ago by nanasess

  • Description modified (diff)

comment:6 Changed 11 years ago by nanasess

  • Description modified (diff)

comment:8 Changed 11 years ago by nanasess

comment:9 Changed 11 years ago by nanasess

version-2_4-dev r18503 で修正

comment:10 Changed 11 years ago by nanasess

r18504 にて, 多次元配列の場合は再帰するように修正

comment:11 Changed 11 years ago by nanasess

r18505 にて, 文字エンコーディングの検出に失敗した場合に対応

comment:12 follow-up: ↓ 13 Changed 11 years ago by Seasoft

  • Cc Seasoft added

意図しない文字列に変換されて継続動作してしまうよりは、エラーで止まる方が良いような気もします。

むしろ、意図しないエンコーディングを検出したら、DB にデータを渡さずに、EC-CUBE でエラーを発生させて、止まるような実装はいかがでしょうか?

なお、(ブラウザ依存などで?) EC-CUBE 単体で不正な文字列が送出される懸念があるようでしたら、フォームの accept-charset 属性などで対処できないでしょうか?

comment:13 in reply to: ↑ 12 ; follow-up: ↓ 14 Changed 11 years ago by nanasess

Seasoft への返信

意図しない文字列に変換されて継続動作してしまうよりは、エラーで止まる方が良いような気もします。

あくまでも可能性ですが, 文字列ならないバイトコードが直接入力される可能性もあり, その場合 SQLインジェクションの危険もあります.

本来ならば, すべての入力値をチェックして, 不正であればエラーにするべきだと思うのですが, なかなか大変です. この修正のように, mb_convert_encoding で, 正常な文字列に変換してしまった方が, 安全だと思います. 変換できなかった文字は, mbstring.substitute_character で無効化することもできますし.

むしろ、意図しないエンコーディングを検出したら、DB にデータを渡さずに、EC-CUBE でエラーを発生させて、止まるような実装はいかがでしょうか?

意図しないエンコーディングなのか, 正常な入力値なのにも係わらず検出できなかったエンコーディングなのか, 判別する術を思いつかなかったため, 文字エンコーディングが検出できなかった場合は, 無変換で通すようにしています.

なお、(ブラウザ依存などで?) EC-CUBE 単体で不正な文字列が送出される懸念があるようでしたら、フォームの accept-charset 属性などで対処できないでしょうか?

元々, 海外のクローラなどが入力してくる文字列で, DB エラーが発生するという問題があるとのことで, このチケットが登録されたとのことなので, リクエストの内容を直接チェックしてやるしか無さそうです;

comment:14 in reply to: ↑ 13 ; follow-up: ↓ 15 Changed 11 years ago by Seasoft

nanasess への返信

Seasoft への返信 意図しないエンコーディングなのか, 正常な入力値なのにも係わらず検出できなかったエンコーディングなのか, 判別する術を思いつかなかったため, 文字エンコーディングが検出できなかった場合は, 無変換で通すようにしています.

「正常な入力値なのにも係わらず検出できなかったエンコーディング」というのは、どういった想定でしょうか?

mb_detect_encoding の第2引数を 'UTF-8' として、返り値が FALSE だったらエラーとするロジックはいかがでしょうか?

comment:15 in reply to: ↑ 14 ; follow-up: ↓ 16 Changed 11 years ago by nanasess

Seasoft への返信

「正常な入力値なのにも係わらず検出できなかったエンコーディング」というのは、どういった想定でしょうか?

あまり想定する範囲では無いかもしれませんが, mb_detect_order = auto かつ ISO-8859-1 なブラウザとか.

mb_detect_encoding の第2引数を 'UTF-8' として、返り値が FALSE だったらエラーとするロジックはいかがでしょうか?

環境によっては mb_detect_order の設定がまずくて, 文字エンコーディングを検出できないと, インストールした途端にエラー画面出まくりになってしまいそうです.

パラメータで設定できるようにしておくと良いかもしれませんね.

comment:16 in reply to: ↑ 15 ; follow-up: ↓ 17 Changed 11 years ago by Seasoft

nanasess への返信

あまり想定する範囲では無いかもしれませんが, mb_detect_order = auto かつ ISO-8859-1 なブラウザとか.

ちなみに、mb_detect_order = utf-8 ならば問題は発生しないという質のものでしょうか?

mb_detect_encoding の第2引数を 'UTF-8' として、返り値が FALSE だったらエラーとするロジックはいかがでしょうか?

環境によっては mb_detect_order の設定がまずくて, 文字エンコーディングを検出できないと, インストールした途端にエラー画面出まくりになってしまいそうです.

EC-CUBE が mb_detect_order に依存するのであれば、「インストールした途端にエラー画面出まくり」なのは正しい動作だと思います。むしろ、エラーも出ずに、暴走しつつ動作する方が深刻な問題だと思います。

また、EC-CUBE が mb_detect_order に依存するのであれば、EC-CUBE サイドで mb_detect_order を設定するのはいかがでしょうか?

パラメータで設定できるようにしておくと良いかもしれませんね.

それは良さそうですね。

comment:17 in reply to: ↑ 16 ; follow-up: ↓ 18 Changed 11 years ago by nanasess

Seasoft への返信

ちなみに、mb_detect_order = utf-8 ならば問題は発生しないという質のものでしょうか?

mb_detect_order = utf-8 だと SJIS や EUC-JP などが検出できなくなってしまい, 別の問題が発生しそうです.

EC-CUBE が mb_detect_order に依存するのであれば、「インストールした途端にエラー画面出まくり」なのは正しい動作だと思います。むしろ、エラーも出ずに、暴走しつつ動作する方が深刻な問題だと思います。

レンタルサーバーなどの多彩な環境で, できるだけ設定をせずに, エラー無くインストールできた方が, 一般ユーザーには好まれるので, あまり厳密な振舞いにはしたくないところです.

また、EC-CUBE が mb_detect_order に依存するのであれば、EC-CUBE サイドで mb_detect_order を設定するのはいかがでしょうか?

mb_detect_order は, CHAR_CODE の設定にも関係してくるので, EC-CUBE で mb_detect_order を決め打ちするのは心配です.

CHAR_CODE が UTF-8 固定なら問題は少なそうですが...

Changed 11 years ago by Seasoft

comment:18 in reply to: ↑ 17 ; follow-up: ↓ 19 Changed 11 years ago by Seasoft

nanasess への返信

ちなみに、mb_detect_order = utf-8 ならば問題は発生しないという質のものでしょうか?

mb_detect_order = utf-8 だと SJIS や EUC-JP などが検出できなくなってしまい, 別の問題が発生しそうです.

今のところ SJIS や EUC-JP などを検出する必要のあるケースに遭遇したことが無いのですが、需要はありそうでしょうか?
また、必要な場合は、mbstring.encoding_translation を有効にすれば済むようにも感じますが、EC-CUBE で独自に実装してカバーする必要はあるのでしょうか?

EC-CUBE が mb_detect_order に依存するのであれば、「インストールした途端にエラー画面出まくり」なのは正しい動作だと思います。むしろ、エラーも出ずに、暴走しつつ動作する方が深刻な問題だと思います。

レンタルサーバーなどの多彩な環境で, できるだけ設定をせずに, エラー無くインストールできた方が, 一般ユーザーには好まれるので, あまり厳密な振舞いにはしたくないところです.

インストールしてから (若しくは運用に回してから)、問題が発覚するほうが実害を伴うため、商用利用が前提であることを考えると信頼性に関わると考えます。

また、EC-CUBE が mb_detect_order に依存するのであれば、EC-CUBE サイドで mb_detect_order を設定するのはいかがでしょうか?

mb_detect_order は, CHAR_CODE の設定にも関係してくるので, EC-CUBE で mb_detect_order を決め打ちするのは心配です.

mb_detect_order を CHAR_CODE の値で設定すれば問題ないと推測しています。

CHAR_CODE が UTF-8 固定なら問題は少なそうですが...

そもそも、Ver 2系は、UTF-8 固定だと思っていました。

ちなみに、最新のコミュニティ版で CHAR_CODE を SJIS に設定したら、下記のようになりました。

comment:19 in reply to: ↑ 18 ; follow-up: ↓ 20 Changed 11 years ago by nanasess

Seasoft への返信

今のところ SJIS や EUC-JP などを検出する必要のあるケースに遭遇したことが無いのですが、需要はありそうでしょうか?
また、必要な場合は、mbstring.encoding_translation を有効にすれば済むようにも感じますが、EC-CUBE で独自に実装してカバーする必要はあるのでしょうか?

憶測ですが, その需要のために, 本チケットが登録されたのだと思います.

mb_detect_order を EC-CUBE 側で設定するか, CHAR_CODE は固定か否かは本筋から外れますので, 議論は別の機会にさせて頂くとして, 本チケットの修正が必要かどうかを株式会社ロックオンさんに判断して頂いた方がよろしいでしょうか.

comment:20 in reply to: ↑ 19 Changed 11 years ago by Seasoft

nanasess への返信

憶測ですが, その需要のために, 本チケットが登録されたのだと思います.

なるほど。私は「文字コードのチェックを行っていない為、 DBで下記のようなエラーが起こる場合がある」という部分から、DB 処理より前でチェックを行い、適切なエラー処理をすれば十分であり、DB 処理に到達させる必要は無いと解釈していました。

mb_detect_order を EC-CUBE 側で設定するか, CHAR_CODE は固定か否かは本筋から外れますので, 議論は別の機会にさせて頂くとして, 本チケットの修正が必要かどうかを株式会社ロックオンさんに判断して頂いた方がよろしいでしょうか.

そうですね。2.4.3 として改修する必要が無ければ、もう少し検討や検証に時間を頂き、満を持してリリースしたいところですね。

comment:21 Changed 11 years ago by nanasess

  • Owner changed from nanasess to kajiwara
  • Status changed from assigned to new

本チケットの修正が必要かどうか判断しかねるため, 一旦 kajiwara さんに投げます

comment:22 Changed 11 years ago by kajiwara

株式会社ロックオンの梶原です。

本件、色々なご意見ありがとうございます。

まず、本チケットに対する影響度はかなり低い方だと考えておりますので、方向性としては、一旦2.4.3のマイルストーンから外す、ということでいいと思います。
(本チケットに対する修正は、一旦本番用にはマージしないということで。)

また、本件の肝は、やはり、以下を解消することだと思いますので、その他の問題点は(色々考えられると思いますが)切り離して考えていく方がよさそうです。

元々, 海外のクローラなどが入力してくる文字列で, DB エラーが発生するという問題があるとのことで, このチケットが登録されたとのことなので

概ねアプリに対する影響というよりは、サーバ(及びサーバ管理者)に対する影響が問題であるように思いますので、できればサーバの負担にならない修正方法であれば、個人的にはありがたいと思います。

チケットがクローズにされなくても、本件のような事象があるということを書き留めておくだけでも有益だと思いますので、じっくり検討できればと思います!

comment:23 Changed 11 years ago by nanasess

  • Milestone changed from EC-CUBE2.4.3 to EC-CUBE2.4.4

一旦, milestone:EC-CUBE2.4.4 へ変更します

comment:24 Changed 10 years ago by nanasess

影響が大きそうですので、 milestone:EC-CUBE2.5.0 へ変更します

comment:25 Changed 10 years ago by nanasess

  • Milestone changed from EC-CUBE2.4.4 to EC-CUBE2.5.0

comment:26 Changed 10 years ago by nanasess

r18503:18505 の修正だと, Windows 環境で文字化けが発生することがあるようです. mb_detect_order の設定に依る可能性が高いですが, 念のため.

comment:27 Changed 10 years ago by nanasess

  • Milestone changed from EC-CUBE2.5.0beta to EC-CUBE2.6.0(仮)

comment:28 Changed 8 years ago by kajiwara

  • Owner changed from kajiwara to somobody
  • 修正済み unset

comment:29 Changed 8 years ago by h_yoshimoto

  • Milestone changed from EC-CUBE2.12.2 to EC-CUBE 2.12.3

comment:30 Changed 8 years ago by h_yoshimoto

  • Status changed from new to closed
  • Resolution set to 無効

こちら仕様という事で一旦クローズさせて頂きます。

comment:31 Changed 8 years ago by kim

  • 修正済み set
  • Milestone changed from EC-CUBE2.12.3 to EC-CUBE2.11.0

2.11.0リリース時点での仕様でFIXという事とします。

Note: See TracTickets for help on using tickets.