Permalink
Browse files

BACKWARDS INCOMPATIBLE change: when using BaseEntity::set() a whitelist

of fields is mandatory. Look into the "Field whitelisting in
BaseEntity::set()" section of the documentation.
  • Loading branch information...
1 parent dde0694 commit 56d45cf659e76d84cc4747af50de71eb03060106 @mariano committed Mar 5, 2012
Showing with 42 additions and 2 deletions.
  1. +32 −1 README.md
  2. +10 −1 models/BaseEntity.php
View
@@ -258,9 +258,12 @@ You should also note that `BaseEntity` provides a method named `set()` which
comes very handy if the user data is to be populated from a form submission.
If so, the above code could be rewritten as:
+> You may notice that we send a list of field names as the second argument to
+> the `set()` method. More about this in the section *Field whitelist in set()*
+
```php
$user = new User();
-$user->set($this->request->data);
+$user->set($this->request->data, array('name', 'email'));
try {
$em->persist($user);
@@ -284,6 +287,34 @@ errors:
<?php echo $this->form->end(); ?>
```
+#### Field whitelist in set() ####
+
+In the preceeding example, we shown the `set()` method, inherited from
+`BaseEntity`, as a convenient way to populate entity fields as a result of a
+form submission. As part of the shown `set()` usage, you may have notice a
+list of fields passed on the second argument:
+
+```php
+$user->set($this->request->data, array('name', 'email'));
+```
+
+This argument is a whitelist of fields, that specifies which fields that are
+part of the first argument (`$this->request->data` in this case) are allowed to
+be set on the entity.
+
+I've struggled against not including the whitelist argument with the idea that
+security should be enforced in the application logic. However, recent events
+in the [rails arena] [rails-fiasco] convinced me that my original intention
+of forcing a whitelist has more advantages than disadvantages.
+
+In any case, if you wish to avoid setting a whitelist, you can pass an empty
+array on the second argument, and `false` to the third argument. So the
+example below would be changed to:
+
+```php
+$user->set($this->request->data, array(), false)
+```
+
# Extensions #
li3\_doctrine2 also offers a set of extensions to integrate different parts
View
@@ -155,10 +155,19 @@ public function validates(array $options = array()) {
*
* @see IModel::validates()
* @param array $data An associative array of fields and values to assign to this instance.
+ * @param array $whitelist Fields to allow setting
+ * @param bool $useWhitelist Set to false to ignore whitelist
+ * @throws Exception
*/
- public function set(array $data) {
+ public function set(array $data, array $whitelist = array(), $useWhitelist = true) {
if (empty($data)) {
return;
+ } elseif ($useWhitelist && empty($whitelist)) {
+ throw new \Exception('Must set whitelist of fields');
+ }
+
+ if ($useWhitelist) {
+ $data = array_intersect_key($data, array_flip($whitelist));
}
foreach($data as $field => $value) {

0 comments on commit 56d45cf

Please sign in to comment.