HTMLForm: Allow distinguishing between form views and submission attempts

Calling HTMLForm::setFormIdentifier() will set an internal identifier
for this form. It will be submitted as a hidden form field, allowing
HTMLForm to determine whether the form was submitted (or just viewed).
Setting this serves two purposes:

* If you use two or more forms on one page, it allows HTMLForm to
  identify which of the forms was submitted, and not attempt to
  validate the other ones. (T102114)
* If you use checkbox or multiselect fields inside a form using the
  GET method, it allows HTMLForm to distinguish between the initial
  page view and a form submission with all checkboxes or select
  options unchecked. (T29676)

Bug: T102114
Bug: T29676
Change-Id: Ib6ce3fd8941be86211cff5c6932b5e84982490fa

diff --git a/includes/htmlform/HTMLCheckField.php b/includes/htmlform/HTMLCheckField.php
index 4a6b804..a553839 100644 (file)
--- a/includes/htmlform/HTMLCheckField.php
+++ b/includes/htmlform/HTMLCheckField.php
@@ -118,9 +118,9 @@ class HTMLCheckField extends HTMLFormField {
 
                // GetCheck won't work like we want for checks.
                // Fetch the value in either one of the two following case:
-               // - we have a valid token (form got posted or GET forged by the user)
+               // - we have a valid submit attempt (form was just submitted, or a GET URL forged by the user)
                // - checkbox name has a value (false or true), ie is not null
-               if ( $request->getCheck( 'wpEditToken' ) || $request->getVal( $this->mName ) !== null ) {
+               if ( $this->isSubmitAttempt( $request ) || $request->getVal( $this->mName ) !== null ) {
                        return $invert
                                ? !$request->getBool( $this->mName )
                                : $request->getBool( $this->mName );

diff --git a/includes/htmlform/HTMLCheckMatrix.php b/includes/htmlform/HTMLCheckMatrix.php
index 9f67233..b324fb6 100644 (file)
--- a/includes/htmlform/HTMLCheckMatrix.php
+++ b/includes/htmlform/HTMLCheckMatrix.php
@@ -225,22 +225,13 @@ class HTMLCheckMatrix extends HTMLFormField implements HTMLNestedFilterable {
         * @return array
         */
        function loadDataFromRequest( $request ) {
-               if ( $this->mParent->getMethod() == 'post' ) {
-                       if ( $request->wasPosted() ) {
-                               // Checkboxes are not added to the request arrays if they're not checked,
-                               // so it's perfectly possible for there not to be an entry at all
-                               return $request->getArray( $this->mName, [] );
-                       } else {
-                               // That's ok, the user has not yet submitted the form, so show the defaults
-                               return $this->getDefault();
-                       }
-               } else {
-                       // This is the impossible case: if we look at $_GET and see no data for our
-                       // field, is it because the user has not yet submitted the form, or that they
-                       // have submitted it with all the options unchecked. We will have to assume the
-                       // latter, which basically means that you can't specify 'positive' defaults
-                       // for GET forms.
+               if ( $this->isSubmitAttempt( $request ) ) {
+                       // Checkboxes are just not added to the request arrays if they're not checked,
+                       // so it's perfectly possible for there not to be an entry at all
                        return $request->getArray( $this->mName, [] );
+               } else {
+                       // That's ok, the user has not yet submitted the form, so show the defaults
+                       return $this->getDefault();
                }
        }
 

diff --git a/includes/htmlform/HTMLForm.php b/includes/htmlform/HTMLForm.php
index 8ac4cf2..2b7417e 100644 (file)
--- a/includes/htmlform/HTMLForm.php
+++ b/includes/htmlform/HTMLForm.php
@@ -190,6 +190,7 @@ class HTMLForm extends ContextSource {
        protected $mSubmitText;
        protected $mSubmitTooltip;
 
+       protected $mFormIdentifier;
        protected $mTitle;
        protected $mMethod = 'post';
        protected $mWasSubmitted = false;
@@ -480,7 +481,14 @@ class HTMLForm extends ContextSource {
                }
 
                # Load data from the request.
-               $this->loadData();
+               if (
+                       $this->mFormIdentifier === null ||
+                       $this->getRequest()->getVal( 'wpFormIdentifier' ) === $this->mFormIdentifier
+               ) {
+                       $this->loadData();
+               } else {
+                       $this->mFieldData = [];
+               }
 
                return $this;
        }
@@ -492,22 +500,29 @@ class HTMLForm extends ContextSource {
        public function tryAuthorizedSubmit() {
                $result = false;
 
-               $submit = false;
+               $identOkay = false;
+               if ( $this->mFormIdentifier === null ) {
+                       $identOkay = true;
+               } else {
+                       $identOkay = $this->getRequest()->getVal( 'wpFormIdentifier' ) === $this->mFormIdentifier;
+               }
+
+               $tokenOkay = false;
                if ( $this->getMethod() !== 'post' ) {
-                       $submit = true; // no session check needed
+                       $tokenOkay = true; // no session check needed
                } elseif ( $this->getRequest()->wasPosted() ) {
                        $editToken = $this->getRequest()->getVal( 'wpEditToken' );
                        if ( $this->getUser()->isLoggedIn() || $editToken !== null ) {
                                // Session tokens for logged-out users have no security value.
                                // However, if the user gave one, check it in order to give a nice
                                // "session expired" error instead of "permission denied" or such.
-                               $submit = $this->getUser()->matchEditToken( $editToken, $this->mTokenSalt );
+                               $tokenOkay = $this->getUser()->matchEditToken( $editToken, $this->mTokenSalt );
                        } else {
-                               $submit = true;
+                               $tokenOkay = true;
                        }
                }
 
-               if ( $submit ) {
+               if ( $tokenOkay && $identOkay ) {
                        $this->mWasSubmitted = true;
                        $result = $this->trySubmit();
                }
@@ -1042,6 +1057,12 @@ class HTMLForm extends ContextSource {
         */
        public function getHiddenFields() {
                $html = '';
+               if ( $this->mFormIdentifier !== null ) {
+                       $html .= Html::hidden(
+                               'wpFormIdentifier',
+                               $this->mFormIdentifier
+                       ) . "\n";
+               }
                if ( $this->getMethod() === 'post' ) {
                        $html .= Html::hidden(
                                'wpEditToken',
@@ -1327,6 +1348,27 @@ class HTMLForm extends ContextSource {
                return $this;
        }
 
+       /**
+        * Set an internal identifier for this form. It will be submitted as a hidden form field, allowing
+        * HTMLForm to determine whether the form was submitted (or merely viewed). Setting this serves
+        * two purposes:
+        *
+        * - If you use two or more forms on one page, it allows HTMLForm to identify which of the forms
+        *   was submitted, and not attempt to validate the other ones.
+        * - If you use checkbox or multiselect fields inside a form using the GET method, it allows
+        *   HTMLForm to distinguish between the initial page view and a form submission with all
+        *   checkboxes or select options unchecked.
+        *
+        * @since 1.28
+        * @param string $ident
+        * @return $this
+        */
+       public function setFormIdentifier( $ident ) {
+               $this->mFormIdentifier = $ident;
+
+               return $this;
+       }
+
        /**
         * Stop a default submit button being shown for this form. This implies that an
         * alternate submit method must be provided manually.

diff --git a/includes/htmlform/HTMLFormField.php b/includes/htmlform/HTMLFormField.php
index 9f5e728..7cb497d 100644 (file)
--- a/includes/htmlform/HTMLFormField.php
+++ b/includes/htmlform/HTMLFormField.php
@@ -349,6 +349,20 @@ abstract class HTMLFormField {
                $this->mShowEmptyLabels = $show;
        }
 
+       /**
+        * Can we assume that the request is an attempt to submit a HTMLForm, as opposed to an attempt to
+        * just view it? This can't normally be distinguished for e.g. checkboxes.
+        *
+        * Returns true if the request has a field for a CSRF token (wpEditToken) or a form identifier
+        * (wpFormIdentifier).
+        *
+        * @param WebRequest $request
+        * @return boolean
+        */
+       protected function isSubmitAttempt( WebRequest $request ) {
+               return $request->getCheck( 'wpEditToken' ) || $request->getCheck( 'wpFormIdentifier' );
+       }
+
        /**
         * Get the value that this input has been set to from a posted form,
         * or the input's default value if it has not been set.

diff --git a/includes/htmlform/HTMLMultiSelectField.php b/includes/htmlform/HTMLMultiSelectField.php
index 23125bd..a231b2f 100644 (file)
--- a/includes/htmlform/HTMLMultiSelectField.php
+++ b/includes/htmlform/HTMLMultiSelectField.php
@@ -123,23 +123,13 @@ class HTMLMultiSelectField extends HTMLFormField implements HTMLNestedFilterable
         * @return string
         */
        function loadDataFromRequest( $request ) {
-               if ( $this->mParent->getMethod() == 'post' ) {
-                       if ( $request->wasPosted() ) {
-                               # Checkboxes are just not added to the request arrays if they're not checked,
-                               # so it's perfectly possible for there not to be an entry at all
-                               return $request->getArray( $this->mName, [] );
-                       } else {
-                               # That's ok, the user has not yet submitted the form, so show the defaults
-                               return $this->getDefault();
-                       }
-               } else {
-                       # This is the impossible case: if we look at $_GET and see no data for our
-                       # field, is it because the user has not yet submitted the form, or that they
-                       # have submitted it with all the options unchecked? We will have to assume the
-                       # latter, which basically means that you can't specify 'positive' defaults
-                       # for GET forms.
-                       # @todo FIXME...
+               if ( $this->isSubmitAttempt( $request ) ) {
+                       // Checkboxes are just not added to the request arrays if they're not checked,
+                       // so it's perfectly possible for there not to be an entry at all
                        return $request->getArray( $this->mName, [] );
+               } else {
+                       // That's ok, the user has not yet submitted the form, so show the defaults
+                       return $this->getDefault();
                }
        }