作った。
https://github.com/nishphp/phpstan-safestring-rule
この前htmlspecialcharsをチェックする拡張を作っていて、PDO->query()でも同じようなことがしたくなった。
なのでHTMLに限定せずに汎用的にチェックできるようにした。
例えば、
<?php
namespace App;
class ProductSearchForm
{
/** @var ?string */
public $name;
/** @var ?string */
public $code;
/** @var array<mixed> */
public $categoryIds;
/** @var ?string */
public $queryString;
}
こんなクラスにDB検索用のデータが入っていたとする。
<?php
namespace App;
use PDO;
class ProductDb
{
/** @var PDO */
private $pdo;
public function __construct(PDO $pdo)
{
$this->pdo = $pdo;
}
/**
* @return array<int,ProductDto>
*/
public function getProductList(ProductSearchForm $form): array
{
$conds = [];
$params = [];
if ($form->name !== null){
$conds[] = 'name = ?';
$params[] = $form->name;
}
if ($form->code !== null){
$conds[] = 'code = ?';
$parmas[] = $form->code;
}
if ($form->categoryIds){
$conds[] = sprintf('category_id in (%s)', implode(',', $form->categoryIds));
}
if ($conds)
$where = 'where ' . implode(' and ', $conds);
else
$where = '';
$sql = sprintf('select * from products %s', $where);
$stmt = $this->pdo->prepare($sql);
$stmt->execute($params);
$ret = $stmt->fetchAll(PDO::FETCH_CLASS, ProductDto::class);
if (!$ret)
return [];
/** @var array<int,ProductDto> $ret */
return $ret;
}
}
こんなDB処理を書く。あと.neonに設定を追記する。
そうすると
1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
------ ----------------------------------------------------------
Line ProductDb.php
------ ----------------------------------------------------------
42 PDO::prepare() Parameter #1 (string) is not safe-string.
------ ----------------------------------------------------------
[ERROR] Found 1 error
エラーを報告してくれる。
ちゃんとプリペアドステートメントを使ってるはずなのに…とか思ってたら、in (%s) は内容が不明な文字列を渡していたことに気付く。
それで、ProductSearchForm->$categoryIdsがmixedになっているのが良くないので、array<int>にする。
/** @var array<int> */
public $categoryIds;
そうすると
1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
[OK] No errors
PHPStanはパスする。
PHPStanは文字列の中の情報を保持しているので、内部的には普通のstringと$foo = 'my string';で作った文字列は別の型になっている。
それを利用して、プログラマーが自分で書いたであろう文字列だけ通過させるようなExtensionにした。
PHPStanの内部的な動作は
$conds = [];
//
// $conds: Constant Array ([])
//
if ($form->name !== null){
$conds[] = 'name = ?';
$params[] = $form->name;
}
//
// $conds: Union (Constant Array ([])|Constant Array ([0 => 'name = ?']))
//
if ($form->code !== null){
$conds[] = 'code = ?';
$parmas[] = $form->code;
}
//
// $conds: Union (Constant Array ([]) |
// Constant Array ([0 => 'name = ?']) |
// Constant Array ([0 => 'code = ?']) |
// Constant Array ([0 => 'name = ?', 1 => 'code = ?']))
//
このようになっている。変数の中の文字列パターンが全て分かっているので、ここまでの$conds変数は安全であることも分かる。
標準ではimplodeやsprintfの引数にconstantでない変数がある場合は標準化されて通常のarray<int,string>などになってしまう（constantのみの関数呼び出しならconstantのまま計算してくれる。ソースを読んで初めて気付いたすごい細かい設計）。
先程のsprintf('where id > %d', (int)$id); のような変換は、不明な文字列ではあるものの、HTMLテンプレートやSQLに使う文字列としては特に問題ないはずである。
$form->categoryIds は intの配列だから、それをimplodeしてsprintfしてもまあまあ安全だろう。
ということで、
$conds[] = sprintf(
'category_id in (%s)',
// constant-string
implode(',', $form->categoryIds)
// implode(constant-string, int|constant|union(constant|int|...)...) => safe-string
);
// sprintf(constant-string|safe-string, safe-string, ...) => safe-string
// $conds => array<int,safe-string>
stringよりは安全なsafe-stringという型に変換するようにした。
SQLを手で書く人はあまり居ないかもしれないけれど、他にもコマンドを実行したりファイルシステムのpathを指定したりするような箇所で使える。
ORMを使うとしても、念の為にIlluminate\Support\Facades\DB::select等に警告を設定しておけば知らないところで安全でないSQLが実行されることを防げる。
基本的な型の動作を変更するためにオーバーライドした関数は実験的だと言われたので、この先本体がバージョンアップしたらどうなるかは分からないところではあるけども。
あと余談。
型の自動解析をしていると、関数が長くなったからと言って細かく分けるのはやめた方が良いというような、コードの書き方にも影響が出てくる。
せっかく変数の中身が分かっているのに、関数を細かく分割すると変数の中身の情報が消えちゃう。
かと言って配列とUnion Typeの入り組んだ型をコメントで書くのはつらい。PHPStanのPHPDoc解析もそこまで細かくできないし。
自動で計算できるのならそっちの方がいい。