Skip to content

Commit

Permalink
fix(floatfield,integerfield,textfield): fix escaping in regex
Browse files Browse the repository at this point in the history
Signed-off-by: Thierry Bugier <tbugier@teclib.com>
  • Loading branch information
btry committed Nov 15, 2019
1 parent 8a54258 commit 4a12235
Show file tree
Hide file tree
Showing 7 changed files with 116 additions and 28 deletions.
15 changes: 15 additions & 0 deletions inc/field.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -269,4 +269,19 @@ protected function getParametersHtmlForDesign() {
public function getQuestionId() {
return $this->question->getID();
}

/**
* 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;
}
}
14 changes: 7 additions & 7 deletions inc/fields/floatfield.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,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->question->fields['name'], false, ERROR);
return false;
Expand Down Expand Up @@ -181,14 +185,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) {
Expand Down
14 changes: 7 additions & 7 deletions inc/fields/integerfield.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,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->question->fields['name'], false, ERROR);
return false;
Expand Down Expand Up @@ -182,14 +186,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) {
Expand Down
14 changes: 7 additions & 7 deletions inc/fields/textfield.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ public function isValid() {
}

private function isValidValue($value) {
if (strlen($value) == 0) {
return true;
}

$parameters = $this->getParameters();

// Check the field matches the format regex
Expand Down Expand Up @@ -174,14 +178,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) {
Expand Down
24 changes: 24 additions & 0 deletions tests/suite-unit/PluginFormcreatorFloatField.php
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,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;
Expand Down
28 changes: 25 additions & 3 deletions tests/suite-unit/PluginFormcreatorIntegerField.php
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,30 @@ public function providerIsValid() {
'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;
Expand All @@ -185,9 +209,7 @@ public function testIsValid($fields, $expectedValue, $expectedValidity) {
$section = $this->getSection();
$fields[$section::getForeignKeyField()] = $section->getID();

$question = new \PluginFormcreatorQuestion();
$question->add($fields);
$question->updateParameters($fields);
$question = $this->getQuestion($fields);

$instance = new \PluginFormcreatorIntegerField($question);
$instance->deserializeValue($fields['default_values']);
Expand Down
35 changes: 31 additions & 4 deletions tests/suite-unit/PluginFormcreatorTextField.php
Original file line number Diff line number Diff line change
Expand Up @@ -160,23 +160,50 @@ 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;
}

/**
* @dataProvider provider
*/
public function testFieldIsValid($fields, $expectedValue, $expectedValidity) {
public function testIsValid($fields, $expectedValue, $expectedValidity) {
$section = $this->getSection();
$fields[$section::getForeignKeyField()] = $section->getID();

$question = $this->getQuestion($fields);

$fieldInstance = new \PluginFormcreatorTextField($question);
$instance = new \PluginFormcreatorTextField($question);
$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, json_encode($_SESSION['MESSAGE_AFTER_REDIRECT'], JSON_PRETTY_PRINT));
}

public function testGetEmptyParameters() {
Expand Down

0 comments on commit 4a12235

Please sign in to comment.