ZF-9586: A NonArray Subform with equal Name like a chain segment from a previous attached SubForm overwrites data in getValidValues

Description

While fixing getErrors i had a closer look at [ZF-5222] again, as Vivek Chandran states there, mixing !isArray() SubForm with SubForms and interferencing Name and elementsBelongTo segments, data will be stripped because of array_merge or not merging at all.


$sub1->setElementsBelongTo('foo[bar]');
$sub2->setIsArray(false);
$form->addSubForm($sub1, 'bar')
     ->addSubForm($sub2, 'foo');
...
$values = array_merge($values, $this->_attachToArray($sub1->getValidValues(...), 'foo[bar]'));
// $values === array('foo' => array('bar' => ... ))
...
// next cycle
if (!$sub2->isArray()) {
    $values['foo'] = $sub2->getValidValues(...);
    // o.O
}

$sub1->setIsArray(false);
$sub2->setElementsBelongTo('foo[bar]');
$form->addSubForm($sub1, 'foo')
     ->addSubForm($sub2, 'bar');
...
if (!$sub1->isArray()) {
    $values['foo'] = $sub1->getValidValues(...);
    // $values === array('foo' => array('elem' => 'thisShouldBeKept'))
}
// next cycle
$values = array_merge($values, $this->_attachToArray($sub2->getValidValues(...), 'foo[bar]'));
// $values === array('foo' => array( 'elem' => 'thrownAway' ))
...

To check against similar failures in this parents previous SubTasks i updated their tests to use the new Setup as well, however getValidValues is the only one to fail from the commited patches.

before Patch


...
        foreach ($this->getSubForms() as $key => $form) {
            if (isset($data[$key]) && !$form->isArray()) {
                $tmp = $form->getValidValues($data[$key]);
                if (!empty($tmp)) {
                    $values[$key] = $tmp;
                }
            } else {
                $tmp = $form->getValidValues($data, true);
                if (!empty($tmp)) {
                    $fValues = $this->_attachToArray($tmp, $form->getElementsBelongTo());
                    $values = array_merge($values, $fValues);
                }
            }
        }
...

after Patch


...
        foreach ($this->getSubForms() as $key => $form) {
            $merge = array();
            if (isset($data[$key]) && !$form->isArray()) {
                $tmp = $form->getValidValues($data[$key]);
                if (!empty($tmp)) {
                    $merge[$key] = $tmp;
                }
            } else {
                $tmp = $form->getValidValues($data, true);
                if (!empty($tmp)) {
                    $merge = $this->_attachToArray($tmp, $form->getElementsBelongTo());
                }
            }
            $values = array_merge_recursive($values, $merge);
        }
...

