search

sakura-editor / sakura

SAKURA Editor (Japanese text editor for MS Windows)
https://sakura-editor.github.io/
Other
1.26k stars 165 forks source link

デフォルトの文字コードが反映されない。SJISに設定してもUTF-8で読み込まれる。 #1418

Closed ghost closed 4 years ago

ghost commented 4 years ago

問題内容

BugReport/195

デフォルトの文字コードが反映されない。 XMLファイルの編集にて発生。 タイプ別設定でSJISをデフォルトに設定しても、毎回UTF-8で読み込まれてしまい、文字化けが発生する。 2.2.0.1より前のバージョンでは起きなかった。

encoding名がMS932など認識できないときにUTF-8に設定されるバグがありました -- Moca 2016-04-05 (火) 13:44:53

再現方法

  1. XML宣言におけるencodingに「ms932」や「sjis」などを記述し、UTF-8以外の文字コードで保存したものを用意します。
  2. サクラエディタを起動し、タイプ別設定にXML用の設定を作成します。 この時、文字コードとしてSJISを使うように指定しておきます。
  3. 作成したXML文書を開きます。

この手順において、当該ファイルはUTF-8で読み込まれます。 これを正しい文字コードで開き直した後は、履歴に残っている限り問題は発生しません。 なお、初回起動の状態でもUTF-8で読み込まれました。

再現頻度

SJISを使うように設定したXMLのタイプ別設定がある状態で、一度も開いたことのないXML文書を扱うときは必ず遭遇することになると思います。

問題の原因

XML宣言から文字コードを判別する処理に問題があると思われます。

当該ファイルを開く際の文字コード判別処理ではCESI::AutoDetectByXMLが実行されていました。 この中で読み取った文字列を文字コードの定義（encodingNameToCode）と比較する処理において、どの定義とも一致しなかった場合はループが終了してしまいます。 このループの後に実行されるコードは「encoding指定無しでxml宣言が終了した」と判断して呼び出し元にUTF-8である旨を返答しており、これによって誤った文字コードでファイルを開いていました。 また、「encoding指定が無い」XML文書はUTF-8またはUTF-16のどちらかとして取り扱うことがXMLの仕様書で規定されていますが、このコードはUTF-16である可能性を考慮していませんので、もしUTF-16で作成されながらencoding指定を省略していた場合（仕様上は可能です）、同様の問題が発生します。

環境情報

バージョン: 2.2.0.1

2.3.0.0 でも同じ症状が起きました。 -- anonymous 2016-04-05 (火) 10:59:32

手元では現時点で最新のmasterでも再現できました。

このバグに対するパッチについて

上記バグ修正→upatchid:1050。 xmlでencodingがない場合、通常の自動認識の優先度を上げました -- Moca 2016-04-05 (火) 13:54:10

sf.netには、本件に対するパッチとしてpatchunicode#1050がすでに提案されていますが、UTF-16でエンコーディング宣言を省略したパターンに対する考慮がありません。 また、文字コード名の別名を追加する変更も行われていますが、追加される別名はXMLの仕様的に正しくないようです。

なお余談になりますが、encodingNameToCodeには重複が1件（windows-1252）あります。patchunicode#1050ではさらに1件（shift_jis）増えています。

以上、BugReport/195及びpatchunicode#1050からの転記と、自分なりの調査の報告をさせていただきます。

ghost commented 4 years ago

追加される別名はXMLの仕様的に正しくない

本文に記載すると長くなりそうなので、コメントに分けました

patchunicode#1050を確認するにあたって、XMLの仕様を見てみました。 XML宣言におけるエンコーディングの宣言はチャプター4.3.3にあり、次の形式になっています。

EncodingDecl ::= S 'encoding' Eq ('"' EncName '"' | "'" EncName "'" ) EncName ::= [A-Za-z] ([A-Za-z0-9._] | '-')*

UTF-8/UTF-16のほかに、IANAが定義するものと"x-"接頭辞から始まるもので上記のフォーマットを満たしたものが使えるようです。 IANAの定義ではMIMEで使える名前とエイリアス が も含まれています。 また、エイリアスに含まれる"cs"で始まるものは、MIBで利用するためにRFC3808で定義されたという注釈がありました。MIBはSNMPなどで利用されるデータベースとのことです。 なお、HTML/CSSも以前はIANAの定義を参照していたようですが、W3Cの文書によると現在はWHATWGが提供している文字コードのリストを参照しており（リンク先のチャプター4.2にあります）、左側の列から選ぶようにと記述されていました。

IANAの定義とpatchunicode#1050の変更を突き合せたところ、「MS_Kanji」と"cs"で始まるもの以外は含まれていませんでした。 仮に追加するとした場合、少なくともIANAの定義かWHATWGのリストに含まれるものから選ぶべきではないかと思いました。

berryzplus commented 4 years ago

困ったこと

問題が 2つ 書いてあるように見えます。 タイトル通りの問題を解決する issue と捉えてよいでしょうか？ タイトルに書いてない問題もかなり重要な課題だと思うのでちょっと扱いに困る感じです。

