Skip to content

Commit

Permalink
User selector goes wrong in user id validation
Browse files Browse the repository at this point in the history
Under some circumstances the "User selector" implicitly assumes that "no selection" means "user id = 0", which is a valid value to be saved in the database.
This brings to paradoxes, for example you can inserts records with user id = 0 even when the "user id" is a required filed.
For example: Go to User > User Notes and create a new note. There is no user selected, but it is a required field, as shown by the asterisk. Fill out "Subject" and "Note", but leave empty the user. Press the "Save & Close" button. You have just created a user note which is not associated to any user.

This is caused by an useless type cast to int, which turns invalid values like an empty string to 0 which is a valid id.

Under different circumstances the id is handled correctly simply by avoiding the type cast to int. For example, repeat the steps above, but before saving the record, press the user selection button. Once in the users list clear the selection by pressing the button "- No User -". This time the record won't be saved and a the message "Field required: User ID" will be displayed.

Tracker: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=33620&start=0

In addition, this pull request fixes another potential bug:
when $this->value == 'CURRENT', the current user id is used to load the table, but the hidden field below ignores the loaded value and it continue using the string "current" stored in $this->value which of course is not a valid user id.
  • Loading branch information
demis-palma committed Apr 15, 2014
1 parent 02cc723 commit a12a633
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions libraries/cms/form/field/user.php
Expand Up @@ -76,7 +76,9 @@ protected function getInput()
// Handle the special case for "current".
elseif (strtoupper($this->value) == 'CURRENT')
{
$table->load(JFactory::getUser()->id);
// 'CURRENT' is not a reasonable value to be placed in the html
$this->value = JFactory::getUser()->id;
$table->load($this->value);
}
else
{
Expand All @@ -99,7 +101,7 @@ protected function getInput()
$html[] = '</div>';

// Create the real field, hidden, that stored the user id.
$html[] = '<input type="hidden" id="' . $this->id . '_id" name="' . $this->name . '" value="' . (int) $this->value . '" />';
$html[] = '<input type="hidden" id="' . $this->id . '_id" name="' . $this->name . '" value="' . $this->value . '" />';

return implode("\n", $html);
}
Expand Down

0 comments on commit a12a633

Please sign in to comment.