The New Test Setup


    public function _setup9350()
    {
        $this->form->addSubForm(new Zend_Form_SubForm(), 'foo')
                   ->foo->setElementsBelongTo('foo[foo]')            // foo[foo]
                        ->addSubForm(new Zend_Form_SubForm(), 'foo') // foo[foo][foo]
                        ->foo->setIsArray(false)
                             ->addElement('text', 'foo')             // foo[foo][foo][foo]
                             ->foo->addValidator('Identical',
                                                 false,
                                                 array('foo Value'));

        $this->form->foo->addSubForm(new Zend_Form_SubForm(), 'baz') // foo[foo][baz]
                   ->baz->setIsArray(false)
                        ->addSubForm(new Zend_Form_SubForm(), 'baz')
                        ->baz->setElementsBelongTo('baz[baz]')       // foo[foo][baz][baz][baz]
                             ->addElement('text', 'baz')             // foo[foo][baz][baz][baz][baz]
                             ->baz->addValidator('Identical',
                                                 false,
                                                 array('baz Value'));

        // This is appending a different named SubForm and setting
        // elementsBelongTo to a !isArray() Subform name from same level
        $this->form->foo->addSubForm(new Zend_Form_SubForm(), 'quo')
                        ->quo->setElementsBelongTo('foo')            // foo[foo][foo] !!!!
                             ->addElement('text', 'quo')             // foo[foo][foo][quo]
                             ->quo->addValidator('Identical',
                                                 false,
                                                 array('quo Value'));
        
        // This is setting elementsBelongTo point into the middle of 
        // a chain of another SubForms elementsBelongTo
        $this->form->addSubForm(new Zend_Form_SubForm(), 'duh')
                   ->duh->setElementsBelongTo('foo[zoo]')            // foo[zoo] !!!!
                        ->addElement('text', 'zoo')                  // foo[zoo][zoo]
                        ->zoo->addValidator('Identical',
                                            false,
                                            array('zoo Value'));

        // This is !isArray SubForms Name equal to the last segment
        // of another SubForms elementsBelongTo
        $this->form->addSubForm(new Zend_Form_SubForm(), 'iek')
                   ->iek->setElementsBelongTo('foo')                 // foo !!!!
                        ->addSubForm(new Zend_Form_SubForm(), 'zoo') // foo[zoo] !!!!
                        ->zoo->setIsArray(false)
                             ->addElement('text', 'iek')             // foo[zoo][iek]
                             ->iek->addValidator('Identical',
                                                 false,
                                                 array('iek Value'));

        $data = array('valid'   => array('foo' =>
                                         array('foo' =>
                                               array('foo' =>
                                                     array('foo' => 'foo Value',
                                                           'quo' => 'quo Value'),
                                                     'baz' => 
                                                     array('baz' => 
                                                           array('baz' =>
                                                                 array('baz' => 'baz Value')))),
                                               'zoo' =>
                                               array('zoo' => 'zoo Value',
                                                     'iek' => 'iek Value'))),
                      'invalid' => array('foo' =>
                                         array('foo' =>
                                               array('foo' =>
                                                     array('foo' => 'foo Invalid',
                                                           'quo' => 'quo Value'),
                                                     'baz' => 
                                                     array('baz' => 
                                                           array('baz' =>
                                                                 array('baz' => 'baz Value')))),
                                               'zoo' =>
                                               array('zoo' => 'zoo Value',
                                                     'iek' => 'iek Invalid'))),
                      'partial' => array('foo' =>
                                         array('foo' =>
                                               array('baz' => 
                                                     array('baz' => 
                                                           array('baz' =>
                                                                 array('baz' => 'baz Value'))),
                                                    'foo' => 
                                                     array('quo' => 'quo Value')),
                                               'zoo' =>
                                               array('zoo' => 'zoo Value'))));
        return $data;
    }

    public function testGetValidValuesWithElementsBelongTo()
    {
        $data = $this->_setup9350();
        $this->assertSame($this->form->getValidValues($data['invalid']), $data['partial']);
    }

This patch fixes the issue and updates the Unit Tests of previous SubTasks within this group as well, to make sure they do not suffer from similar issues.


Index: tests/Zend/Form/FormTest.php
===================================================================
--- tests/Zend/Form/FormTest.php    (Revision 21732)
+++ tests/Zend/Form/FormTest.php    (Arbeitskopie)
@@ -1473,106 +1473,118 @@
         $this->assertTrue($this->form->isValid($data));
     }
 
