Issues

ZF-11609: ZF 1.11.9: No Captcha img rendered

Issue Type: Bug Created: 2011-07-25T21:13:29.000+0000 Last Updated: 2012-05-11T15:56:20.000+0000 Status: Resolved Fix version(s): - 1.11.10 (04/Aug/11)

Reporter: rossi (segnior_rossi) Assignee: Kai Uwe (kaiuwe) Tags: - Zend_Form

Related issues: - ZF-8094

Attachments: - Captcha.php.patch

Description

Hi,

i've upgraded from ZF 1.11.8 to 1.11.9. After the upgrade no captcha image is displayed any more, because the captcha img tag is not rendered. I could solve it by explicitely adding the captcha decorator to Zend_Form_Element_Captcha.

Issue is explained here as well: http://stackoverflow.com/questions/6726987/…

ninsky

Comments

Posted by Kai Uwe (kaiuwe) on 2011-07-25T21:53:49.000+0000

Component changed to Zend_Form. Priority changed because a simple workaround is available.

Posted by Kai Uwe (kaiuwe) on 2011-07-25T21:56:00.000+0000

Please have a look at: ZF-8094 - Captcha element always set captcha decorator

Posted by Kai Uwe (kaiuwe) on 2011-07-26T08:34:52.000+0000

Example to reproduce the problem.

<pre class="highlight">
$form = new Zend_Form(); 

$form->addElement( 
    'captcha', 
    'foo', 
    array( 
        'label' => 'Foo', 
            'captcha' => array( 
            'captcha' => 'Figlet', 
            'wordLen' => 6, 
            'timeout' => 300, 
        ), 
    ) 
); 

echo '

Before rendering

'; 
echo '
'; 
foreach ($form->getElement('foo')->getDecorators() as $decorator) { 
    echo '' . get_class($decorator) . ''; 
} 
echo ''; 

echo '

Output

'; 
echo $form->render($view); 

echo '

After rendering

'; 
echo '
'; 
foreach ($form->getElement('foo')->getDecorators() as $decorator) { 
    echo '' . get_class($decorator) . ''; 
} 
echo '';


<pre class="highlight">

Before rendering

Zend_Form_Decorator_Errors
    Zend_Form_Decorator_Description
    Zend_Form_Decorator_HtmlTag
    Zend_Form_Decorator_Label

Output

            Foo
        
        
            
    

After rendering

Zend_Form_Decorator_Captcha_Word
    Zend_Form_Decorator_Errors
    Zend_Form_Decorator_Description
    Zend_Form_Decorator_HtmlTag
    Zend_Form_Decorator_Label

Before rendering

  • Zend_Form_Decorator_Errors
  • Zend_Form_Decorator_Description
  • Zend_Form_Decorator_HtmlTag
  • Zend_Form_Decorator_Label

After rendering

  • Zend_Form_Decorator_Captcha_Word
  • Zend_Form_Decorator_Errors
  • Zend_Form_Decorator_Description
  • Zend_Form_Decorator_HtmlTag
  • Zend_Form_Decorator_Label

The decorator "Zend_Form_Decorator_Captcha" is missing.

BUT:

<pre class="highlight">
$form->getElement('foo')->clearDecorators(); 



<pre class="highlight">

Before rendering

``### Output

After rendering

Zend_Form_Decorator_Captcha_Word

Before rendering

(empty)

After rendering

  • Zend_Form_Decorator_Captcha_Word

Posted by Thanos Kyritsis (djart) on 2011-07-26T10:14:58.000+0000

Thanks for opening this bug. It is affected by Subversion revision #24223 for library/Zend/Form/Element/Captcha.php which fixed ReCaptcha, but broke the other captchas (Figlet, Image and Dumb which are all Word Captchas). Specifically, the following block of code doesn't work well for Word Captchas (it only does for ReCaptcha):