問題1

デフォルトの文字コードが反映されない。SJISに設定してもUTF-8で読み込まれる。

原因： encodding="SJIS" がサポートされていないため。 対策１： ユーザーが、サポートされているエンコーディング名 Windows-31J に書き換える。 対策２： ユーザーが、サポートされているエンコーディング名 x-sjis に書き換える。 対策３： 開発者が、サポートするエンコーディングに SJIS を追加する。

普通に考えて、対策３の一択ですよね :smile:

問題2

encoding名がMS932など認識できないときにUTF-8に設定されるバグがありました

原因： エンコーディング名を認識できないときにエンコーディングを判定するロジックが誤っている？ 対策： 正しくはUTF-8かUTF-16のいずれかとして扱う仕様なので、ロジックを修正する。

No. encoding encoding別名 判定条件
1 UTF-16 UTF16LE 先頭2バイトが{ 0xff, 0xfe }と一致する
2 UTF-16 UTF16BE 先頭2バイトが{ 0xfe, 0xff }と一致する
3 UTF-8 UTF8BOM 先頭3バイトが{ 0xef, 0xbb, 0xbf }と一致する
4 UTF-8 UTF8N 上記以外

修正対象はこの関数なんですけど、全体的にリファクタリングしたほうが良さそうな雰囲気です。 https://github.com/sakura-editor/sakura/blob/8f58ec825d2cc29c192725b13e6820fd89718e8d/sakura_core/charset/CESI.cpp#L879-L884

修正するのは簡単だけど、レビューするのがしんどそう・・・

berryzplus commented 4 years ago

ん？ちょっと読み違っているかも。

2. この時、文字コードとしてSJISを使うように指定しておきます。

要するに、問題2であげたエンコーディングの判定表が正しくないですね :sob:

No. encoding encoding別名 判定条件
1 UTF-16 UTF16LE 先頭2バイトが{ 0xff, 0xfe }と一致する
2 UTF-16 UTF16BE 先頭2バイトが{ 0xfe, 0xff }と一致する
3 UTF-8 UTF8BOM 先頭3バイトが{ 0xef, 0xbb, 0xbf }と一致する
4 (設定に依存) - 上記以外、かつ、タイプ別設定にデフォルトエンコーディングがある
5 UTF-8 UTF8N 上記以外

現状、タイプ別設定はファイルの拡張子で行っていますが、XML文書内容に基づくエンコーディング判定の処理のコンテキストには解析中データのタイプ別設定を参照できません。

なのでまぁ、ファイルの拡張子に基づいてタイプ別設定のデフォルトエンコーディングを適用するのは、設計的に不可能です。

ghost commented 4 years ago

お疲れ様です。

タイトル通りの問題を解決する issue と捉えてよいでしょうか？ タイトルに書いてない問題もかなり重要な課題だと思うのでちょっと扱いに困る感じです。

自分はsf.netのページをそう解釈しました。

「SJISを設定しても」のくだりはタイプ別設定の画面の話だと思います。 タイプ別設定のウィンドウタブにデフォルトの文字コードを設定する箇所があり、ここでSJISを選択した、ということだと思いました。 既定は「改行コード：CR+LF」「文字コード：UTF-8」「BOMなし」「CPなし」「自動判別時にCESU-8を優先しない」が設定されています。 選択肢は、SJIS/EUC-JP/Latin1/UTF-16/UTF-16BE/UTF-8/CESU-8の7個です。 ここでSJISを選択しているのにUTF-8で読み込まれた、ということかと思います。 （なお、本文の再現方法はこれを踏まえてSJISを選ぶように書いています）

問題①については、追加対応で良いと思います。 記述を修正するというシチュエーションもあるので、IANA定義にないものも含めて、ありえそうなものを足しておきたいです。 ただ、IANA定義にあるのに追加されていないものがあるので、それをどうしたらいいか悩んでいます（数が多いです）。

berryzplus commented 4 years ago

現状、タイプ別設定はファイルの拡張子で行っていますが、XML文書内容に基づくエンコーディング判定の処理のコンテキストには解析中データのタイプ別設定を参照できません。

あ、持ってました。

https://github.com/sakura-editor/sakura/blob/8f58ec825d2cc29c192725b13e6820fd89718e8d/sakura_core/charset/CESI.h#L216-L217

ghost commented 4 years ago

問題②について（コメントを分けました）ですが、試したところUTF-16でencoding指定無しのパターンでは、懸念していた問題は発生しませんでした。自分の杞憂に終わったようです。

CESI::DetectUnicodeBomがCESI::AutoDetectByXMLよりも先に呼ばれており、BOMがある場合はこの段階でUnicodeであると決まってしまうので正しく処理されます。 BOMがない場合はCESI::AutoDetectByXMLに入りますが、突合せのループ処理に入らずに別の処理でやはりUnicodeと判定されていました。 このため、対応しないといけないのは「encoding指定がないか、あっても認識できない文字コードが指定されている場合で、Unicode以外の文字コードで作成されている」パターンだけのようです。

