TL; DR
実際に業務で発生した怖い話。
-
request()
やRequest::__get()
では外部入力とルートパラメータが競合し,意図しない結果を取得してしまうことがある -
Request::input()
とRequest::route()
を使い分けておけば間違いはない
実際のケース
前提
以下のように,すべてのフィールドを受け取る前提でユーザ情報を更新するAPIエンドポイントがあったとする。入力形式はJSONで,フィールドを user
というキーでラップする構造である。
import axios from 'axios'
const id = 1
const params = {
user: {
name: 'John',
description: 'Hello',
password: '', // パスワードは変更しない場合は空文字列
},
}
axios.put(`/users/${id}`, params)
しかし,フロントエンドのバグで実際には
const params = {
user: {
name: 'John',
description: 'Hello',
},
}
というパラメータになってしまっていた。何故か password
が送信されていなかった。
…そして,Laravel 側ではルートモデルバインディングを使って以下のようにリクエストを受け取っていた。
class UserController extends Controller
{
public function update(User $user)
{
$name = request('user.name');
$description = request('user.description');
$password = request('user.password');
if (strlen($password) > 0) {
$user->password = Hash::make($password);
}
$user->fill(compact('name', 'description'))->save();
}
}
問題と原因
さて,フロントエンドだけに留まらず,このバックエンドのコードには致命的なバグがあった。なんと,
$password = request('user.password');
の中身がこの時点でハッシュ値になっていたのである!そもそも送信されてきてもいないし,ハッシュ関数も通していないのに,いったいなぜ…!?
原因は,request()
関数が Request::__get()
を経由し,Request::input()
相当だけに留まらず,Request::route()
にフォールバックしていたことにあった。
/**
* Get an input element from the request.
*
* @param string $key
* @return mixed
*/
public function __get($key)
{
return Arr::get($this->all(), $key, function () use ($key) {
return $this->route($key);
});
}
基本は Request::all()
から取ってこようとするが,値が存在しなかった場合には Request::route()
にフォールバックするコードになっていたのだ。
入力のキーは "user.password"
という文字列であった。内部で使用されている Arr::get()
はドットチェーンを解釈して展開する。パスワードはそのキー自体が存在しなかったため,**ルートモデルバインディングの対象になっていた $user->password
から引っ張り出されてきたのだ。**なんてこった。
対策
Request::input()
と Request::route()
を厳密に使い分けておくのが一番よい。
class UserController extends Controller
{
public function update(Request $request, User $user)
{
$name = $request->input('user.name');
$description = $request->input('user.description');
$password = $request->input('user.password');
if (strlen($password) > 0) {
$user->password = Hash::make($password);
}
$user->fill(compact('name', 'description'))->save();
}
}
それでも使いたいという場合,今回のようにルートモデルバインディングのキーと絶対に被らないように細心の注意を払わなければならない。