fix(floatfield,integerfield,textfield): fix escaping in regex

Signed-off-by: Thierry Bugier <tbugier@teclib.com>
pluginsGLPI · Nov 18, 2019 · ca0eb4e · ca0eb4e
1 parent e3ae8f3
commit ca0eb4e
Show file tree

Hide file tree

Showing 7 changed files with 114 additions and 24 deletions.
diff --git a/inc/field.class.php b/inc/field.class.php
@@ -210,4 +210,19 @@ public final function deleteParameters(PluginFormcreatorQuestion $question) {
    public function getQuestionId() {
       return $this->fields['id'];
    }
+
+   /**
+    * Validate a regular expression
+    *
+    * @param string $regex
+    * @return boolean true if the regex is valid, false otherwise
+    */
+   protected function checkRegex($regex) {
+      // Avoid php notice when validating the regular expression
+      set_error_handler(function($errno, $errstr, $errfile, $errline, $errcontext) {});
+      $isValid = !(preg_match($regex, null) === false);
+      restore_error_handler();
+
+      return $isValid;
+   }
 }
diff --git a/inc/fields/floatfield.class.php b/inc/fields/floatfield.class.php
@@ -103,6 +103,10 @@ public function isValid() {
    }
 
    private function isValidValue($value) {
+      if (strlen($value) == 0) {
+         return true;
+      }
+
       if (!empty($value) && !is_numeric($value)) {
          Session::addMessageAfterRedirect(__('This is not a number:', 'formcreator') . ' ' . $this->fields['name'], false, ERROR);
          return false;
@@ -150,14 +154,10 @@ public function prepareQuestionInputForSave($input) {
       $fieldType = $this->getFieldTypeName();
       // Add leading and trailing regex marker automaticaly
       if (isset($input['_parameters'][$fieldType]['regex']['regex']) && !empty($input['_parameters'][$fieldType]['regex']['regex'])) {
-         // Avoid php notice when validating the regular expression
-         set_error_handler(function($errno, $errstr, $errfile, $errline, $errcontext) {});
-         $isValid = !(preg_match($input['_parameters'][$fieldType]['regex']['regex'], null) === false);
-         restore_error_handler();
-
-         if (!$isValid) {
+         $regex = Toolbox::stripslashes_deep($input['_parameters'][$fieldType]['regex']['regex']);
+         $success = $this->checkRegex($regex);
+         if (!$success) {
             Session::addMessageAfterRedirect(__('The regular expression is invalid', 'formcreator'), false, ERROR);
-            $success = false;
          }
       }
       if (!$success) {

diff --git a/inc/fields/integerfield.class.php b/inc/fields/integerfield.class.php
@@ -104,6 +104,10 @@ public function isValid() {
    }
 
    public function isValidValue($value) {
+      if (strlen($value) == 0) {
+         return true;
+      }
+
       if (!empty($value) && !ctype_digit($value)) {
          Session::addMessageAfterRedirect(__('This is not an integer:', 'formcreator') . ' ' . $this->fields['name'], false, ERROR);
          return false;
@@ -151,14 +155,10 @@ public function prepareQuestionInputForSave($input) {
       $fieldType = $this->getFieldTypeName();
       // Add leading and trailing regex marker automaticaly
       if (isset($input['_parameters'][$fieldType]['regex']['regex']) && !empty($input['_parameters'][$fieldType]['regex']['regex'])) {
-         // Avoid php notice when validating the regular expression
-         set_error_handler(function($errno, $errstr, $errfile, $errline, $errcontext) {});
-         $isValid = !(preg_match($input['_parameters'][$fieldType]['regex']['regex'], null) === false);
-         restore_error_handler();
-
-         if (!$isValid) {
+         $regex = Toolbox::stripslashes_deep($input['_parameters'][$fieldType]['regex']['regex']);
+         $success = $this->checkRegex($regex);
+         if (!$success) {
             Session::addMessageAfterRedirect(__('The regular expression is invalid', 'formcreator'), false, ERROR);
-            $success = false;
          }
       }
       if (!$success) {

diff --git a/inc/fields/textfield.class.php b/inc/fields/textfield.class.php
@@ -105,6 +105,10 @@ public function isValid() {
    }
 
    private function isValidValue($value) {
+      if (strlen($value) == 0) {
+         return true;
+      }
+
       $parameters = $this->getParameters();
 
       // Check the field matches the format regex
@@ -141,14 +145,10 @@ public function prepareQuestionInputForSave($input) {
       $fieldType = $this->getFieldTypeName();
       // Add leading and trailing regex marker automaticaly
       if (isset($input['_parameters'][$fieldType]['regex']['regex']) && !empty($input['_parameters'][$fieldType]['regex']['regex'])) {
-         // Avoid php notice when validating the regular expression
-         set_error_handler(function($errno, $errstr, $errfile, $errline, $errcontext) {});
-         $isValid = !(preg_match($input['_parameters'][$fieldType]['regex']['regex'], null) === false);
-         restore_error_handler();
-
-         if (!$isValid) {
+         $regex = Toolbox::stripslashes_deep($input['_parameters'][$fieldType]['regex']['regex']);
+         $success = $this->checkRegex($regex);
+         if (!$success) {
             Session::addMessageAfterRedirect(__('The regular expression is invalid', 'formcreator'), false, ERROR);
-            $success = false;
          }
       }
       if (!$success) {

diff --git a/tests/suite-unit/PluginFormcreatorFloatField.php b/tests/suite-unit/PluginFormcreatorFloatField.php
@@ -157,6 +157,30 @@ public function provider() {
             'expectedValue'   => '3.141592',
             'expectedIsValid' => true
          ],
+         [
+            'fields'          => [
+               'fieldtype'       => 'float',
+               'name'            => 'question',
+               'required'        => '0',
+               'default_values'  => "",
+               'order'           => '1',
+               'show_rule'       => 'always',
+               'show_empty'      => '0',
+               'values'          => '',
+               '_parameters'     => [
+                  'float' => [
+                     'range' => [
+                        'range_min'       => '',
+                        'range_max'       => '',
+                     ],
+                     'regex' => ['regex' => '/[0-9]{2}\\\\.[0-9]{3}\\\\.[0-9]{3}\\\\/[0-9]{4}-[0-9]{2}/'],
+                  ]
+               ]
+            ],
+            'data'            => null,
+            'expectedValue'   => '',
+            'expectedIsValid' => true
+         ],
       ];
 
       return $dataset;

diff --git a/tests/suite-unit/PluginFormcreatorIntegerField.php b/tests/suite-unit/PluginFormcreatorIntegerField.php
@@ -181,6 +181,30 @@ public function provider() {
             'expectedValue'   => '4',
             'expectedIsValid' => true
          ],
+         [
+            'fields'          => [
+               'fieldtype'       => 'integer',
+               'name'            => 'question',
+               'required'        => '0',
+               'default_values'  => '',
+               'order'           => '1',
+               'show_rule'       => 'always',
+               'show_empty'      => '0',
+               'values'          => '',
+               '_parameters'     => [
+                  'integer' => [
+                     'range' => [
+                        'range_min' => '',
+                        'range_max' => '',
+                     ],
+                     'regex' => ['regex' => '/[0-9]{2}\\\\.[0-9]{3}\\\\.[0-9]{3}\\\\/[0-9]{4}-[0-9]{2}/'],
+                  ]
+               ],
+            ],
+            'data'            => null,
+            'expectedValue'   => '4',
+            'expectedIsValid' => true
+         ],
       ];
 
       return $dataset;

diff --git a/tests/suite-unit/PluginFormcreatorTextField.php b/tests/suite-unit/PluginFormcreatorTextField.php
@@ -166,6 +166,32 @@ public function provider() {
             'expectedValue'   => '1',
             'expectedIsValid' => false
          ],
+         'regex with escaped chars' => [
+            'fields'          => [
+               'fieldtype'       => 'text',
+               'name'            => 'question',
+               'required'        => '0',
+               'show_empty'      => '0',
+               'default_values'  => '',
+               'values'          => "",
+               'order'           => '1',
+               'show_rule'       => 'good',
+               '_parameters'     => [
+                  'text' => [
+                     'range' => [
+                        'range_min' => '',
+                        'range_max' => '',
+                     ],
+                     'regex' => [
+                        'regex' => '/[0-9]{2}\\\\.[0-9]{3}\\\\.[0-9]{3}\\\\/[0-9]{4}-[0-9]{2}/'
+                     ]
+                  ]
+               ],
+            ],
+            'data'            => null,
+            'expectedValue'   => '',
+            'expectedIsValid' => true
+         ],
       ];
       return $dataset;
    }
@@ -181,10 +207,11 @@ public function testFieldIsValid($fields, $data, $expectedValue, $expectedValidi
       $question->add($fields);
       $question->updateParameters($fields);
 
-      $fieldInstance = new \PluginFormcreatorTextField($question->fields, $data);
+      $instance = new \PluginFormcreatorTextField($question->fields, $data);
+      $instance->deserializeValue($fields['default_values']);
 
-      $isValid = $fieldInstance->isValid($fields['default_values']);
-      $this->boolean($isValid)->isEqualTo($expectedValidity, json_encode($_SESSION['MESSAGE_AFTER_REDIRECT'], JSON_PRETTY_PRINT));
+      $isValid = $instance->isValid();
+      $this->boolean((boolean) $isValid)->isEqualTo($expectedValidity);
    }
 
    public function testGetEmptyParameters() {