Added generic filtering to ParameterBag. #2261
Conversation
Adds filtering convenience using PHP's filter_var() e.g. `$request->get->filter($key, '', false, FITLER_SANITIZE_STRING);` See http://php.net/manual/en/filter.filters.php for capabilities.
|
What is the use case ? |
|
Input variable validation/sanitization. ParameterBag has a few built in like |
|
Can you add some unit tests for this new feature? |
|
Sure thing. |
|
Before I make the commit, is the method name ok for you or would you prefer it is called |
|
|
|
I've added some tests. |
|
@Drak IMO you must check that user don't use unknown filter and/or flags for filter. |
|
@stloyd - I'm not sure that's practical at all, this is a wrapper for a built-in PHP function and I don't understand why we would need validate arguments for a PHP function - it's the coder's job to use the API correctly - none of the inputs to this function are coming from a web request. It would also mean that the API would need to keep track of any upstream changes to constants in the PHP engine (which are just integers after all). It's really just not practical. |
| * | ||
| * @return mixed | ||
| */ | ||
| public function filter($key, $default = null, $deep = false, $filter=FILTER_DEFAULT, array $options=array()) |
There was a problem hiding this comment.
$options could be a mixed type
There was a problem hiding this comment.
yes, the phpdoc must be fixed and the array type hint removed.
|
@Drak it's could be cool to use |
| } | ||
|
|
||
| return filter_var($value, $filter, $options); | ||
| } |
There was a problem hiding this comment.
I was mulling over that earlier today and came up with the following:
public function filter($key, $default = null, $deep = false, $filter=FILTER_DEFAULT, $options=array())
{
$value = $this->get($key, $default, $deep);
// Always turn $options into an array - this allows filter_var option shortcuts.
if (!is_array($options) && $options) {
$options = array('flags' => $options);
}
// Add a convenience check for arrays.
if (is_array($value) && !isset($options['flags'])) {
$options['flags'] = FILTER_REQUIRE_ARRAY;
}
return filter_var($value, $filter, $options);
}
This keeps the convenience of being able to handle arrays simply but the other alternative would be simply this:
public function filter($key, $default = null, $deep = false, $filter=FILTER_DEFAULT, $options=array())
{
return filter_var($this->get($key, $default, $deep), $filter, $options);
}
Let me know your preferences.
There was a problem hiding this comment.
I like the convenience check of the first proposal.
|
@stealth35 regarding this I believe strongly in the use of IDEs when coding and autocomplete nicely provides when you type |
- Simplified logic of tests. - Added more comments/docblocks. - Added more convenience.
|
Ok check it now. |
Commits ------- c4a0f79 Updates according to suggestions. 6aec789 Added tests. 54454ba Added generic filtering to ParameterBag. Discussion ---------- Added generic filtering to ParameterBag. Adds filtering convenience using PHP's filter_var() e.g. $request->get->filter($key, '', false, FITLER_SANITIZE_STRING); See http://php.net/manual/en/filter.filters.php for capabilities. --------------------------------------------------------------------------- by GromNaN at 2011/09/25 15:41:50 -0700 What is the use case ? --------------------------------------------------------------------------- by drak at 2011/09/25 15:52:19 -0700 Input variable validation/sanitization. ParameterBag has a few built in like `getAlnum()` for example. This method offer's PHP's full filtering and sanitization suite. --------------------------------------------------------------------------- by fabpot at 2011/09/27 00:56:41 -0700 Can you add some unit tests for this new feature? --------------------------------------------------------------------------- by drak at 2011/09/27 00:58:56 -0700 Sure thing. --------------------------------------------------------------------------- by drak at 2011/09/27 01:07:03 -0700 Before I make the commit, is the method name ok for you or would you prefer it is called `getFiltered()`? --------------------------------------------------------------------------- by fabpot at 2011/09/27 01:13:46 -0700 `filter` sounds good to me. --------------------------------------------------------------------------- by drak at 2011/09/27 02:37:01 -0700 I've added some tests. --------------------------------------------------------------------------- by stloyd at 2011/09/27 02:42:42 -0700 @Drak IMO you must check that user don't use unknown filter and/or flags for filter. --------------------------------------------------------------------------- by drak at 2011/09/27 02:48:38 -0700 @stloyd - I'm not sure that's practical at all, this is a wrapper for a built-in PHP function and I don't understand why we would need validate arguments for a PHP function - it's the coder's job to use the API correctly - none of the inputs to this function are coming from a web request. It would also mean that the API would need to keep track of any upstream changes to constants in the PHP engine (which are just integers after all). It's really just not practical. --------------------------------------------------------------------------- by stealth35 at 2011/09/27 05:16:50 -0700 @Drak it's could be cool to use `filter_id` ✌️ if (is_string) { $filter = filter_id($filter); } --------------------------------------------------------------------------- by drak at 2011/09/27 07:05:42 -0700 @stealth35 regarding this if (is_string) { $filter = filter_id($filter); } I believe strongly in the use of IDEs when coding and autocomplete nicely provides when you type `FILTER_`. Additionally, `filter_id()` only works on filters, but not for the flags, so I'm not entirely sure how useful it would be overall compared to using a good IDE (which you need when working with complex frameworks anyhow, imo :) --------------------------------------------------------------------------- by drak at 2011/09/27 07:30:10 -0700 Ok check it now.
|
Would be nice to have the method added to the documentation as well: http://api.symfony.com/2.0/Symfony/Component/HttpFoundation/ParameterBag.html |
|
@datibbaw It is in there. Your issue is that you are searching a 2.1+ method in the API doc for the 2.0 code. |
|
@stof You're absolutely right, my bad; documentation noob mistake :( |
Adds filtering convenience using PHP's filter_var() e.g.
See http://php.net/manual/en/filter.filters.php for capabilities.