Issues

ZF-11102: Zend_Form_Element issue in _loadFilter method when you use Zend_Filter with an "options" array

Issue Type: Bug Created: 2011-02-22T02:08:59.000+0000 Last Updated: 2012-05-03T15:29:24.000+0000 Status: Open Fix version(s): Reporter: Przemys?aw Wróbel (wrobel) Assignee: Matthew Weier O'Phinney (matthew) Tags: - Zend_Form

  • zf-crteam-review

Related issues: Attachments:

Description

considering this Zend_Form class:

<pre class="highlight">
class MyForm extends Zend_Form

public function __construct () { 

...

/* country is a Zend_Form_Element_Text object */
$this->country
     ->addFilter('AutocompleteHelper', array('minlength' => 2, 'url' => '/json/coutries'));

}

...

}

As you can see I add a filter to text field with options associative array. Zend_Form_Element class doesn't instance well the Zend_Filter object and during the initializing phase it loose "options" parameters.

To solve this I've change the way how Zend_Filter is initialized on line 1920 (_loadFilter method) of Zend_Form_Element class

<pre class="highlight">
            if ($r->hasMethod('__construct')) {
                $instance = new $name($filter['options']);
                //$instance = $r->newInstanceArgs($filter['options']);
            } else {
                $instance = new $name();
                //$instance = $r->newInstance();
            }

Regards

Comments

Posted by Przemys?aw Wróbel (wrobel) on 2011-02-22T02:14:45.000+0000

I decided to reopen this issue since it seems to me to be an inconsistent behaviour when compared to the addValidator() method which does not require an extra wrapping of options array. I have spent some time to make things work and I believe more users will have the same problem - since there is even no such an example in ZF Guide.

this->grade->addValidator('Float', false, array('locale' => 'en')); //english locale since value is already normalized this->grade->addFilter('LocalizedToNormalized', array(array('precision' => 2)));

Posted by Przemys?aw Wróbel (wrobel) on 2011-02-22T02:16:39.000+0000

Sorry for cloning the issue - is there a better way to reopening it?

Posted by Adam Lundrigan (adamlundrigan) on 2012-03-06T01:33:24.000+0000

Is this still an issue against current SVN trunk? This test passes against trunk for me:

<pre class="highlight">
Index: Zend/Form/ElementTest.php
===================================================================
--- Zend/Form/ElementTest.php   (revision 24672)
+++ Zend/Form/ElementTest.php   (working copy)
@@ -2189,6 +2189,22 @@
         $validator = $username->getValidator('regex');
         $this->assertTrue($validator->zfBreakChainOnFailure);
     }
+
+    /**
+     * @group ZF-11102
+     */
+    public function testAddFilterPassesOptionsOnToFilterObject()
+    {
+        $field = new Zend_Form_Element('foo');
+        $field->addFilter('Alpha', array(
+            'allowwhitespace' => true,
+        ));
+        $filters = $field->getFilters();
+        $this->assertArrayHasKey('Zend_Filter_Alpha', $filters);
+        $filter = $filters['Zend_Filter_Alpha'];
+        $this->assertType('Zend_Filter_Alpha', $filter);
+        $this->assertTrue($filter->getAllowWhitespace());
+    }
 }
 
 class Zend_Form_ElementTest_Decorator extends Zend_Form_Decorator_Abstract

Could you let me know if i've missed the point with the above test, or if the issue really is fixed in SVN trunk?

Posted by Adam Lundrigan (adamlundrigan) on 2012-03-06T01:39:01.000+0000

Apparently I did. This test fails against trunk:

<pre class="highlight">
Index: tests/Zend/Form/ElementTest.php
===================================================================
--- tests/Zend/Form/ElementTest.php (revision 24672)
+++ tests/Zend/Form/ElementTest.php (working copy)
@@ -2189,6 +2189,22 @@
         $validator = $username->getValidator('regex');
         $this->assertTrue($validator->zfBreakChainOnFailure);
     }
+
+    /**
+     * @group ZF-11102
+     */
+    public function testAddFilterPassesOptionsOnToFilterObjectLocalizedToNormalized()
+    {
+        $field = new Zend_Form_Element('foo');
+        $field->addFilter('LocalizedToNormalized', array(
+            'precision' => 2,
+        ));
+        $filters = $field->getFilters();
+        $this->assertArrayHasKey('Zend_Filter_LocalizedToNormalized', $filters);
+        $filter = $filters['Zend_Filter_LocalizedToNormalized'];
+        $this->assertType('Zend_Filter_LocalizedToNormalized', $filter);
+        $this->assertEquals(2, $filter->getPrecision());
+    }
 }
 
 class Zend_Form_ElementTest_Decorator extends Zend_Form_Decorator_Abstract

