Watch out for Request
The Illuminate\Http\Request
class is awesome; this is known. Laravel added some magic in this class that extends from Symfony's request class. There are a few things to watch out for.
You don't get
it
Avoid the get
method. This is a method inherited from Symfony's class and doesn't act the same as Illuminates input
method, which is the documented and recommended way to get input from the request.
The input
method pulls different input sources together and pulls the input you want by name. The all
method calls the input
method to retrieve these inputs and adds the files
to it. This is important to know.
Validation will use $request->all()
to pull the inputs. So if you are doing some validation then after it you do $request->get(...)
you may be getting a value that is not the same as what was validated (based on the different input sources used).
The get
method is not documented in the Laravel docs and should not be used on Request
. The Request
and Input
facade both use the request
binding which is an instance of Illuminate\Http\Request
. The one difference is that the Input
facade has one real static method on it get
. This method is just calling $request->input(...)
.
Forget that Request@get
exists!
All the things are dynamic
The Illuminate Request
class has a nice __get
method which allows you to use dynamic properties to access request inputs. This isn't a bad thing, the only inputs this won't work as expected for are (attributes
, request
, query
, server
, files
, cookies
and headers
). These are real public properties that are inherited from Symfony's Request
class. PHP will always access publicly accessible properties; the __get
is never called for these as they exist.
The dynamic property works as long as you know this when using it. If it can't find any input matching what you are looking for it will fall back to trying to return a route parameter by that name.
Simple demonstration
To demonstrate this:
Route::get('test', function (Illuminate\Http\Request $request) {
dd(
$request->server,
$request->input('server'),
$request->attributes,
$request->input('attributes')
);
});
Then hit that route: http://yoursite.com/test?server=bob&attributes=tom
.
ServerBag {#45 ▼
#parameters: array:33 [▶]
}
"bob"
ParameterBag {#42 ▼
#parameters: []
}
"tom"
Security please
This is a big one for me. If you can, avoid using the dynamic property for retrieving route parameters from the request. It is the fallback if no inputs exist by the name.
$request->id;
That is going to look at the inputs returned by all
to see if one is named id
, if not it will fallback to trying a route parameter named id
.
I kinda hope you can see how this can be very dangerous, as it is not explicit. There is actually some security risk here if you aren't paying attention to what you are doing.
The example goes like this.
// some form request
public function authorize()
{
$somemodel = SomeModel::find($this->id);
// does the user own this record
return $this->user()->id == $somemodel->user_id;
}
// controller
public function edit(Request $request, $id)
{
$model = SomeModel::find($id);
....
}
You may think that you are getting the same id
in each of these but that may not be true at all.
The controller method is receiving the route parameter. In the FormRequest, using the dynamic property of Request, you are getting what? If there are no inputs named id
you are getting the route parameter presumably.
Now lets throw a wrench into the mix.
Simple DB layout:
posts
id user_id
---------------
1 1
4 2 <--- I am logged in as User 2
Lets hit that route:
http://yoursite.com/posts/1/edit?id=4
In this example Post with id 1
is what I want to edit. The ?id=4
, 4
is a Post I know that I own (different user than the user who owns Post 1
).
Now look what happens. The authorize
method will return true even though I don't own Post 1
because we are not checking Post 1
, we are checking Post 4
. This is because the dynamic property returns 'inputs' first then falls back to route parameters.
The controller doesn't know about this mistake, it is receiving the route parameter directly, so it recieves 1
for $id
. I just bypassed the authorization you have in place by simply passing along an input with the query string.
The correct way to avoid this problem is to be explicit with what you are requesting.
public function authorize()
{
$somemodel = SomeModel::find($this->route('id'));
// we are explicitly requesting the route parameter named 'id'
...
}
Seems trivial but you need to know what these methods are doing. Yes someone would need to know some things about your application potentially and how your route parameters are named, but this isn't something that someone couldn't just guess and check over and over and figure this out.
Simple demonstration
Simple demonstration of dynamic property pitfall vs explicitly grabbing the route parameter:
Route::get('testit/{id}', function (Illuminate\Http\Request $request, $id) {
dd($request->id, $request->route('id'), $id);
});
Lets hit that route: http://yoursite.com/testit/1?id=4
"4"
"1"
"1"
Food for thought
Be very aware of what 'magic' methods are doing. As long as you are aware of what is happening you should be okay. It is pretty simple to demonstrate these pitfalls.
Some github action about get
pitfalls:
laravel/docs PR - Add a note explaining the pitfalls of ->get #2269
Dynamic Property and Route Parameters:
laravel/docs PR (closed) - [5.2] Updated Requests - dynamic property #2030
Laravel 5.2 Docs - Requests - Retrieving Input - Dynamic Properties