[凍結中] サクラエディタのコマンドライン引数に超長い文字列を指定するとクラッシュする問題に対処する。

[berryzplus]

PR の目的

サクラエディタのコマンドライン引数に超長い文字列を指定するとクラッシュする問題(#1406)に対処します。

途中で方針を変えて、最終的な仕様は「超長い文字列を指定した場合、エラーメッセージを表示して、その引数はなかったものとして続行する」としました。

カテゴリ

• 不具合修正
• リファクタリング

PR の背景

#1406 の対策 PR です。

PR のメリット

• コマンドラインで超長いパスを指定したときにクラッシュする（≒何もメッセージを出さずに異常終了する）不具合を解消できます。

PR のデメリット (トレードオフとかあれば)

• コマンドライン解析処理に スマートポインタ が導入されます。
• 「ファイルパスが長過ぎるエラー」（＝例外クラス）が追加されます。
• この PR が実現するのは、異常検出時に正しく異常終了させることなので サクラエディタのコマンドライン引数でファイル名がMAXPATH超に長すぎるとうんともすんとも言わずに落ちて消える #1406 で期待された挙動の要件（＝発生したエラーを無視して続行させたい）を満たしません。
• 「ファイルパスが長過ぎるエラー」の例外クラスを追加しますが、例外としては利用しません。
• 既存コードに存在していた、超分かりづらい仕様を一部リファクタリングして単純化しているので、なんか間違っている可能性があります。

仕様・動作説明

サクラエディタは_MAX_PATH(259字) を超えるファイルパスをサポートしていませんが、サポートされない超長いパスが指定されたときにどうなるかが定義されていませんでした。もともと一部のパス処理用の独自関数には定義があるのですが、肝心のコマンドライン引数の解析処理で「超長いパス」を指定された場合の挙動が未定義でした。このため、現状で「超長いパス」を指定した場合、問答無用でクラッシュしていまいます。

この PR ではとりあえずの対処として、異常を検知したらメッセージを出して終了するようにします。

この PR ではコマンドライン文字列の解釈のうち オプション以外の解釈方法 を変更します。
コマンドラインのオプション部分についてはヘルプが存在します。
コマンドラインのオプション以外の部分についてはヘルプがありませんが、このPRで補完する意図はありません。

大前提

世間一般の話として、プログラムはコマンドラインから起動することができます。
コマンドラインからプログラムを起動する場合、オプションを指定することができます。
指定したオプションをどのように解釈するかは、プログラム側の実装に依存します。

• 世間一般的には、コマンドラインを 半角スペース ' ' で区切って分割したものが、定義済みのコマンドラインオプションと一致したら「そのオプション」が指定されたと見做すものが多いと思います。C Runtime の エントリポイント main のプロトタイプ定義が int main(int argc,char**argv) がそんな感じの実装になっているためです。正確には、起動するコマンドシェルの仕様により空白を含んだパスを1つの引数として渡すこともできるんですが、意識して実装しなければこのパターンになります。
• 世間一般的には、指定されたコマンドライン文字列を独自で解釈したい要求がないのか？というとそんなことはないです。UNIX系文化には古来より yacc という文法解析器生成ツールがあり、コマンドライン解析に特化したライブラリ getopt があるくらいに、そういう需要は多いです。
  サクラエディタのコマンドライン解析も独自になっていますが、実装は完全独自になっています。
• このPRで修正しようとしているのは 独自がコマンドライン解析 の実装部分です。
  

  sakura/sakura_core/_main/CCommandLine.cpp

  Line 243 in 8f58ec8

  void CCommandLine::ParseCommandLine( LPCWSTR pszCmdLineSrc, bool bResponse )

サクラエディタのコマンドライン解析仕様

1. コマンドラインが 半角ハイフン '-' 以外で始まっている場合、コマンドラインの先頭部分を「空白を含むファイルパス」として解釈できないか検証して、存在したファイルパスと認識できた部分をファイル名と解釈して残りのコマンドライン解析から除外します。
2. コマンドラインは 半角スペース ' ' で区切って引数の集まりに分割します。
   ただし、引数の先頭が 半角二重引用符 '"' で始まる場合、 半角二重引用符 '"' に続く 半角スペース ' ' だけを次の区切りとします。
3. 半角ハイフン '-' または 文字列 "\"-" で始まる引数は「オプション」と解釈されます。
   ただし、引数 "--" が登場したあとはすべての引数が「非オプション」と看做されます。
4. 「オプション」と解釈された引数が認識できなかった場合、引数は無視されます。
5. (今回廃止する仕様)
   「非オプション」と解釈された引数にファイル名に利用できない文字が含まれていた場合、メッセージを表示して引数を無視して続行します。メッセージボックスのタイトルは "FileNameError" として表示します。(標準動作を上書き)
6. (今回追加する仕様)
   「非オプション」と解釈された引数にファイル名に利用できない文字が含まれていた場合、エラーメッセージを表示して引数を無視して続行します。
7. (今回追加する仕様)
   「非オプション」と解釈された引数が_MAX_PATH(259文字)超の超長いパスだった場合、エラーメッセージを表示して引数を無視して続行します。
8. コマンドラインに含まれる「非オプション」のうち、最初に見つかったファイルパスをファイル名と解釈します。

テスト内容

テスト1

_MAX_PATH(259字) を超える引数を指定したときに異常終了すること。

手順

1. sakura.exe "12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890" xxxxx を実行する。(1つ目の引数は260字)
2. "12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890x\nというファイルを開けません。\nファイルのパスが長すぎます。" と表示されてプログラムが終了すること。(1つ目の引数260字がファイル名に埋め込まれる)

テスト2

_MAX_PATH(259字) を超えない引数を指定したときに正しく起動できること。

手順

1. sakura.exe を普通に起動する。

PR の影響範囲

• サクラエディタの起動パラメータ（≒コマンドライン引数）の解析にルールを追加する変更です。
• コマンドラインからの起動に影響します。
• Windows エクスプローラーからの起動に影響します。（コマンドラインからの起動と実体は同じもの）
• Windows API からの起動に影響します。（コマンドラインからの起動と実体は同じものですが、最大32,766文字を指定できるやり方です）

関連 issue, PR

#1406

参考資料

https://docs.microsoft.com/ja-jp/cpp/c-runtime-library/reference/strcspn-wcscspn-mbscspn-mbscspn-l?view=vs-2019
https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw

[AppVeyorBot]

✅ Build sakura 1.0.3124 completed (commit 8840617371 by @berryzplus)

[usagisita]

PR自体は、非常にうれしいです。ありがとうございます。
なかなかいい感じですが、細かいところは、ちょっとおしいと思います。

[beru]

追加されたローカル変数の型が int なのが気になりました。size_t の方が良いのではないかと思います。

[AppVeyorBot]

✅ Build sakura 1.0.3125 completed (commit e5f5e45f9d by @berryzplus)

[AppVeyorBot]

✅ Build sakura 1.0.3126 completed (commit 206d4b918d by @berryzplus)

[usagisita]

焦らず元コードを解析しましょう。
上の削除したコードは、""で囲われていないスペースのあるパス名を開くのに必要な処置です。削除すると互換性うんちゃらでほぼ確実に困ります。
szPathは削除された上側と、my_strtok以降の部分で、データの中身は共有されていません。
(だからszPathはmy_strtokの上と下で別変数にできます)
fexistは上でするから意味があります。下に追加したfexistは何がしたいか不明です。

[berryzplus]

上の削除したコードは、""で囲われていないスペースのあるパス名を開くのに必要な処置です。削除すると互換性うんちゃらでほぼ確実に困ります。

意味が分からなかったので試してみました。

sakura.exe C:\Program Files\sakura\warning-alpha.txt

• 説明： ""で囲われていないスペースのあるパス名 を指定している。
• 想定： C:\Program と Files\sakura\warning-alpha.txt という存在しない2つのパスを指定したと解釈される。
• 結果： 存在するパスが指定されてないのでパスは 2つとも m_vFiles に入り、ブラクラ状態になる・・・（オイ

というわけで、対策入れました。 233247d

fexistは上でするから意味があります。下に追加したfexistは何がしたいか不明です。

コマンドラインは 半角スペース ' ' で区切って引数の集まりに分けられます。
半角ハイフン '-' または 文字列 "\"-" で始まる引数はオプションと解釈されます。
オプションでない引数はファイル名と解釈されますが、正規ルートで開けるファイルはたった1つだけです。

下に追加した fexist は、削除したループにあった仕様「ファイル名と解釈された引数のうち最初に見つかった存在するファイルパスを m_fi.m_szPath に格納する」を引き継いだものです。

233247d の対策は、1つも存在するパスがなかった場合に自身が開くファイルパスが空になりすべて「その他ファイル」（＝別プロセスで起動）に流れてしまっていたのを、自身が開くファイルパス側に回収する処理を追加するものです。

[berryzplus]

あ、いかん。

[berryzplus]

すべてのファイルパスがレスポンスファイルで指定され、かつ、すべて存在しないファイルパスだった場合の対策ができていなかったので処理位置を修正しました。

[AppVeyorBot]

✅ Build sakura 1.0.3127 completed (commit 3059f15c7d by @berryzplus)

[AppVeyorBot]

✅ Build sakura 1.0.3128 completed (commit e1f9a502aa by @berryzplus)

[berryzplus]

PRの構成を組みなおしたくなったので一旦Draftにしました。

[AppVeyorBot]

✅ Build sakura 1.0.3130 completed (commit a837a5ddec by @berryzplus)

[usagisita]

もはやこのPRは、名が体を表していません。
僕が報告、お願いしたいもの→MAX_PATHを超えた場合の対処のみ
このPRの現状→仕様変更を含む、様々な変更
こうなっちょります

[berryzplus]

C:\> echo > "test file.txt"
C:\> sakura.exe test files.txt

こんな仕様しりませんでした。

[AppVeyorBot]

✅ Build sakura 1.0.3143 completed (commit 2e0f1ef016 by @berryzplus)

[berryzplus]

ここの実装が QuotedTooLongFilePath と異なっている理由は、まだ不具合が残っているからです。

• ファイルパスと解釈された引数は、最大 259 文字までの配列に格納される。
• ファイルパスと解釈された引数が絶対パスでない場合、カレントディレクトリのパスと結合されてフルパスにされる。この時に利用する一時バッファも最大 259 文字までの配列なので、カレントディレクトリのパス長とファイル名の長さの合計が 258 文字を超える場合バッファが溢れて正しく処理できない。

この不具合を修正するのはちょっとしんどいので、一旦放置してあります。

[berryzplus]

ちょっとしんどいんだけど、解決できないか対処方法を模索中・・・。

[AppVeyorBot]

✅ Build sakura 1.0.3148 completed (commit 3ed27df887 by @berryzplus)

[AppVeyorBot]

✅ Build sakura 1.0.3155 completed (commit 2bcbffe55a by @berryzplus)

[AppVeyorBot]

✅ Build sakura 1.0.3157 completed (commit 166137a5eb by @berryzplus)

[berryzplus]

関連する修正が多過ぎてマージ困難と判断したので、このPRは凍結します。

[berryzplus]

ちなみにこの件は、まだ完全には直っていない認識です。