<pre class="highlight">
$decorator  = $captcha->getDecorator();
if (!empty($decorator)) {
    array_unshift($decorators, $decorator);
} else {
     $decorator = array('Captcha', array('captcha' => $captcha));
     array_unshift($decorators, $decorator);
}

$this->setDecorators($decorators);

Recaptcha needs just his own decorator, so the if/else block is correct for it's case. All the other (word) captchas need both their own decorator (which is actually "Captcha_Word") AND additionally the generic Captcha Decorator, so that if/else block ruins it for them.

I can think of two different solutions, I don't know which one Zend Framework developers think as better:

  • Change the above if/else block to a bit more complicated (and "uglier") one
  • Refactor the Captcha_Word decorator (making it as "good" and "rich" as the Captcha_ReCaptcha decorator is) and (probably) drop the generic Captcha decorator completely. (I would say this is the cleaner approach but ZF devs should decide)

For the first case, the if/else could be something like the following pseudo-code:

<pre class="highlight">
$decorator  = $captcha->getDecorator();
if (!empty($decorator)) {
    array_unshift($decorators, $decorator);
}

if (the current captcha object is not ReCaptcha) {
     // push an additional decorator to the stack
     $decorator = array('Captcha', array('captcha' => $captcha));
     array_unshift($decorators, $decorator);
}

$this->setDecorators($decorators);

Posted by Kai Uwe (kaiuwe) on 2011-07-26T10:55:42.000+0000

{quote} Refactor the Captcha_Word decorator (making it as "good" and "rich" as the Captcha_ReCaptcha decorator is) and (probably) drop the generic Captcha decorator completely. (I would say this is the cleaner approach but ZF devs should decide) {quote} I think it is a bad idea, because we need a separation between image/figlet and the text input.

(The example does not use the current implementation, because the current implementation adds additional decorators in the method to render!)

<pre class="highlight">
// Form
$form = new Zend_Form();
$form->removeDecorator('HtmlTag');

// Element
$form->addElement(
    'captcha',
    'foo',
    array(
        'label' => 'Foo',
        'captcha' => array(
            'captcha' => 'Figlet',
            'wordLen' => 6,
            'timeout' => 300,
        ),
    )
);

// Element decorators
$form->getElement('foo')->setDecorators(array(
    // Zend_Form_Decorator_HtmlTag
    array(
        // Decorator name
        array(
            'spanOpen' => 'HtmlTag',
        ),
        // Decorator options
        array(
            'tag'      => 'span',
            'openOnly' => true,
        ),
    ),
    // Zend_Form_Decorator_Captcha
    'Captcha',
    // Zend_Form_Decorator_HtmlTag
    array(
        // Decorator name
        array(
            'spanClose' => 'HtmlTag',
        ),
        // Decorator options
        array(
            'tag'       => 'span',
            'closeOnly' => true,
            'placement' => 'append',
        ),
    ),
    // Zend_Form_Decorator_Captcha_Word
    'Captcha_Word',
    // Zend_Form_Decorator_Errors
    'Errors',
    // Zend_Form_Decorator_Description
    'Description',
    // Zend_Form_Decorator_Label
    'Label',
    // Zend_Form_Decorator_HtmlTag
    array(
        // Decorator name
        array(
            'div' => 'HtmlTag',
        ),
        // Decorator options
        array(
            'tag' => 'div',
        ),
    ),
));

echo $form->render($view);



<pre class="highlight">



        Foo
        <span>
        </span>