ちなみに、当時の担当者であるMocaさんの対処方法をパッチから読み取ると、次のようになっていました。

  1. 別名を追加する
  2. 認識できない文字コードの時はCODE_UTF8ではなくCODE_NONEを返す
  3. encoding指定がないときはCODE_AUTODETECTを返して自動判別処理を行うようにしたうえで、それでも決定できない場合に限りCODE_UTF8を返す

コードからだと、CODE_NONEのまま文字コード判定処理が終わってしまった場合は、デフォルト設定の文字コードが使われるように見えます。そうであればタイプ別設定の文字コードが反映されそうです。

berryzplus commented 4 years ago

ちなみに、当時の担当者であるMocaさんの対処方法をパッチから読み取ると、次のようになっていました。

  1. 別名を追加する
  2. 認識できない文字コードの時はCODE_UTF8ではなくCODE_NONEを返す
  3. encoding指定がないときはCODE_AUTODETECTを返して自動判別処理を行うようにしたうえで、それでも決定できない場合に限りCODE_UTF8を返す

コードからだと、CODE_NONEのまま文字コード判定処理が終わってしまった場合は、デフォルト設定の文字コードが使われるように見えます。そうであればタイプ別設定の文字コードが反映されそうです。

この内容で問題なさそうに思います。

ghost commented 4 years ago

追加する別名ですが、

  1. サクラエディタで保存時に指定できる文字コード
  2. IANAのリストとWHATWGのリストのどちらかにある

とりあえずこの2点を満たすものを抽出してみました。それでも39個あります。

berryzplus commented 4 years ago

追加する別名ですが、

  1. サクラエディタで保存時に指定できる文字コード
  2. IANAのリストとWHATWGのリストのどちらかにある

とりあえずこの2点を満たすものを抽出してみました。それでも39個あります。

39個くらいならそのまま入れてもいいんじゃないかと思います。

IANAのリスト ⊂ WHATWGのリスト という関係（IANAのリストはWHATWGのリストに含まれる）で、 IANAのリスト ⊂ Windowsのコードページリスト という関係（IANAのリストはWindowsのコードページリストに含まれる）だったと思います。

問題はサクラエディタが IANAのリスト にも Windowsのコードページリスト にも存在しないコードページ CESU-8 に対応している（？）ことで、サクラエディタがサポートするコードページの範囲が不当に狭く見えてる気がしています。

サクラエディタには、CPチェックボックスにより Windowsのコードページリスト に含まれるすべてのコードページに対応することができます。なので、エンコーディング名を Windowsのコードページ に変換することができれば、かなりたくさんのエンコーディングに対応することができるはずです。

ghost commented 4 years ago

パッチを読み返したらshift_jisは重複していませんでした。すみませんでした。 （今更ながら訂正を本文に反映しました。）

CPの機能を知らなかったので、いろいろと試してみました。 その過程でISO-8859-1に28591番が割り当てられていることに気が付きました。 そこで、CODE_LATIN1は1252と28591のどちらを指しているのか気になっています。 encodingNameToCode上ではwindows-1252が次のように記述されています。

{ "windows-1252", 12, CODE_LATIN1 },
（中略）
{"windows-1252",  12, 1252},

実際にwindows-1252という宣言に遭遇した場合、CODE_LATIN1と判定されるようです。

berryzplus commented 4 years ago

そこで、CODE_LATIN1は1252と28591のどちらを指しているのか気になっています。

Windows-1252（＝ISO-8859-15、ISO-8859-1の改訂版）です。 https://github.com/sakura-editor/sakura/blob/8f58ec825d2cc29c192725b13e6820fd89718e8d/sakura_core/charset/CLatin1.cpp#L80-L81

encodingNameToCode上ではwindows-1252が次のように記述されています。

{ "windows-1252", 12, CODE_LATIN1 },
（中略）
{"windows-1252",  12, 1252},

実際にwindows-1252という宣言に遭遇した場合、CODE_LATIN1と判定されるようです。

この記述の意味は、わずかに異なります。

CODE_LATIN1 と判定した場合、 Latin1変換専用クラスである CLatin1 が使われます。 1252 と判定した場合、汎用コードページ変換クラスである CCodePage が使われます。

両者の違いは、独自にカスタムした変換を使うかどうかで、実際どっちが速いのか？はぼくも知らないっす。

k-takata commented 4 years ago

Windows-1252（＝ISO-8859-15、ISO-8859-1の改訂版）です。

Windows-1252 は ISO-8859-15 とは異なります。

berryzplus commented 4 years ago

Windows-1252（＝ISO-8859-15、ISO-8859-1の改訂版）です。

Windows-1252 は ISO-8859-15 とは異なります。

https://ja.wikipedia.org/wiki/Windows-1252 https://ja.wikipedia.org/wiki/ISO/IEC_8859-15

符号位置が異なるのでベツモノらしいです。

ghost commented 4 years ago

1422 を提出しました。

ghost commented 4 years ago

タイプ別設定にある文字コード設定が反映されるようにする変更を、#1428で提出しました。

ghost commented 4 years ago

ご対応ありがとうございました。