-    public function testIsValidEqualSubFormAndElementName()
-    {
-        $this->form->addSubForm(new Zend_Form_SubForm(), 'foo')
-                   ->foo->addElement('text', 'foo')
-                        ->foo->setRequired(true)
-                             ->addValidator('Identical',
-                                            false,
-                                            array('Foo Value'));
-        $foo = array('foo' =>
-                     array('foo' => 'Foo Value'));
-
-        $this->assertTrue($this->form->isValid($foo));
-
-        $this->form->foo->setIsArray(false);
-
-        $this->assertTrue($this->form->isValid($foo));
-    } 
-
-    public function testIsValidPartialEqualSubFormAndElementName()
-    {
-        $this->form->addSubForm(new Zend_Form_SubForm(), 'foo')
-                   ->foo->addElement('text', 'foo')
-                        ->foo->setRequired(true)
-                             ->addValidator('Identical',
-                                            false,
-                                            array('Foo Value'));
-        $foo = array('foo' =>
-                     array('foo' => 'Foo Value'));
-
-        $this->assertTrue($this->form->isValidPartial($foo));
-
-        $this->form->foo->setIsArray(false);
-
-        $this->assertTrue($this->form->isValidPartial($foo));
-    } 
-
-    public function testPopulateWithElementsBelongTo()
-    {
-        $this->form->addSubForm(new Zend_Form_SubForm(), 'foo')
-                   ->foo->setElementsBelongTo('foo[foo]')
-                        ->addSubForm(new Zend_Form_SubForm(), 'foo')
-                        ->foo->setIsArray(false)
-                             ->addElement('text', 'foo');
-
-        $foo = array('foo' =>
-                     array('foo' =>
-                           array('foo' =>
-                                 array('foo' => 'foo Value'))));
-
-        $this->form->setView($this->getView())
-                   ->populate($foo);
-
-        $this->assertRegexp('/value=.foo Value./', $this->form->render());
-    }
-
     public function _setup9350()
     {
         $this->form->addSubForm(new Zend_Form_SubForm(), 'foo')
-                   ->foo->setElementsBelongTo('foo[foo]')
-                        ->addSubForm(new Zend_Form_SubForm(), 'foo')
+                   ->foo->setElementsBelongTo('foo[foo]')            // foo[foo]
+                        ->addSubForm(new Zend_Form_SubForm(), 'foo') // foo[foo][foo]
                         ->foo->setIsArray(false)
-                             ->addElement('text', 'foo')
+                             ->addElement('text', 'foo')             // foo[foo][foo][foo]
                              ->foo->addValidator('Identical',
                                                  false,
                                                  array('foo Value'));
 
-        $this->form->foo->addSubForm(new Zend_Form_SubForm(), 'baz')
+        $this->form->foo->addSubForm(new Zend_Form_SubForm(), 'baz') // foo[foo][baz]
                    ->baz->setIsArray(false)
                         ->addSubForm(new Zend_Form_SubForm(), 'baz')
-                        ->baz->setElementsBelongTo('baz[baz]')
-                             ->addElement('text', 'baz')
+                        ->baz->setElementsBelongTo('baz[baz]')       // foo[foo][baz][baz][baz]
+                             ->addElement('text', 'baz')             // foo[foo][baz][baz][baz][baz]
                              ->baz->addValidator('Identical',
                                                  false,
                                                  array('baz Value'));
 
-        $data = array('valid' => array('foo' =>
-                                       array('foo' =>
-                                             array('foo' =>
-                                                   array('foo' => 'foo Value'),
-                                                   'baz' => 
-                                                   array('baz' => 
-                                                         array('baz' =>
-                                                               array('baz' => 'baz Value')))))),
+        // This is appending a different named SubForm and setting
+        // elementsBelongTo to a !isArray() Subform name from same level
+        $this->form->foo->addSubForm(new Zend_Form_SubForm(), 'quo')
+                        ->quo->setElementsBelongTo('foo')            // foo[foo][foo] !!!!
+                             ->addElement('text', 'quo')             // foo[foo][foo][quo]
+                             ->quo->addValidator('Identical',
+                                                 false,
+                                                 array('quo Value'));
+        
+        // This is setting elementsBelongTo point into the middle of 
+        // a chain of another SubForms elementsBelongTo
+        $this->form->addSubForm(new Zend_Form_SubForm(), 'duh')
+                   ->duh->setElementsBelongTo('foo[zoo]')            // foo[zoo] !!!!
+                        ->addElement('text', 'zoo')                  // foo[zoo][zoo]
+                        ->zoo->addValidator('Identical',
+                                            false,
+                                            array('zoo Value'));
+
+        // This is !isArray SubForms Name equal to the last segment
+        // of another SubForms elementsBelongTo
+        $this->form->addSubForm(new Zend_Form_SubForm(), 'iek')
+                   ->iek->setElementsBelongTo('foo')                 // foo !!!!
+                        ->addSubForm(new Zend_Form_SubForm(), 'zoo') // foo[zoo] !!!!
+                        ->zoo->setIsArray(false)
+                             ->addElement('text', 'iek')             // foo[zoo][iek]
+                             ->iek->addValidator('Identical',
+                                                 false,
+                                                 array('iek Value'));
+
+        $data = array('valid'   => array('foo' =>
+                                         array('foo' =>
+                                               array('foo' =>
+                                                     array('foo' => 'foo Value',
+                                                           'quo' => 'quo Value'),
+                                                     'baz' => 
+                                                     array('baz' => 
+                                                           array('baz' =>
+                                                                 array('baz' => 'baz Value')))),
+                                               'zoo' =>
+                                               array('zoo' => 'zoo Value',
+                                                     'iek' => 'iek Value'))),
                       'invalid' => array('foo' =>
                                          array('foo' =>
                                                array('foo' =>
-                                                     array('foo' => 'foo Invalid'),
+                                                     array('foo' => 'foo Invalid',
+                                                           'quo' => 'quo Value'),
                                                      'baz' => 
                                                      array('baz' => 
                                                            array('baz' =>
-                                                                 array('baz' => 'baz Value')))))),
+                                                                 array('baz' => 'baz Value')))),
+                                               'zoo' =>
+                                               array('zoo' => 'zoo Value',
+                                                     'iek' => 'iek Invalid'))),
                       'partial' => array('foo' =>
-                                       array('foo' =>
-                                             array('baz' => 
-                                                   array('baz' => 
-                                                         array('baz' =>
-                                                               array('baz' => 'baz Value')))))));
+                                         array('foo' =>
+                                               array('baz' => 
+                                                     array('baz' => 
+                                                           array('baz' =>
+                                                                 array('baz' => 'baz Value'))),
+                                                    'foo' => 
+                                                     array('quo' => 'quo Value')),
+                                               'zoo' =>
+                                               array('zoo' => 'zoo Value'))));
         return $data;
     }
 
