Issues

ZF-12020: Modify Zend_Form::isValid() to ignore flagged elements

Description

http://framework.zend.com/issues/browse/ZF-2661 http://framework.zend.com/issues/browse/ZF-6909

In both of these older tickets, Matthew Weier O'Phinney states that by setting a form element's ignore status to 'true', both Zend_Form::getValues() and Zend_Form::isValid() will ignore that element. This is indeed the case for Zend_Form::getValues(), but Zend_Form::isValid() does not effectively ignore user input from elements that are supposed to be ignored. A glance at the source code confirms that the phrase 'ignore' appears nowhere in the isValid() function of either Zend_Form or Zend_Form_Element.

Say I have a read-only 'due date' field which is flagged to be ignored -- the user might manipulate this field using a browser such as Chrome, and when the isValid() method is called on the form, the value of 'due date' is set to the user-modified value. Not exactly the behavior one might expect.

Comments

I suppose there are instances where a developer may want to ignore an element when calling getValues() but still validate that element (CSRF, as mentioned by Adam Lundrigan. (http://framework.zend.com/issues/browse/ZF-10056)) Perhaps a new property/getter/setter can be defined? Something along the lines of $element->setProtected(true)?

Or might the populate() method be patched to NOT populate fields flagged as 'ignore'? This way, one might write:

if ($this->_request->getPost()){ $this->form->populate($this->_request->getPost()); if ($this->form->isValid()){ // Do whatever suits you here } }

Oh, that's right, isValid() requires an array. The form can't just check itself after being populated, I guess. Nevermind that idea.

I don't know how to upload patch files or whatever. But I see nobody is apparently checking these issues. Perhaps because they are busy developing ZF which I can greatly appreciate. :)

Anyways, what I have done is patched Zend_Form and Zend_Form_Element as such: In Zend_Form::isValid() and Zend_Form::isValidPartial(), following lines of code have been added:

    foreach ($this->getElements() as $key => $element) { //Included to show where I have inserted
        if ($element->getProtect()) {
            continue;
        }

And then in Zend_Form_Element, this has been added:

/**
 * Protect flag (causes an element to be ignored during validation)
 * @var bool
 */
protected $_protect = false;

/**
 * Set protect flag (causes an element to be ignored during validation)
 *
 * @param  bool $flag
 * @return Zend_Form_Element
 */
public function setProtect($flag)
{
    $this->_protect = (bool) $flag;
    return $this;
}

/**
 * Get protect flag (used when retrieving values at form level)
 *
 * @return bool
 */
public function getProtect()
{
    return $this->_protect;
}

If anyone knows how I can turn these into code blocks, please let me know. I hope the development team considers this addition and the functionality it will provide without conflicting with CSRF and other element that may need to be ignored while still validated.

{quote}If anyone knows how I can turn these into code blocks, please let me know.{quote} Press the question mark below the text area and you get the help for the markup.

Here is an example:

`

Thank you so much Frank. Now I will be able to provide a better look at the 'patch'.


    /**
     * Validate the form
     *
     * @param  array $data
     * @return boolean
     */
    public function isValid($data)
    {
        if (!is_array($data)) {
            require_once 'Zend/Form/Exception.php';
            throw new Zend_Form_Exception(__METHOD__ . ' expects an array');
        }
        $translator = $this->getTranslator();
        $valid      = true;
        $eBelongTo  = null;

        if ($this->isArray()) {
            $eBelongTo = $this->getElementsBelongTo();
            $data = $this->_dissolveArrayValue($data, $eBelongTo);
        }
        $context = $data;
        foreach ($this->getElements() as $key => $element) {
            if ($element->getProtect()) {
                continue;
            }
            if (null !== $translator && $this->hasTranslator()
                    && !$element->hasTranslator()) {
                $element->setTranslator($translator);
            }
            $check = $data;
            if (($belongsTo = $element->getBelongsTo()) !== $eBelongTo) {
                $check = $this->_dissolveArrayValue($data, $belongsTo);
            }
            if (!isset($check[$key])) {
                $valid = $element->isValid(null, $context) && $valid;
            } else {
                $valid = $element->isValid($check[$key], $context) && $valid;
                $data = $this->_dissolveArrayUnsetKey($data, $belongsTo, $key);
            }
        }
        foreach ($this->getSubForms() as $key => $form) {
            if (null !== $translator && !$form->hasTranslator()) {
                $form->setTranslator($translator);
            }
            if (isset($data[$key]) && !$form->isArray()) {
                $valid = $form->isValid($data[$key]) && $valid;
            } else {
                $valid = $form->isValid($data) && $valid;
            }
        }

        $this->_errorsExist = !$valid;

        // If manually flagged as an error, return invalid status
        if ($this->_errorsForced) {
            return false;
        }

        return $valid;
    }

    /**
     * Validate a partial form
     *
     * Does not check for required flags.
     *
     * @param  array $data
     * @return boolean
     */
    public function isValidPartial(array $data)
    {
        $eBelongTo  = null;

        if ($this->isArray()) {
            $eBelongTo = $this->getElementsBelongTo();
            $data = $this->_dissolveArrayValue($data, $eBelongTo);
        }

        $translator = $this->getTranslator();
        $valid      = true;
        $context    = $data;

        foreach ($this->getElements() as $key => $element) {
            if ($element->getProtect()) {
                continue;
            }
            $check = $data;
            if (($belongsTo = $element->getBelongsTo()) !== $eBelongTo) {
                $check = $this->_dissolveArrayValue($data, $belongsTo);
            }
            if (isset($check[$key])) {
                if (null !== $translator && !$element->hasTranslator()) {
                    $element->setTranslator($translator);
                }
                $valid = $element->isValid($check[$key], $context) && $valid;
                $data = $this->_dissolveArrayUnsetKey($data, $belongsTo, $key);
            }
        }
        foreach ($this->getSubForms() as $key => $form) {
            if (null !== $translator && !$form->hasTranslator()) {
                $form->setTranslator($translator);
            }
            if (isset($data[$key]) && !$form->isArray()) {
                $valid = $form->isValidPartial($data[$key]) && $valid;
            } else {
                $valid = $form->isValidPartial($data) && $valid;
            }
        }

        $this->_errorsExist = !$valid;
        return $valid;
    }

    /**
     * Protect flag (causes an element to be ignored during validation)
     * @var bool
     */
    protected $_protect = false;

    /**
     * Set protect flag (causes an element to be ignored during validation)
     *
     * @param  bool $flag
     * @return Zend_Form_Element
     */
    public function setProtect($flag)
    {
        $this->_protect = (bool) $flag;
        return $this;
    }

    /**
     * Get protect flag (used when retrieving values at form level)
     *
     * @return bool
     */
    public function getProtect()
    {
        return $this->_protect;
    }

Yeah... just checked the ZF 2.0.0 Beta 2 source and this feature is not there either.

Please, ZF team, consider this simple and unobtrusive improvement!

@Derek The work on Zend_Form 2.0 has not started yet!

Can you provide some unit tests for your patch? You must sign the CLA, otherwise your patch is not accepted.

I see. I will read about the CLA and unit testing right away. Thank you!

Okay. I will sign and submit the CLA. And honestly, Zend can have all rights to this. It's nothing. I looked at unit testing, and while intriguing, at this time I must put full focus on the application I am currently developing. I don't know when or if I could get to a unit test -- but this is an extremely simple patch.

Derek: Could you provide a diff against SVN trunk? I can take a look and see if it's something that we can push for ZF 1.12, and if so I don't mind writing the unit tests to go with it.

I believe that this will also be addressed in ZF2's Zend\Form component. The RFC (here) has this as part of it's design goals: {quote} * MUST allow partial validations ** SHOULD allow indicating which specific elements must be valid {quote}

Could you have a quick read-through that RFC to ensure that i'm correct in thinking that it takes into account the situation you're encounting?

That RFC does seem to address it. Instead of saying 'indicating which specific elements must be valid', what I have implemented is 'indicating which specific elements should not be validated/populated with user input.'

Here are this difs for 1.11.11:



--- Element.php 2012-02-25 17:17:19.000000000 -0600
+++ Element.php.orig    2012-02-25 17:07:04.000000000 -0600
@@ -121,12 +121,6 @@
      * @var bool
      */
     protected $_ignore = false;
-
-    /**
-     * Protect flag (causes an element to be ignored during validation)
-     * @var bool
-     */
-    protected $_protect = false;

     /**
      * Does the element represent an array?
@@ -783,28 +777,6 @@
     {
         return $this->_ignore;
     }
-
-    /**
-     * Set protect flag (causes an element to be ignored during validation)
-     *
-     * @param  bool $flag
-     * @return Zend_Form_Element
-     */
-    public function setProtect($flag)
-    {
-        $this->_protect = (bool) $flag;
-        return $this;
-    }
-
-    /**
-     * Get protect flag (used when retrieving values at form level)
-     *
-     * @return bool
-     */
-    public function getProtect()
-    {
-        return $this->_protect;
-    }

     /**
      * Set flag indicating if element represents an array

--- Form.php    2012-02-25 17:02:15.000000000 -0600
+++ Form.php.orig       2012-02-25 17:06:53.000000000 -0600
@@ -2238,9 +2238,6 @@
         }
         $context = $data;
         foreach ($this->getElements() as $key => $element) {
-            if ($element->getProtect()) {
-                continue;
-            }
             if (null !== $translator && $this->hasTranslator()
                     && !$element->hasTranslator()) {
                 $element->setTranslator($translator);
@@ -2299,9 +2296,6 @@
         $context    = $data;

         foreach ($this->getElements() as $key => $element) {
-            if ($element->getProtect()) {
-                continue;
-            }
             $check = $data;
             if (($belongsTo = $element->getBelongsTo()) !== $eBelongTo) {
                 $check = $this->_dissolveArrayValue($data, $belongsTo);

Thank you for your response and attention, I greatly appreciate it. :)