Fixed issue #14271: Some words used by LS can be used as question codes

Dev: subquestion already fixed : time and other Dev: don't see for answer ?
LimeSurvey · Nov 22, 2018 · a8e3c8a · a8e3c8a
1 parent 0c41165
commit a8e3c8a
Showing 1 changed file with 39 additions and 18 deletions.
diff --git a/application/models/Question.php b/application/models/Question.php
@@ -99,10 +99,11 @@ public function rules()
                     array('title', 'length', 'min' => 1, 'max'=>20, 'on' => 'update, insert'),
                     array('qid,sid,gid,parent_qid', 'numerical', 'integerOnly'=>true),
                     array('qid', 'unique', 'criteria'=>array(
-                                    'condition'=>'language=:language',
-                                    'params'=>array(':language'=>$this->language)
-                            ),
-                            'message'=>'{attribute} "{value}" is already in use.'),
+                            'condition'=>'language=:language',
+                            'params'=>array(':language'=>$this->language)
+                        ),
+                        'message'=>sprintf(gT("Question id (qid) : '%s' is already in use."),$this->qid),// Usage of {attribute} need attributeLabels, {value} never exist in message
+                    ),
                     array('language', 'length', 'min' => 2, 'max'=>20), // in array languages ?
                     array('title,question,help', 'LSYii_Validators'),
                     array('other', 'in', 'range'=>array('Y', 'N'), 'allowEmpty'=>true),
@@ -116,16 +117,18 @@ public function rules()
                 );
         // Always enforce unicity on Sub question code (DB issue).
         if ($this->parent_qid) {
-            $aRules[] = array('title', 'unique', 'caseSensitive'=>false, 'criteria'=>array(
-                                'condition' => 'language=:language AND sid=:sid AND parent_qid=:parent_qid and scale_id=:scale_id',
-                                'params' => array(
-                                    ':language' => $this->language,
-                                    ':sid' => $this->sid,
-                                    ':parent_qid' => $this->parent_qid,
-                                    ':scale_id' => $this->scale_id
-                                    )
-                                ),
-                            'message' => gT('Subquestion codes must be unique.'));
+            $aRules[] = array('title', 'unique', 'caseSensitive'=>false,
+                'criteria'=>array(
+                    'condition' => 'language=:language AND sid=:sid AND parent_qid=:parent_qid and scale_id=:scale_id',
+                    'params' => array(
+                        ':language' => $this->language,
+                        ':sid' => $this->sid,
+                        ':parent_qid' => $this->parent_qid,
+                        ':scale_id' => $this->scale_id
+                    )
+                ),
+                'message' => gT('Subquestion codes must be unique.')
+            );
             // Disallow other title if question allow other
             $oParentQuestion = Question::model()->findByPk(array("qid"=>$this->parent_qid, 'language'=>$this->language));
             if ($oParentQuestion->other == "Y") {
@@ -141,11 +144,14 @@ public function rules()
         if (!$this->isNewRecord) {
             $oActualValue = Question::model()->findByPk(array("qid"=>$this->qid, 'language'=>$this->language));
             if ($oActualValue && $oActualValue->title == $this->title) {
-                return $aRules; // We don't change title, then don't put rules on title
+                /* We don't change title, then don't put rules on title */
+                /* We don't want to broke existing survey,  We only disallow to set it or update it according to this value */
+                return $aRules;
             }
         }
-        // 0 or empty
-        if (!$this->parent_qid) {
+        /* Question was new or title was updated : we add minor rules. This rules don't broke DB, only potential “Expression Manager” issue. */
+        if (!$this->parent_qid) { // 0 or empty
+            /* Unicity for ExpressionManager */
             $aRules[] = array('title', 'unique', 'caseSensitive'=>true,
                 'criteria'=>array(
                     'condition' => 'language=:language AND sid=:sid AND parent_qid=0',
@@ -157,9 +163,24 @@ public function rules()
                 'message' => gT('Question codes must be unique.'),
                 'except' => 'archiveimport'
             );
+            /* ExpressionManager basic rule */
             $aRules[] = array('title', 'match', 'pattern' => '/^[a-z,A-Z][[:alnum:]]*$/',
                 'message' => gT('Question codes must start with a letter and may only contain alphanumeric characters.'),
-                'except' => 'archiveimport');
+                'except' => 'archiveimport'
+            );
+            /* ExpressionManager reserved word (partial) */
+            $aRules[] = array('title', 'in', 'not' => true,
+                'range' => array(
+                    'LANG',
+                    'SID',
+                    'QID',
+                    'GID',
+                    'SAVEDID',
+                    'SGQ',
+                ),
+                'message'=> sprintf(gT('Code : %s is a reserved word.'),$this->title),// Usage of {attribute} need attributeLabels, {value} never exist in message
+                'except' => 'archiveimport'
+            );
         } else {
             $aRules[] = array('title', 'compare', 'compareValue'=>'time', 'operator'=>'!=',
                 'message'=> gT("'time' is a reserved word and can not be used for a subquestion."),