With this error:

<pre class="highlight">
1) Zend_Form_ElementTest::testAddFilterPassesOptionsOnToFilterObjectLocalizedToNormalized
Argument 1 passed to Zend_Filter_LocalizedToNormalized::setOptions() must be an array, integer given, called in Zend/Filter/LocalizedToNormalized.php on line 64 and defined

Zend/Filter/LocalizedToNormalized.php:84
Zend/Filter/LocalizedToNormalized.php:64

Could be a problem with how that particular filter implements it's option handling?

Posted by Adam Lundrigan (adamlundrigan) on 2012-03-10T02:16:18.000+0000

The problem is exactly as you point out. newInstanceArgs takes it's argument to be an array of constructor arguments to pass on the new instance, which mesh with the intended functionality: the array being passed as first argument. This change (which you also pointed out above) will fix the issue:

<pre class="highlight">

Index: library/Zend/Form/Element.php
===================================================================
--- library/Zend/Form/Element.php   (revision 24672)
+++ library/Zend/Form/Element.php   (working copy)
@@ -2083,7 +2083,7 @@
         } else {
             $r = new ReflectionClass($name);
             if ($r->hasMethod('__construct')) {
-                $instance = $r->newInstanceArgs((array) $filter['options']);
+                $instance = $r->newInstanceArgs(array((array) $filter['options']));
             } else {
                 $instance = $r->newInstance();
             }

Here's another example, this time of Zend_Filter_PregReplace:

<pre class="highlight">
Index: tests/Zend/Form/ElementTest.php
===================================================================
--- tests/Zend/Form/ElementTest.php (revision 24672)
+++ tests/Zend/Form/ElementTest.php (working copy)
@@ -2189,6 +2189,23 @@
         $validator = $username->getValidator('regex');
         $this->assertTrue($validator->zfBreakChainOnFailure);
     }
+
+    /**
+     * @group ZF-11102
+     */
+    public function testAddFilterPassesOptionsOnToFilterObjectPregReplace()
+    {
+        $field = new Zend_Form_Element('foo');
+        $field->addFilter('PregReplace', array(
+            'replace' => 'z',
+            'match' => '/[a-z]/i',
+        ));
+        $filters = $field->getFilters();
+        $this->assertArrayHasKey('Zend_Filter_PregReplace', $filters);
+        $filter = $filters['Zend_Filter_PregReplace'];
+        $this->assertType('Zend_Filter_PregReplace', $filter);
+        $this->assertEquals('z', $filter->getReplacement());
+    }
 }
 
 class Zend_Form_ElementTest_Decorator extends Zend_Form_Decorator_Abstract

And the result:

<pre class="highlight">
1) Zend_Form_ElementTest::testAddFilterPassesOptionsOnToFilterObjectPregReplace
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-z
+/[a-z]/i

Instead of calling constructor with single array argument, the array contents are used as N arguments to the constructor. Which works fine for some filters (like Zend_Filter_Alpha) which have only a single constructor argument anyway. In the second example above, I purposefully changed the order of the array (putting 'replace' first) and the resulting values get switched inside the filter (pattern comes out through getReplacement()).

So, what's the game plan? Zend_Form_Element->_loadFilter is obviously broken, almost to the point of unusability. But this change smells like a BC break to me.

Posted by Frédéric Dewinne (frederic@dwebconsulting.be) on 2012-05-03T15:29:24.000+0000

This fix will result in another bug. I mean when $filter['options'] is a string, it will be cast to an unindexed array.

I explain what I mean :

<pre class="highlight">
->addFilter('null', 'integer');

This code will work with the current bug. But won't with that fix.

By changing your fix into :

<pre class="highlight">
-                $instance = $r->newInstanceArgs((array) $filter['options']);
+                $instance = $r->newInstanceArgs(array($filter['options']));

It will be backward compatible and the second argument of the Zend_Form_Element::addFilter() method will really be the same as the first argument of the filter's constructor

Have you found an issue?

See the Overview section for more details.

Copyright

© 2006-2016 by Zend, a Rogue Wave Company. Made with by awesome contributors.

This website is built using zend-expressive and it runs on PHP 7.

Contacts