From 9a5ac5b21a0cfda921c52c63086973a4f4b16845 Mon Sep 17 00:00:00 2001 From: Nick Liu Date: Tue, 23 Jan 2018 07:48:24 -0600 Subject: [PATCH 1/3] Permissive field validation in db_verify Even if it's a new installation of e107, `db_verify` will be too strict in validating database field defaults that are equivalent to each other. This can happen on some MySQL or MariaDB servers that correct the field definitions, which were not strictly correct in the table structures built into e107 in the first place. This commit introduces db_verify::validateFieldPermissive(), a backwards-compatible way to validate database field defaults. It works regardless of whether the MySQL server stores a corrected representation of the schema and does not require fixing the schema in any existing table structure files. Fixes: #2998 --- e107_handlers/db_verify_class.php | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/e107_handlers/db_verify_class.php b/e107_handlers/db_verify_class.php index ac33cb980..70c8e0938 100644 --- a/e107_handlers/db_verify_class.php +++ b/e107_handlers/db_verify_class.php @@ -131,6 +131,29 @@ class db_verify + } + + /** + * Permissive field validation + */ + private function validateFieldPermissive($expected, $actual) + { + // Permit actual text types that default to null even when + // expected does not explicitly default to null + if(0 === strcasecmp($expected['type'], $actual['type']) && + 1 === preg_match('/[A-Z]*TEXT/i', $expected['type']) && + 0 === strcasecmp($actual['default'], "DEFAULT NULL")) + { + $expected['default'] = $actual['default']; + } + + // Loosely typed default value for int-like types + if(1 === preg_match('/[A-Z]*INT/i', $expected['type'])) + { + $expected['default'] = preg_replace("/DEFAULT '(\d+)'/i", 'DEFAULT $1', $expected['default']); + $actual['default'] = preg_replace("/DEFAULT '(\d+)'/i", 'DEFAULT $1', $actual['default'] ); + } + return !count($off = array_diff_assoc($expected, $actual)); } /** @@ -347,7 +370,7 @@ class db_verify $this->results[$tbl][$field]['_valid'] = $info; $this->results[$tbl][$field]['_file'] = $selection; } - elseif(count($off = array_diff_assoc($info,$sqlFieldData[$field]))) + elseif(!$this->validateFieldPermissive($info, $sqlFieldData[$field])) { $this->errors[$tbl]['_status'] = 'mismatch'; $this->results[$tbl][$field]['_status'] = 'mismatch'; @@ -1671,4 +1694,4 @@ function table_list() */ -?> \ No newline at end of file +?> From c959610f7d0f56416021a8b50f6451e0c216ceaa Mon Sep 17 00:00:00 2001 From: Nick Liu Date: Tue, 23 Jan 2018 08:38:31 -0600 Subject: [PATCH 2/3] Fixed regression from 9a5ac5b Regression fixed: Diff wasn't being set --- e107_handlers/db_verify_class.php | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/e107_handlers/db_verify_class.php b/e107_handlers/db_verify_class.php index 70c8e0938..3b4399600 100644 --- a/e107_handlers/db_verify_class.php +++ b/e107_handlers/db_verify_class.php @@ -136,7 +136,7 @@ class db_verify /** * Permissive field validation */ - private function validateFieldPermissive($expected, $actual) + private function diffFieldPermissive($expected, $actual) { // Permit actual text types that default to null even when // expected does not explicitly default to null @@ -153,7 +153,8 @@ class db_verify $expected['default'] = preg_replace("/DEFAULT '(\d+)'/i", 'DEFAULT $1', $expected['default']); $actual['default'] = preg_replace("/DEFAULT '(\d+)'/i", 'DEFAULT $1', $actual['default'] ); } - return !count($off = array_diff_assoc($expected, $actual)); + + return array_diff_assoc($expected, $actual); } /** @@ -370,11 +371,11 @@ class db_verify $this->results[$tbl][$field]['_valid'] = $info; $this->results[$tbl][$field]['_file'] = $selection; } - elseif(!$this->validateFieldPermissive($info, $sqlFieldData[$field])) + elseif(count($diff = $this->diffFieldPermissive($info, $sqlFieldData[$field]))) { $this->errors[$tbl]['_status'] = 'mismatch'; $this->results[$tbl][$field]['_status'] = 'mismatch'; - $this->results[$tbl][$field]['_diff'] = $off; + $this->results[$tbl][$field]['_diff'] = $diff; $this->results[$tbl][$field]['_valid'] = $info; $this->results[$tbl][$field]['_invalid'] = $sqlFieldData[$field]; $this->results[$tbl][$field]['_file'] = $selection; @@ -398,14 +399,14 @@ class db_verify $this->indices[$tbl][$field]['_valid'] = $info; $this->indices[$tbl][$field]['_file'] = $selection; } - elseif(count($offin = array_diff_assoc($info,$sqlIndexData[$field]))) // missmatch data + elseif(count($diff = $this->diffFieldPermissive($info, $sqlIndexData[$field]))) // mismatched data { // print_a($info); // print_a($sqlIndexData[$field]); $this->errors[$tbl]['_status'] = 'mismatch_index'; $this->indices[$tbl][$field]['_status'] = 'mismatch'; - $this->indices[$tbl][$field]['_diff'] = $offin; + $this->indices[$tbl][$field]['_diff'] = $diff; $this->indices[$tbl][$field]['_valid'] = $info; $this->indices[$tbl][$field]['_invalid'] = $sqlIndexData[$field]; $this->indices[$tbl][$field]['_file'] = $selection; From 3e2cdd5c9e9b37462d24cdc820180d510d14f95c Mon Sep 17 00:00:00 2001 From: Nick Liu Date: Tue, 23 Jan 2018 10:19:37 -0600 Subject: [PATCH 3/3] Refactored away duplicate code in db_verify Hopefully Code Climate likes this one better --- e107_handlers/db_verify_class.php | 102 ++++++++++-------------------- 1 file changed, 34 insertions(+), 68 deletions(-) diff --git a/e107_handlers/db_verify_class.php b/e107_handlers/db_verify_class.php index 3b4399600..ae49653ef 100644 --- a/e107_handlers/db_verify_class.php +++ b/e107_handlers/db_verify_class.php @@ -136,7 +136,7 @@ class db_verify /** * Permissive field validation */ - private function diffFieldPermissive($expected, $actual) + private function diffStructurePermissive($expected, $actual) { // Permit actual text types that default to null even when // expected does not explicitly default to null @@ -316,11 +316,11 @@ class db_verify // echo "

PARSED

"; // print_a($sqlDataArr); - $fileFieldData = $this->getFields($this->sqlFileTables[$selection]['data'][$key]); - $sqlFieldData = $this->getFields($sqlDataArr['data'][0]); + $fileData['field'] = $this->getFields($this->sqlFileTables[$selection]['data'][$key]); + $sqlData['field'] = $this->getFields($sqlDataArr['data'][0]); - $fileIndexData = $this->getIndex($this->sqlFileTables[$selection]['data'][$key]); - $sqlIndexData = $this->getIndex($sqlDataArr['data'][0]); + $fileData['index'] = $this->getIndex($this->sqlFileTables[$selection]['data'][$key]); + $sqlData['index'] = $this->getIndex($sqlDataArr['data'][0]); /* $debugA = print_r($fileFieldData,TRUE); // Extracted Field Arrays $debugA .= "

Index

"; @@ -353,74 +353,40 @@ class db_verify $tbl = "lan_".$language."_".$tbl; } - - // Check Field Data. - foreach($fileFieldData as $field => $info ) + // Check field and index data + foreach(['field', 'index'] as $type) { - - - $this->results[$tbl][$field]['_status'] = 'ok'; - - if(!is_array($sqlFieldData[$field])) + foreach($fileData[$type] as $key => $value) { - // echo "

".$field."

- // 
".print_r($info,TRUE)."
 - ".print_r($sqlFieldData[$field],TRUE)."
"; - - $this->errors[$tbl]['_status'] = 'error'; // table status - $this->results[$tbl][$field]['_status'] = 'missing_field'; // field status - $this->results[$tbl][$field]['_valid'] = $info; - $this->results[$tbl][$field]['_file'] = $selection; + $this->results[$tbl][$key]['_status'] = 'ok'; + + //print("EXPECTED"); + //print_a($value); + //print("ACTUAL"); + //print_a($sqlData[$type][$key]); + + if(!is_array($sqlData[$type][$key])) + { + $this->errors[$tbl]['_status'] = 'error'; // table status + $this->results[$tbl][$key]['_status'] = "missing_$type"; // type status + $this->results[$tbl][$key]['_valid'] = $value; + $this->results[$tbl][$key]['_file'] = $selection; + } + elseif(count($diff = $this->diffStructurePermissive($value, $sqlData[$type][$key]))) + { + $this->errors[$tbl]['_status'] = "mismatch_$type"; + $this->results[$tbl][$key]['_status'] = 'mismatch'; + $this->results[$tbl][$key]['_diff'] = $diff; + $this->results[$tbl][$key]['_valid'] = $value; + $this->results[$tbl][$key]['_invalid'] = $sqlData[$type][$key]; + $this->results[$tbl][$key]['_file'] = $selection; + } + + // TODO Check for additional fields in SQL that should be removed. + // TODO Add support for MYSQL 5 table layout .eg. journal_id INT( 10 ) UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY , } - elseif(count($diff = $this->diffFieldPermissive($info, $sqlFieldData[$field]))) - { - $this->errors[$tbl]['_status'] = 'mismatch'; - $this->results[$tbl][$field]['_status'] = 'mismatch'; - $this->results[$tbl][$field]['_diff'] = $diff; - $this->results[$tbl][$field]['_valid'] = $info; - $this->results[$tbl][$field]['_invalid'] = $sqlFieldData[$field]; - $this->results[$tbl][$field]['_file'] = $selection; - } - - } - // print_a($fileIndexData); - // print_a($sqlIndexData); - // Check Index data - foreach($fileIndexData as $field => $info ) - { - if(!is_array($sqlIndexData[$field])) // missing index. - { - // print_a($info); - // print_a($sqlIndexData[$field]); - - $this->errors[$tbl]['_status'] = 'error'; // table status - $this->indices[$tbl][$field]['_status'] = 'missing_index'; // index status - $this->indices[$tbl][$field]['_valid'] = $info; - $this->indices[$tbl][$field]['_file'] = $selection; - } - elseif(count($diff = $this->diffFieldPermissive($info, $sqlIndexData[$field]))) // mismatched data - { - // print_a($info); - // print_a($sqlIndexData[$field]); - - $this->errors[$tbl]['_status'] = 'mismatch_index'; - $this->indices[$tbl][$field]['_status'] = 'mismatch'; - $this->indices[$tbl][$field]['_diff'] = $diff; - $this->indices[$tbl][$field]['_valid'] = $info; - $this->indices[$tbl][$field]['_invalid'] = $sqlIndexData[$field]; - $this->indices[$tbl][$field]['_file'] = $selection; - - } - - // TODO Check for additional fields in SQL that should be removed. - // TODO Add support for MYSQL 5 table layout .eg. journal_id INT( 10 ) UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY , - - } - - - - unset($data); }