ファイルパスに「ファイルに使えない文字」を含めた場合の処理を改善する

[berryzplus]

PR の目的

ファイルパスに「ファイルに使えない文字」を含めた場合の処理を改善します。

カテゴリ

• リファクタリング

PR の背景

#1420 の修正の一部切り出しです。

問題箇所があまりにも多過ぎるので、普通に理解できそうな小分けのブロックを先出しでPR作成します。
このPRで修正する内容は、 #1283 で解決したい問題を結果的に解決します。

このPRで解決する問題は2つです。

1. パスに「ファイル名に使えない文字」を含んでいたときのメッセージを組み立てるのに wsprintf を使っているが、一時バッファの領域確保で埋め込み文字列の長さを考慮していないので状況によってバッファオーバーフローが起きる可能性がある。
2. パスに「ファイル名に使えない文字」を含んでいるかどうかをチェックする関数の実装が微妙。

「不具合対策」とすると説明がうっとおしいので、「ただのリファクタリング」として修正します。

PR のメリット

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

仕様・動作説明

Windowsには「ファイル名に使えない文字」があります。
サクラエディタもこれに対応していて、コマンドラインから指定したファイル名が「ファイル名に使えない文字」を含んでいた場合には警告メッセージが出るようになっています。
このPRでは、仕様・動作を変更しません。

テスト内容

新規関数のテストケースを作成しています。
新規関数を利用するコマンドライン解析のテストケースを追加しています。
したがって、追加のテストを実施する必要はないと思われます。

PR の影響範囲

コマンドライン解析の結果に影響する変更です。

関連 issue, PR

#1420
close #1283

参考資料

[AppVeyorBot]

✅ Build sakura 1.0.3215 completed (commit ef1347e2fe by @berryzplus)

[AppVeyorBot]

✅ Build sakura 1.0.3218 completed (commit 727b07d030 by @berryzplus)

[beru]

パスからファイル名部分を抽出する記述は他の場所でもあったので関数化した方が良いと思います。
shlwapi.h に PathFindFileNameW という関数があるのでそれでも良い気がしますがどうでしょうか？

[berryzplus]

PathFindFileNameは : をパス区切りとして扱うので使えませんでした。

const auto szPath = L"localhost:8080";
const auto pszFile = ::PathFindFileNameW(szPath);
//pszFile is "8080".

[berryzplus]

こっちは対応すると仕様を満たせないので対応しないです。

[beru]

localhost:8080 こういうのも扱う必要があるんですね、知りませんでした…。

既存の関数が仕様を満たせないので使えないとしても、同じ記述が他の場所でも追加されたような気がするので別途関数化はした方が良い気がします。というかファイルパスからファイル名を取り出す記述って色々な場所である気がしますが、全体的に統一されてるんでしょうか？

ここについてはコマンドラインに関するところで他の場所の処理とはまた話が違うのでしょうか？

[berryzplus]

localhost:8080 こういうのも扱う必要があるんですね、知りませんでした…。

既存の関数が仕様を満たせないので使えないとしても、同じ記述が他の場所でも追加されたような気がするので別途関数化はした方が良い気がします。というかファイルパスからファイル名を取り出す記述って色々な場所である気がしますが、全体的に統一されてるんでしょうか？

ここについてはコマンドラインに関するところで他の場所の処理とはまた話が違うのでしょうか？

localhost:8080 で分かりづらければ、C:\work\test:001.txt とかでも良いです。
この関数は、「ファイル名に使えない文字」を検出する関数なので : は検出できる必要があります。
コマンドラインの処理だから、は関係ないです。

他の処理で冗長な _wsplitpath を使っていて非効率なのは知っています。
ここで話を広げても論点がボヤけて一歩も進めなくなるだけなのでスルーしときます。

[berryzplus]

既存の関数が仕様を満たせないので使えないとしても、同じ記述が他の場所でも追加されたような気がするので別途関数化はした方が良い気がします。というかファイルパスからファイル名を取り出す記述って色々な場所である気がしますが、全体的に統一されてるんでしょうか？

全体的に統一されてるか？って観点で見ると、サクラエディタはたぶん０点でしょう。

統一できてない理由としては、次のようなことを考えられます。

• 単体テストがないので、実装済みの関数が使い物になるかどうか判断できない
• 自分が触る箇所以外に影響を与える変更を、できればしたくない

[AppVeyorBot]

✅ Build sakura 1.0.3219 completed (commit 559ee14b0a by @berryzplus)

[sanomari]

問題なさそうに思います。

[berryzplus]

レビューありがとうございます。マージしちゃいます。

[usagisita]

一足先にマージされてしまいましたが、一応コメントしておきます。

NTFSには代替データストリーム（ADS）というものがあり、ファイル名またはフォルダ名に : を続けてストリーム名を指定するものがあります。
localhost:8080 も C:\work\test:001.txt もNTFSであれば、パスとしては正しいです。
この場合.txtは拡張子ではなく、testというファイルの「001.txt」というストリームです。
実際に相対パスのlocalhostや絶対パスでtestというファイルが実在するなら、どちらもメモ帳やサクラエディタで読み書き可能です。
ただし問題はあって、ファイルダイアログでは代替データストリームを正しく扱うことができないんじゃなかったかなと思います。
代替データストリームは、ネットからダウンロードしてきたファイルをマークしたりするZoneIDなどで使われています。
これを「封印」することによって、どれくらいの人が実際に困るかはわからないけれど、今まで一応読み書き可能であったものが、パス指定で開けなくなるんじゃないかな、と思います。

sakura/sakura_core/charset/charcode.h

Line 181 in 8acb720

static const wchar_t* table = L"<>?\"|*";

今までの動作では、メッセージでは:がファイル名に使えないと表示されていますが、実際の処理では:をチェックしていません。
OS側の処理として、ドライブレター以外のコロンが2つあったり、パスの途中にあったりした場合はどうなのかとかは、ちょっと知らないです。