+    public function testIsValidEqualSubFormAndElementName()
+    {
+        $data = $this->_setup9350();
+        $this->assertTrue($this->form->isValid($data['valid']));
+    } 
+
+    public function testIsValidPartialEqualSubFormAndElementName()
+    {
+        $data = $this->_setup9350();
+        $this->assertTrue($this->form->isValidPartial($data['partial']));
+    } 
+
+    public function testPopulateWithElementsBelongTo()
+    {
+        $data = $this->_setup9350();
+
+        $this->form->setView($this->getView())->populate($data['valid']);
+        $html = $this->form->render();
+
+        $this->assertRegexp('/value=.foo Value./', $html);
+        $this->assertRegexp('/value=.baz Value./', $html);
+        $this->assertRegexp('/value=.quo Value./', $html);
+        $this->assertRegexp('/value=.zoo Value./', $html);
+        $this->assertRegexp('/value=.iek Value./', $html);
+    }
+
     public function testGetValidValuesWithElementsBelongTo()
     {
         $data = $this->_setup9350();
Index: library/Zend/Form.php
===================================================================
--- library/Zend/Form.php   (Revision 21732)
+++ library/Zend/Form.php   (Arbeitskopie)
@@ -1343,18 +1343,19 @@
             }
         }
         foreach ($this->getSubForms() as $key => $form) {
+            $merge = array();
             if (isset($data[$key]) && !$form->isArray()) {
                 $tmp = $form->getValidValues($data[$key]);
                 if (!empty($tmp)) {
-                    $values[$key] = $tmp;
+                    $merge[$key] = $tmp;
                 }
             } else {
                 $tmp = $form->getValidValues($data, true);
                 if (!empty($tmp)) {
-                    $fValues = $this->_attachToArray($tmp, $form->getElementsBelongTo());
-                    $values = array_merge($values, $fValues);
+                    $merge = $this->_attachToArray($tmp, $form->getElementsBelongTo());
                 }
             }
+            $values = array_merge_recursive($values, $merge);
         }
         if (!$suppressArrayNotation && $this->isArray() && !empty($values)) {
             $values = $this->_attachToArray($values, $this->getElementsBelongTo());

Comments

Updated Unit Test

Patch applied to trunk and 1.10 release branch.