__    __   __   __   __   _    __   __    ______    _____   
\ \\ / //  \ \\/ // | || | ||  \ \\/ //  /_   _//  |  ___|| 
 \ \/ //    \ ` //  | '--' ||   \ ` //     | ||    | ||__   
  \  //      | ||   | .--. ||    | ||     _| ||    | ||__   
   \//       |_||   |_|| |_||    |_||    /__//     |_____|| 
    `        `-`'   `-`  `-`     `-`'    `--`      `-----`  
                                                            


        

Posted by Kai Uwe (kaiuwe) on 2011-07-26T11:08:54.000+0000

@Thanos The new implementation must respect "loadDefaultDecoratorsIsDisabled()". See my example above with “clearDecorators()" and ZF-8094.

Posted by Thanos Kyritsis (djart) on 2011-07-26T12:21:25.000+0000

{quote} The new implementation must respect "loadDefaultDecoratorsIsDisabled()". See my example above with “clearDecorators()" and ZF-8094. {quote}

@Kai I agree, it's a chance to solve both problems/issues with one patch. I think that the Zend_Form_Element_Captcha render function currently doesn't show much respect neither for ReCaptcha nor for the others, am I correct ? It is the only render function (among other zend form elements) that actually pushes additional decorators in the stack.

Are the Zend_Form_Decorator_Captcha_Word, Zend_Form_Decorator_Captcha and Zend_Form_Decorator_Captcha_ReCaptcha regarded as default decorators or not ? Which of them should or shouldn't be in the default decorators group ?

Posted by Kai Uwe (kaiuwe) on 2011-07-26T14:59:43.000+0000

{quote}I think that the Zend_Form_Element_Captcha render function currently doesn't show much respect neither for ReCaptcha nor for the others, am I correct ?{quote} Right. {quote}It is the only render function (among other zend form elements) that actually pushes additional decorators in the stack.{quote} Zend_Form_Element_File needs also a special decorator, but takes a different approach. {quote}Are the Zend_Form_Decorator_Captcha_Word, Zend_Form_Decorator_Captcha and Zend_Form_Decorator_Captcha_ReCaptcha regarded as default decorators or not ? Which of them should or shouldn't be in the default decorators group ?{quote} This is the problem: ||Captcha adapters||Required decorators|| |Dumb, Figlet, Image|Zend_Form_Decorator_Captcha + Zend_Form_Decorator_Captcha_Word| |ReCaptcha|Zend_Form_Decorator_Captcha_ReCaptcha|

Posted by Thanos Kyritsis (djart) on 2011-07-26T16:45:22.000+0000

All right, how about this patch:

<pre class="highlight">
--- library/Zend/Form/Element/Captcha.php.orig  2011-07-26 19:02:59.473494443 +0300
+++ library/Zend/Form/Element/Captcha.php   2011-07-26 19:21:08.795165709 +0300
@@ -178,17 +178,21 @@
         $captcha    = $this->getCaptcha();
         $captcha->setName($this->getFullyQualifiedName());
 
-        $decorators = $this->getDecorators();
+        if (!$this->loadDefaultDecoratorsIsDisabled()) {
+            $decorators = $this->getDecorators();
 
-        $decorator  = $captcha->getDecorator();
-        if (!empty($decorator)) {
-            array_unshift($decorators, $decorator);
-        } else {
-            $decorator = array('Captcha', array('captcha' => $captcha));
-            array_unshift($decorators, $decorator);
-        }
+            $decorator  = $captcha->getDecorator();
+            if (!empty($decorator)) {
+                array_unshift($decorators, $decorator);
+            }
+
+            if ($captcha instanceof Zend_Captcha_Word) {
+                $decorator = array('Captcha', array('captcha' => $captcha));
+                array_unshift($decorators, $decorator);
+            }
 
-        $this->setDecorators($decorators);
+            $this->setDecorators($decorators);
+        }
 
         $this->setValue($this->getCaptcha()->generate());
 

It makes the render function look like this:

<pre class="highlight">
public function render(Zend_View_Interface $view = null)
    {
        $captcha    = $this->getCaptcha();
        $captcha->setName($this->getFullyQualifiedName());

        if (!$this->loadDefaultDecoratorsIsDisabled()) {
            $decorators = $this->getDecorators();

            $decorator  = $captcha->getDecorator();
            if (!empty($decorator)) {
                array_unshift($decorators, $decorator);
            }

            if ($captcha instanceof Zend_Captcha_Word) {
                $decorator = array('Captcha', array('captcha' => $captcha));
                array_unshift($decorators, $decorator);
            }

            $this->setDecorators($decorators);
        }

        $this->setValue($this->getCaptcha()->generate());

        return parent::render($view);
    }

Posted by Kai Uwe (kaiuwe) on 2011-07-27T09:00:19.000+0000

Should we check for existing captcha decorators?

<pre class="highlight">
/**
 * @group ZF-11609
 */
public function testDecoratorsBeforeAndAfterRendering()
{
    // Disable default decorators is true
    $element = new Zend_Form_Element_Captcha(
        'foo',
        array(
            'captcha'        => 'Dumb',
            'captchaOptions' => array(
                'sessionClass' => 'Zend_Form_Element_CaptchaTest_SessionContainer',
            ),
            'disableLoadDefaultDecorators' => true,
            'decorators'                   => array(
                'Description',
                'Errors',
                'Captcha_Word',
                'Captcha',
                'Label',
            ),
        )
    );

    // Before rendering
    $decorators = array_keys($element->getDecorators());
    $this->assertSame(
        array(
            'Zend_Form_Decorator_Description',
            'Zend_Form_Decorator_Errors',
            'Zend_Form_Decorator_Captcha_Word',
            'Zend_Form_Decorator_Captcha',
            'Zend_Form_Decorator_Label',
        ),
        $decorators,
        var_export($decorators, true)
    );

    $element->render();

    // After rendering
    $decorators = array_keys($element->getDecorators());
    $this->assertSame(
        array(
            'Zend_Form_Decorator_Description',
            'Zend_Form_Decorator_Errors',
            'Zend_Form_Decorator_Captcha_Word',
            'Zend_Form_Decorator_Captcha',
            'Zend_Form_Decorator_Label',
        ),
        $decorators,
        var_export($decorators, true)
    );

    // Disable default decorators is false
    $element = new Zend_Form_Element_Captcha(
        'foo',
        array(
            'captcha'        => 'Dumb',
            'captchaOptions' => array(
                'sessionClass' => 'Zend_Form_Element_CaptchaTest_SessionContainer',
            ),
            'decorators' => array(
                'Description',
                'Errors',
                'Captcha_Word',
                'Captcha',
                'Label',
            ),
        )
    );

    // Before rendering
    $decorators = array_keys($element->getDecorators());
    $this->assertSame(
        array(
            'Zend_Form_Decorator_Description',
            'Zend_Form_Decorator_Errors',
            'Zend_Form_Decorator_Captcha_Word',
            'Zend_Form_Decorator_Captcha',
            'Zend_Form_Decorator_Label',
        ),
        $decorators,
        var_export($decorators, true)
    );

    $element->render();
    
    // After rendering
    $decorators = array_keys($element->getDecorators());
    $this->assertSame(
        array(
            'Zend_Form_Decorator_Description',
            'Zend_Form_Decorator_Errors',
            'Zend_Form_Decorator_Captcha_Word',
            'Zend_Form_Decorator_Captcha',
            'Zend_Form_Decorator_Label',
        ),
        $decorators,
        var_export($decorators, true)
    );
}



<pre class="highlight">
There was 1 failure:

1) Zend_Form_Element_CaptchaTest::testDecoratorsBeforeAndAfterRendering
array (
  0 => 'Zend_Form_Decorator_Captcha',
  1 => 'Zend_Form_Decorator_Captcha_Word',
  2 => 'Zend_Form_Decorator_Description',
  3 => 'Zend_Form_Decorator_Errors',
  4 => 'Zend_Form_Decorator_Label',
)
--- Expected
+++ Actual
@@ @@
 Array
 (
-    [0] => Zend_Form_Decorator_Description
-    [1] => Zend_Form_Decorator_Errors
-    [2] => Zend_Form_Decorator_Captcha_Word
-    [3] => Zend_Form_Decorator_Captcha
+    [0] => Zend_Form_Decorator_Captcha
+    [1] => Zend_Form_Decorator_Captcha_Word
+    [2] => Zend_Form_Decorator_Description
+    [3] => Zend_Form_Decorator_Errors
     [4] => Zend_Form_Decorator_Label
 )

Posted by Thanos Kyritsis (djart) on 2011-07-27T10:31:56.000+0000

(I would upload the patch as an attachment, but perhaps I don't have the permissions to do so). Right, here is an improved version that should not fail the above Unit test (it checks for existing captcha decorators):

<pre class="highlight">
--- library/Zend/Form/Element/Captcha.php.orig  2011-07-26 19:02:59.473494443 +0300
+++ library/Zend/Form/Element/Captcha.php   2011-07-27 13:17:00.216725827 +0300
@@ -178,17 +178,24 @@
         $captcha    = $this->getCaptcha();
         $captcha->setName($this->getFullyQualifiedName());
 
-        $decorators = $this->getDecorators();
+        if (!$this->loadDefaultDecoratorsIsDisabled()) {
+            $decorators = $this->getDecorators();
+            $decorator  = $captcha->getDecorator();
+            $key        = get_class($this->_getDecorator($decorator, null));
+
+            if (!empty($decorator) && !array_key_exists($key, $decorators)) {
+                array_unshift($decorators, $decorator);
+            }
 
-        $decorator  = $captcha->getDecorator();
-        if (!empty($decorator)) {
-            array_unshift($decorators, $decorator);
-        } else {
             $decorator = array('Captcha', array('captcha' => $captcha));
-            array_unshift($decorators, $decorator);
-        }
+            $key       = get_class($this->_getDecorator($decorator[0], $decorator[1]));
 
-        $this->setDecorators($decorators);
+            if ($captcha instanceof Zend_Captcha_Word && !array_key_exists($key, $decorators)) {
+                array_unshift($decorators, $decorator);
+            }
+
+            $this->setDecorators($decorators);
+        }
 
         $this->setValue($this->getCaptcha()->generate());
 

It makes the render function look like this:

<pre class="highlight">
    public function render(Zend_View_Interface $view = null)
    {
        $captcha    = $this->getCaptcha();
        $captcha->setName($this->getFullyQualifiedName());

        if (!$this->loadDefaultDecoratorsIsDisabled()) {
            $decorators = $this->getDecorators();
            $decorator  = $captcha->getDecorator();
            $key        = get_class($this->_getDecorator($decorator, null));

            if (!empty($decorator) && !array_key_exists($key, $decorators)) {
                array_unshift($decorators, $decorator);
            }

            $decorator = array('Captcha', array('captcha' => $captcha));
            $key       = get_class($this->_getDecorator($decorator[0], $decorator[1]));

            if ($captcha instanceof Zend_Captcha_Word && !array_key_exists($key, $decorators)) {
                array_unshift($decorators, $decorator);
            }

            $this->setDecorators($decorators);
        }

        $this->setValue($this->getCaptcha()->generate());

        return parent::render($view);
    }

Posted by Kai Uwe (kaiuwe) on 2011-07-27T12:47:01.000+0000

Fix and unit tests

Posted by Adam Lundrigan (adamlundrigan) on 2011-07-27T13:23:09.000+0000

@Thanos Kyritsis You will need to sign and submit a CLA before you can attach your patch to this issue See here: http://framework.zend.com/wiki/display/…

Posted by Kai Uwe (kaiuwe) on 2011-07-27T13:37:10.000+0000

Hi Adam, Thanos wrote the patch. I've tested the patch, write the unit tests and added both files.

Posted by Matthew Weier O'Phinney (matthew) on 2011-07-28T21:39:09.000+0000

Patches reviewed and applied to both trunk and 1.11 release branch.

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