From 598fb3e593b5adcc88fa4c93271abc196df71abc Mon Sep 17 00:00:00 2001 From: Jan Puziewicz Date: Wed, 5 Mar 2014 13:40:31 +0100 Subject: [PATCH 1/6] debugging test --- tests/library/CM/FormField/TextTest.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/library/CM/FormField/TextTest.php b/tests/library/CM/FormField/TextTest.php index 48bb87256..10a59bf94 100644 --- a/tests/library/CM/FormField/TextTest.php +++ b/tests/library/CM/FormField/TextTest.php @@ -119,11 +119,16 @@ public function testValidateIsEmpty() { * checks if an invalid UTF char is properly converted to substitute character */ public function testValidateInvalidUTF() { + ini_set('mbstring.substitute_character', "z"); $substituteChar = ini_get('mbstring.substitute_character'); + var_dump($substituteChar); + + $field = new CM_FormField_Text(); $response = $this->getMockBuilder('CM_Response_View_Form')->disableOriginalConstructor()->getMockForAbstractClass(); $render = new CM_Render(); - $validated = $field->validate(chr(220), $response); + $validated = $field->validate(chr(192), $response); + var_dump($validated); $this->assertEquals($validated, $substituteChar); } } From dbf40b8a4f5edad539ddae45ebae1d4976e978c0 Mon Sep 17 00:00:00 2001 From: Jan Puziewicz Date: Wed, 5 Mar 2014 14:38:08 +0100 Subject: [PATCH 2/6] debugging invalid utf char test --- tests/library/CM/FormField/TextTest.php | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/library/CM/FormField/TextTest.php b/tests/library/CM/FormField/TextTest.php index 10a59bf94..478f9edba 100644 --- a/tests/library/CM/FormField/TextTest.php +++ b/tests/library/CM/FormField/TextTest.php @@ -119,16 +119,12 @@ public function testValidateIsEmpty() { * checks if an invalid UTF char is properly converted to substitute character */ public function testValidateInvalidUTF() { - ini_set('mbstring.substitute_character', "z"); - $substituteChar = ini_get('mbstring.substitute_character'); - var_dump($substituteChar); - - + //mb_substitute_character(ord('?')); + $substituteChar = chr(mb_substitute_character()); $field = new CM_FormField_Text(); $response = $this->getMockBuilder('CM_Response_View_Form')->disableOriginalConstructor()->getMockForAbstractClass(); $render = new CM_Render(); $validated = $field->validate(chr(192), $response); - var_dump($validated); $this->assertEquals($validated, $substituteChar); } } From d5d3ecfa79b8952266af24d5a20497f42cdb2ecc Mon Sep 17 00:00:00 2001 From: Jan Puziewicz Date: Wed, 5 Mar 2014 16:07:21 +0100 Subject: [PATCH 3/6] new tests for bad UTF removal --- library/CM/Form/Abstract.php | 33 ++++++++++++++---------- library/CM/FormField/Text.php | 3 +-- tests/library/CM/Form/AbstractTest.php | 34 ++++++++++++++++++++++--- tests/library/CM/FormField/TextTest.php | 11 ++++---- 4 files changed, 55 insertions(+), 26 deletions(-) diff --git a/library/CM/Form/Abstract.php b/library/CM/Form/Abstract.php index cab2e7d34..1911df95a 100755 --- a/library/CM/Form/Abstract.php +++ b/library/CM/Form/Abstract.php @@ -141,22 +141,28 @@ public function process(array $data, $actionName, CM_Response_View_Form $respons $formData = array(); foreach ($action->getFieldList() as $fieldName => $required) { $field = $this->getField($fieldName); - - if (array_key_exists($fieldName, $data) && !$field->isEmpty($data[$fieldName])) { - try { - $formData[$fieldName] = $field->validate($data[$fieldName], $response); - } catch (CM_Exception_FormFieldValidation $e) { - $response->addError($e->getMessagePublic($response->getRender()), $fieldName); - } - } else { - if ($required) { - $response->addError($response->getRender()->getTranslation('Required'), $fieldName); - } else { - $formData[$fieldName] = null; + $formData[$fieldName] = null; + + $isEmpty = true; + if (array_key_exists($fieldName, $data)) { + // get rid of broken UTF chars + $fieldValue = mb_convert_encoding($data[$fieldName], 'UTF-8', 'UTF-8'); // assuming always string + + if (!$field->isEmpty($fieldValue)) { + $isEmpty = false; + try { + $fieldValue = $field->validate($fieldValue, $response); + } catch (CM_Exception_FormFieldValidation $e) { + $response->addError($e->getMessagePublic($response->getRender()), $fieldName); + } } + $formData[$fieldName] = $fieldValue; } - } + if ($isEmpty && $required) { + $response->addError($response->getRender()->getTranslation('Required'), $fieldName); + } + } if (!$response->hasErrors()) { $action->checkData($formData, $response, $this); } @@ -164,7 +170,6 @@ public function process(array $data, $actionName, CM_Response_View_Form $respons if ($response->hasErrors()) { return null; } - return $action->process($formData, $response, $this); } } diff --git a/library/CM/FormField/Text.php b/library/CM/FormField/Text.php index 537e92264..6a52b8147 100755 --- a/library/CM/FormField/Text.php +++ b/library/CM/FormField/Text.php @@ -14,8 +14,7 @@ public function __construct($lengthMin = null, $lengthMax = null, $forbidBadword } public function validate($userInput, CM_Response_Abstract $response) { - // get rid of broken UTF chars - $userInput = mb_convert_encoding($userInput, 'UTF-8', 'UTF-8'); + if (isset($this->_options['lengthMax']) && mb_strlen($userInput) > $this->_options['lengthMax']) { throw new CM_Exception_FormFieldValidation('Too long'); diff --git a/tests/library/CM/Form/AbstractTest.php b/tests/library/CM/Form/AbstractTest.php index 3005e3d82..28f21a4a3 100644 --- a/tests/library/CM/Form/AbstractTest.php +++ b/tests/library/CM/Form/AbstractTest.php @@ -2,8 +2,12 @@ class CM_Form_AbstractTest extends CMTest_TestCase { + /** @var int */ public static $formActionProcessCount = 0; + /** @var CM_Params|null */ + public static $formActionData = null; + function testForm() { $data = $this->_getData(); self::$formActionProcessCount = 0; @@ -26,14 +30,34 @@ function testAllowedMissingField() { $this->assertFormResponseSuccess($response); } + function testProcessInvalidCharsRequired() { + foreach (array(chr(192), chr(214), chr(255), chr(140)) as $inputChar) { + $request = $this->getMockBuilder('CM_Request_Post')->setConstructorArgs(array('/form/null'))->setMethods(array('getQuery'))->getMock(); + $data = array('must_check' => 'checked', 'text' => $inputChar); + $query = array('data' => $data, 'actionName' => 'TestExampleAction', + 'form' => array('className' => 'CM_Form_MockForm', 'params' => array(), 'id' => 'mockFormId')); + $request->expects($this->any())->method('getQuery')->will($this->returnValue($query)); + /** @var CM_Request_Post $request */ + + $response = new CM_Response_View_Form($request); + $response->process(); + + try { + $this->assertFormResponseError($response, null, 'text'); + } catch (PHPUnit_Framework_AssertionFailedError $e) { + $this->assertNotSame($inputChar, self::$formActionData->getString('text')); + } + } + } + /** * @return array */ private function _getData() { return array( - "action" => "TestExampleAction", - "classname" => "CM_Form_MockForm", - "data" => array("color" => "#123123", "must_check" => "checked")); + 'action' => 'TestExampleAction', + 'classname' => 'CM_Form_MockForm', + 'data' => array('color' => '#123123', 'must_check' => 'checked', 'text' => 'foo')); } } @@ -42,6 +66,7 @@ class CM_Form_MockForm extends CM_Form_Abstract { public function setup() { $this->registerField('must_check', new CM_FormField_Boolean()); $this->registerField('color', new CM_FormField_Color()); + $this->registerField('text', new CM_FormField_Text()); $this->registerAction(new CM_FormAction_MockForm_TestExampleAction($this)); } } @@ -49,10 +74,11 @@ public function setup() { class CM_FormAction_MockForm_TestExampleAction extends CM_FormAction_Abstract { protected function _getRequiredFields() { - return array('must_check'); + return array('must_check', 'text'); } protected function _process(CM_Params $params, CM_Response_View_Form $response, CM_Form_Abstract $form) { CM_Form_AbstractTest::$formActionProcessCount++; + CM_Form_AbstractTest::$formActionData = $params; } } diff --git a/tests/library/CM/FormField/TextTest.php b/tests/library/CM/FormField/TextTest.php index 478f9edba..fd3f4c199 100644 --- a/tests/library/CM/FormField/TextTest.php +++ b/tests/library/CM/FormField/TextTest.php @@ -103,28 +103,27 @@ public function testValidateBadwords() { $this->fail('Badword-validation shouldn\'t be activated'); } } - /** * checks if an invalid chars that are stripped (as opposed to being converted to ? by mb_convert_encoding) + * + * NOTE: temporarily removed due to differences in how such chars are treated across PHP versions. */ public function testValidateIsEmpty() { $field = new CM_FormField_Text(); $response = $this->getMockBuilder('CM_Response_View_Form')->disableOriginalConstructor()->getMockForAbstractClass(); - $render = new CM_Render(); - $validated = $field->validate(chr(240), $response); + $validated = $field->validate(chr(9*16), $response); $this->assertTrue($field->isEmpty($validated)); } + /* * checks if an invalid UTF char is properly converted to substitute character */ public function testValidateInvalidUTF() { - //mb_substitute_character(ord('?')); $substituteChar = chr(mb_substitute_character()); $field = new CM_FormField_Text(); $response = $this->getMockBuilder('CM_Response_View_Form')->disableOriginalConstructor()->getMockForAbstractClass(); - $render = new CM_Render(); - $validated = $field->validate(chr(192), $response); + $validated = $field->validate(chr(255), $response); $this->assertEquals($validated, $substituteChar); } } From 240982ba41a4ceeabcd11596c2a9fdbbda911bb9 Mon Sep 17 00:00:00 2001 From: Jan Puziewicz Date: Wed, 5 Mar 2014 16:11:07 +0100 Subject: [PATCH 4/6] cleanup --- tests/library/CM/FormField/TextTest.php | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/tests/library/CM/FormField/TextTest.php b/tests/library/CM/FormField/TextTest.php index fd3f4c199..1d3df10c8 100644 --- a/tests/library/CM/FormField/TextTest.php +++ b/tests/library/CM/FormField/TextTest.php @@ -103,27 +103,4 @@ public function testValidateBadwords() { $this->fail('Badword-validation shouldn\'t be activated'); } } - /** - * checks if an invalid chars that are stripped (as opposed to being converted to ? by mb_convert_encoding) - * - * NOTE: temporarily removed due to differences in how such chars are treated across PHP versions. - */ - public function testValidateIsEmpty() { - $field = new CM_FormField_Text(); - $response = $this->getMockBuilder('CM_Response_View_Form')->disableOriginalConstructor()->getMockForAbstractClass(); - $validated = $field->validate(chr(9*16), $response); - $this->assertTrue($field->isEmpty($validated)); - } - - - /* - * checks if an invalid UTF char is properly converted to substitute character - */ - public function testValidateInvalidUTF() { - $substituteChar = chr(mb_substitute_character()); - $field = new CM_FormField_Text(); - $response = $this->getMockBuilder('CM_Response_View_Form')->disableOriginalConstructor()->getMockForAbstractClass(); - $validated = $field->validate(chr(255), $response); - $this->assertEquals($validated, $substituteChar); - } } From 0f08e4dec550814e2a8327ce58bca3d10a40abb1 Mon Sep 17 00:00:00 2001 From: Jan Puziewicz Date: Wed, 5 Mar 2014 17:37:50 +0100 Subject: [PATCH 5/6] cleanup + additional cast to string during validation --- library/CM/Form/Abstract.php | 4 ++-- library/CM/FormField/Text.php | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/library/CM/Form/Abstract.php b/library/CM/Form/Abstract.php index 1911df95a..8a92ebb0e 100755 --- a/library/CM/Form/Abstract.php +++ b/library/CM/Form/Abstract.php @@ -140,13 +140,13 @@ public function process(array $data, $actionName, CM_Response_View_Form $respons $formData = array(); foreach ($action->getFieldList() as $fieldName => $required) { - $field = $this->getField($fieldName); + $field = (string)$this->getField($fieldName); $formData[$fieldName] = null; $isEmpty = true; if (array_key_exists($fieldName, $data)) { // get rid of broken UTF chars - $fieldValue = mb_convert_encoding($data[$fieldName], 'UTF-8', 'UTF-8'); // assuming always string + $fieldValue = mb_convert_encoding($data[$fieldName], 'UTF-8', 'UTF-8'); if (!$field->isEmpty($fieldValue)) { $isEmpty = false; diff --git a/library/CM/FormField/Text.php b/library/CM/FormField/Text.php index 6a52b8147..2b0e00648 100755 --- a/library/CM/FormField/Text.php +++ b/library/CM/FormField/Text.php @@ -14,8 +14,6 @@ public function __construct($lengthMin = null, $lengthMax = null, $forbidBadword } public function validate($userInput, CM_Response_Abstract $response) { - - if (isset($this->_options['lengthMax']) && mb_strlen($userInput) > $this->_options['lengthMax']) { throw new CM_Exception_FormFieldValidation('Too long'); } From d2a4048b6d919a0b9dd034d3a33d21c52c82a65d Mon Sep 17 00:00:00 2001 From: Jan Puziewicz Date: Wed, 5 Mar 2014 17:57:23 +0100 Subject: [PATCH 6/6] fixed casting --- library/CM/Form/Abstract.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/library/CM/Form/Abstract.php b/library/CM/Form/Abstract.php index 8a92ebb0e..129c02ecd 100755 --- a/library/CM/Form/Abstract.php +++ b/library/CM/Form/Abstract.php @@ -140,13 +140,14 @@ public function process(array $data, $actionName, CM_Response_View_Form $respons $formData = array(); foreach ($action->getFieldList() as $fieldName => $required) { - $field = (string)$this->getField($fieldName); + $field = $this->getField($fieldName); $formData[$fieldName] = null; $isEmpty = true; if (array_key_exists($fieldName, $data)) { // get rid of broken UTF chars - $fieldValue = mb_convert_encoding($data[$fieldName], 'UTF-8', 'UTF-8'); + $fieldValue = (string)$data[$fieldName]; + $fieldValue = mb_convert_encoding($fieldValue, 'UTF-8', 'UTF-8'); if (!$field->isEmpty($fieldValue)) { $isEmpty = false;