From 125bdf9e97e1a958b66dc93ae40a5760079cdf24 Mon Sep 17 00:00:00 2001 From: Oliver Hader <oliver@typo3.org> Date: Tue, 20 Nov 2012 12:20:35 +0100 Subject: [PATCH] [BUGFIX] Activating NULL value field does not work with blank string Activating a field that supports NULL values and just using a blank string ("") does not work. The problem is a strcmp() call that returns a false-positive on comparing NULL to blank strings Change-Id: I59417f5f5cd814db15e2b6b725f1778d098014f6 Fixes: #43139 Releases: 6.0 Reviewed-on: http://review.typo3.org/16599 Reviewed-by: Oliver Hader Tested-by: Oliver Hader --- .../core/Classes/DataHandling/DataHandler.php | 46 ++++- .../Unit/DataHandling/DataHandlerTest.php | 165 ++++++++++++++++++ 2 files changed, 206 insertions(+), 5 deletions(-) diff --git a/typo3/sysext/core/Classes/DataHandling/DataHandler.php b/typo3/sysext/core/Classes/DataHandling/DataHandler.php index a7ede19f759d..fe729432a42e 100644 --- a/typo3/sysext/core/Classes/DataHandling/DataHandler.php +++ b/typo3/sysext/core/Classes/DataHandling/DataHandler.php @@ -6022,12 +6022,12 @@ class DataHandler { $GLOBALS['TYPO3_DB']->sql_free_result($res); // Unset the fields which are similar: foreach ($fieldArray as $col => $val) { - $isAlreadyNull = ($val === NULL && $currentRecord[$col] === NULL); - $isNotNull = ($val !== NULL); - // Unset field values if they are empty, which is "0" for integer types and "" for string - except the current field holds MM relations. - // If NULL values shall be stored, then the value will only be unset if the current stored value is already NULL. + $fieldConfiguration = $GLOBALS['TCA'][$table]['columns'][$col]['config']; + $isNullField = (!empty($fieldConfiguration['eval']) && \TYPO3\CMS\Core\Utility\GeneralUtility::inList($fieldConfiguration['eval'], 'null')); + + // Unset fields if stored and submitted values are equal - except the current field holds MM relations. // In general this avoids to store superfluous data which also will be visualized in the editing history. - if (!$GLOBALS['TCA'][$table]['columns'][$col]['config']['MM'] && ($isAlreadyNull || $isNotNull && (!strcmp($val, $currentRecord[$col]) || $cRecTypes[$col] == 'int' && $currentRecord[$col] == 0 && !strcmp($val, '')))) { + if (!$fieldConfiguration['MM'] && $this->isSubmittedValueEqualToStoredValue($val, $currentRecord[$col], $cRecTypes[$col], $isNullField)) { unset($fieldArray[$col]); } else { if (!isset($this->mmHistoryRecords[($table . ':' . $id)]['oldRecord'][$col])) { @@ -6049,6 +6049,42 @@ class DataHandler { return $fieldArray; } + /** + * Determines whether submitted values and stored values are equal. + * This prevents from adding superfluous field changes which would be shown in the record history as well. + * For NULL fields (see accordant TCA definition 'eval' = 'null'), a special handling is required since + * (!strcmp(NULL, '')) would be a false-positive. + * + * @param mixed $submittedValue Value that has submitted (e.g. from a backend form) + * @param mixed $storedValue Value that is currently stored in the database + * @param string $storedType SQL type of the stored value column (see mysql_field_type(), e.g 'int', 'string', ...) + * @param boolean $allowNull Whether NULL values are allowed by accordant TCA definition ('eval' = 'null') + * @return boolean Whether both values are considered to be equal + */ + protected function isSubmittedValueEqualToStoredValue($submittedValue, $storedValue, $storedType, $allowNull = FALSE) { + // No NULL values are allowed, this is the regular behaviour. + // Thus, check whether strings are the same or whether integer values are empty ("0" or ""). + if (!$allowNull) { + $result = ( + !strcmp($submittedValue, $storedValue) + || $storedType == 'int' && $storedValue == 0 && !strcmp($submittedValue, '') + ); + // Null values are allowed, but currently there's a real (not NULL) value. + // Thus, ensure no NULL value was submitted and fallback to the regular behaviour. + } elseif ($storedValue !== NULL) { + $result = ( + $submittedValue !== NULL + && $this->isSubmittedValueEqualToStoredValue($submittedValue, $storedValue, $storedType, FALSE) + ); + // Null values are allowed, and currently there's a NULL value. + // Thus, check whether a NULL value was submitted. + } else { + $result = ($submittedValue === NULL); + } + + return $result; + } + /** * Calculates the bitvalue of the permissions given in a string, comma-sep * diff --git a/typo3/sysext/core/Tests/Unit/DataHandling/DataHandlerTest.php b/typo3/sysext/core/Tests/Unit/DataHandling/DataHandlerTest.php index 83a0e38ad339..8b5d20015d42 100644 --- a/typo3/sysext/core/Tests/Unit/DataHandling/DataHandlerTest.php +++ b/typo3/sysext/core/Tests/Unit/DataHandling/DataHandlerTest.php @@ -316,6 +316,171 @@ class DataHandlerTest extends \TYPO3\CMS\Core\Tests\UnitTestCase { $this->assertStringEndsWith($expected, $this->fixture->errorLog[0]); } + /** + * @param boolean $expected + * @param string $submittedValue + * @param string $storedValue + * @param string $storedType + * @param boolean $allowNull + * @dataProvider equalSubmittedAndStoredValuesAreDeterminedDataProvider + * @test + */ + public function equalSubmittedAndStoredValuesAreDetermined($expected, $submittedValue, $storedValue, $storedType, $allowNull) { + $result = $this->callInaccessibleMethod( + $this->fixture, + 'isSubmittedValueEqualToStoredValue', + $submittedValue, $storedValue, $storedType, $allowNull + ); + $this->assertEquals($expected, $result); + } + + /** + * @return array + */ + public function equalSubmittedAndStoredValuesAreDeterminedDataProvider() { + return array( + // String + 'string value "" vs. ""' => array( + TRUE, + '', '', 'string', FALSE + ), + 'string value 0 vs. "0"' => array( + TRUE, + 0, '0', 'string', FALSE + ), + 'string value 1 vs. "1"' => array( + TRUE, + 1, '1', 'string', FALSE + ), + 'string value "0" vs. ""' => array( + FALSE, + '0', '', 'string', FALSE + ), + 'string value 0 vs. ""' => array( + FALSE, + 0, '', 'string', FALSE + ), + 'string value null vs. ""' => array( + TRUE, + NULL, '', 'string', FALSE + ), + // Integer + 'integer value 0 vs. 0' => array( + TRUE, + 0, 0, 'int', FALSE + ), + 'integer value "0" vs. "0"' => array( + TRUE, + '0', '0', 'int', FALSE + ), + 'integer value 0 vs. "0"' => array( + TRUE, + 0, '0', 'int', FALSE + ), + 'integer value "" vs. "0"' => array( + TRUE, + '', '0', 'int', FALSE + ), + 'integer value 1 vs. 1' => array( + TRUE, + 1, 1, 'int', FALSE + ), + 'integer value 1 vs. "1"' => array( + TRUE, + 1, '1', 'int', FALSE + ), + 'integer value "0" vs. "1"' => array( + FALSE, + '0', '1', 'int', FALSE + ), + // String with allowed NULL values + 'string with allowed null value "" vs. ""' => array( + TRUE, + '', '', 'string', TRUE + ), + 'string with allowed null value 0 vs. "0"' => array( + TRUE, + 0, '0', 'string', TRUE + ), + 'string with allowed null value 1 vs. "1"' => array( + TRUE, + 1, '1', 'string', TRUE + ), + 'string with allowed null value "0" vs. ""' => array( + FALSE, + '0', '', 'string', TRUE + ), + 'string with allowed null value 0 vs. ""' => array( + FALSE, + 0, '', 'string', TRUE + ), + 'string with allowed null value null vs. ""' => array( + FALSE, + NULL, '', 'string', TRUE + ), + 'string with allowed null value "" vs. null' => array( + FALSE, + '', NULL, 'string', TRUE + ), + 'string with allowed null value null vs. null' => array( + TRUE, + NULL, NULL, 'string', TRUE + ), + // Integer with allowed NULL values + 'integer with allowed null value 0 vs. 0' => array( + TRUE, + 0, 0, 'int', TRUE + ), + 'integer with allowed null value "0" vs. "0"' => array( + TRUE, + '0', '0', 'int', TRUE + ), + 'integer with allowed null value 0 vs. "0"' => array( + TRUE, + 0, '0', 'int', TRUE + ), + 'integer with allowed null value "" vs. "0"' => array( + TRUE, + '', '0', 'int', TRUE + ), + 'integer with allowed null value 1 vs. 1' => array( + TRUE, + 1, 1, 'int', TRUE + ), + 'integer with allowed null value 1 vs. "1"' => array( + TRUE, + 1, '1', 'int', TRUE + ), + 'integer with allowed null value "0" vs. "1"' => array( + FALSE, + '0', '1', 'int', TRUE + ), + 'integer with allowed null value null vs. ""' => array( + FALSE, + NULL, '', 'int', TRUE + ), + 'integer with allowed null value "" vs. null' => array( + FALSE, + '', NULL, 'int', TRUE + ), + 'integer with allowed null value null vs. null' => array( + TRUE, + NULL, NULL, 'int', TRUE + ), + 'integer with allowed null value null vs. "0"' => array( + FALSE, + NULL, '0', 'int', TRUE + ), + 'integer with allowed null value "0" vs. null' => array( + FALSE, + '0', NULL, 'int', TRUE + ), + 'integer with allowed null value null vs. null' => array( + TRUE, + NULL, NULL, 'int', TRUE + ), + ); + } } ?> \ No newline at end of file -- GitLab