Skip to content

SENTRY-514224 Prevent array to string conversion error #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

SENTRY-514224 Prevent array to string conversion error #2

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions lib/validator/sfValidatorChoice.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,10 @@ protected function isEmpty($value)
*/
static protected function inChoices($value, array $choices = array())
{
if (is_array($value) && 1 === count($value)) {
$value = reset($value);
}

Copy link
Collaborator

@obukhov obukhov May 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please shortly explain why is it needed and why is it php7 related?
Is (string)$array works differently in php7?
How it supposed to work in $value is an array but with more them 1 element?

Copy link
Author

@wutsch0 wutsch0 May 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not really php7 related, did popup before as well. The Choice validator casts choice values to strings before checking if they exist in $choices array, but the values can be arrays as well (here a sfWidgetFormSelectRadio widget). To prevent the warning/notification the value array should be replaced by its only item

Copy link
Collaborator

@obukhov obukhov May 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is a misuse of the validator - because it has other clean function for the case when multiple choice is enabled

  protected function doClean($value)
  {
    $choices = $this->getChoices();

    if ($this->getOption('multiple'))
    {
      $value = $this->cleanMultiple($value, $choices);
    }
    else
    {
      if (!self::inChoices($value, $choices))
      {
        throw new sfValidatorError($this, 'invalid', array('value' => $value));
      }
    }

    return $value;
  }

So somewhere "multiple" widget is used with choice validator where "multiple" option is not enabled. We rather fix that one, than making this change.

foreach ($choices as $choice)
{
if ((string) $choice == (string) $value)
Expand Down