Skip to content
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.

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

[BUG] Validator Message Group iteration incorrect behavior after unsetting a message index #12455

Closed
remitoffoli opened this issue Dec 5, 2016 · 5 comments
Milestone

Comments

@remitoffoli
Copy link

remitoffoli commented Dec 5, 2016

Hi, I just ran into a bug trying to iterate a Phalcon\Validation\Message\Group after having unset a message index. Here is an exemple :

use Phalcon\Validation;
use Phalcon\Validation\Message;

$validation = new Validation();

// adds messages
$validation->appendMessage(new Message('error a', 'myField', 'MyValidator'));
// [0] => 'error a'
$validation->appendMessage(new Message('error b', 'myField', 'MyValidator'));
// [0] => 'error a',  [1] => 'error b'

// removes the last message (for whatever reason, I've mine...)
$messages = $validator->getMessages();
unset($messages[count($messages)-1]); // you can't array_splice because Group isn't an array... :(
// [0] => 'error a'

// adds a new message
$validation->appendMessage(new Message('error c', 'myField', 'MyValidator'));
// [0] => 'error a',  [2] => 'error c'

// now the last message isn't in the range available by the foreach...
foreach ($messages as $msg) {
    echo $msg ."<br>";
}
// will only echo the 'error a'

FYI, if you have to iterate with this problem you cant use the following hack.

$messages = (array) $messages;
$messages = $messages["\0*\0_messages"];
// more informations about the \0*\0 here :
// https://coderwall.com/p/36h6qa/converting-php-objects-to-arrays-to-read-private-members

The problem can easily be fixed by replacing unset this->_messages[index]; by an array_splice in the following function of https://github.com/phalcon/cphalcon/blob/master/phalcon/validation/message/group.zep#L115

Just like that :

public function offsetUnset(index)
{
    if isset this->_messages[index] {
        array_splice(this->_messages, index, 1);
    }
    return false;
}

Details

  • Phalcon version: 3.0.1
  • PHP Version: 5.6.24
  • Operating System: Windows 10
  • Installation type: installing windows dll
  • Zephir version (if any):
  • Apache
  • Other related info (Database, table schema): nope
@stamster
Copy link
Contributor

stamster commented Dec 7, 2016

@Jurigag
Copy link
Contributor

Jurigag commented Dec 7, 2016

But unset should work too. It's implementing ArrayAccess. When doing unset it's calling offsetUnset anyway.

@remitoffoli
Copy link
Author

@Jurigag is right.

In fact, I thinks you should not use directly offsetUnset(). PHP calls it when you use unset(...) : http://php.net/manual/en/class.arrayaccess.php

Have a look at the code exemple just below the method documentation ;)
https://docs.phalconphp.com/en/3.0.1/api/Phalcon_Validation_Message_Group.html

@gguridi
Copy link
Contributor

gguridi commented Dec 28, 2016

Yes, the change is working. I've added a pull request for the change after checking that it's working properly. Not sure if there is another pull request already about this, but just in case I went ahead.

@sergeyklay
Copy link
Contributor

Fixed in the 3.0.x branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants