From fb16f918d321a01abf4e33bdeb277c221155a7fa Mon Sep 17 00:00:00 2001 From: Denis Ryabov Date: Tue, 23 Feb 2021 23:23:45 +0300 Subject: [PATCH] clean PHP code (by removing comments, html, and strings) in the framework rules to avoid false-positives --- .../libraries/rules/framework.php | 136 ++++++++++++++++-- 1 file changed, 125 insertions(+), 11 deletions(-) diff --git a/administrator/components/com_jedchecker/libraries/rules/framework.php b/administrator/components/com_jedchecker/libraries/rules/framework.php index 12f7ad0..36afdb1 100644 --- a/administrator/components/com_jedchecker/libraries/rules/framework.php +++ b/administrator/components/com_jedchecker/libraries/rules/framework.php @@ -104,12 +104,14 @@ class JedcheckerRulesFramework extends JEDcheckerRule */ protected function find($file) { - $content = (array) file($file); + $origContent = (array) file($file); + $cleanContent = preg_split("/(?:\r\n|\n|\r)(?!$)/", $this->cleanNonCode($file)); + $result = false; foreach ($this->getTests() as $testObject) { - if ($this->runTest($file, $content, $testObject)) + if ($this->runTest($file, $origContent, $cleanContent, $testObject)) { $result = true; } @@ -118,35 +120,146 @@ class JedcheckerRulesFramework extends JEDcheckerRule return $result; } + /** + * @param string $file + * + * @return string + */ + protected function cleanNonCode($file) + { + $content = file_get_contents($file); + + if (!preg_match('/<\?php\s/i', $content, $match, PREG_OFFSET_CAPTURE)) + { + // No PHP code found + return ''; + } + + $pos = $match[0][1]; + $cleanContent = $this->removeContent(substr($content, 0, $pos)); + + while (preg_match('/(?:[\'"]|\/\*|\/\/|\?>)/', $content, $match, PREG_OFFSET_CAPTURE, $pos)) + { + $foundPos = $match[0][1]; + $cleanContent .= substr($content, $pos, $foundPos - $pos); + $pos = $foundPos; + + switch ($match[0][0]) + { + case '"': + case "'": + $q = $match[0][0]; + + if (!preg_match("/$q(?>[^$q\\\\]+|\\\\.)*$q/As", $content, $match, 0, $pos)) + { + return $cleanContent . $q; + } + + $cleanContent .= $q . $this->removeContent($match[0]) . $q; + $pos += strlen($match[0]); + break; + + case '/*': + $cleanContent .= '/*'; + $pos += 2; + + $endPos = strpos($content, '*/', $pos); + + if ($endPos === false) + { + return $cleanContent; + } + + $cleanContent .= $this->removeContent(substr($content, $pos, $endPos - $pos)) . '*/'; + $pos = $endPos + 2; + + break; + + case '//': + $pos += strcspn($content, "\r\n", $pos); + break; + + case '?>': + $cleanContent .= '?>'; + $pos += 2; + + if (!preg_match('/<\?php\s/i', $content, $match, PREG_OFFSET_CAPTURE, $pos)) + { + // No PHP code found (up to the end of the file) + return $cleanContent; + } + + $foundPos = $match[0][1]; + $cleanContent .= $this->removeContent(substr($content, $pos, $foundPos - $pos)) . $match[0][0]; + $pos = $foundPos + strlen($match[0][0]); + + break; + } + } + + return $cleanContent; + } + + /** + * Remove all text content by keeping newline characters only + * + * @param string $content + * + * @return string + */ + protected function removeContent($content) + { + return str_repeat("\n", substr_count($content, "\n")); + } + /** * runs tests and reports to the appropriate function if strings match. * * @param string $file The file name - * @param array $content The file content + * @param array $origContent The file content + * @param array $cleanContent The file content w/o non-code elements * @param object $testObject The test object generated by getTests() * * @return boolean */ - private function runTest($file, $content, $testObject) + private function runTest($file, $origContent, $cleanContent, $testObject) { + // @todo remove as unused? $error_count = 0; - foreach ($content as $line_number => $line) + foreach ($cleanContent as $line_number => $line) { + $origLine = $origContent[$line_number]; + foreach ($testObject->tests AS $singleTest) { - if (stripos($line, $singleTest) !== false) + $regex = preg_quote($singleTest, '/'); + + if (ctype_alpha($singleTest[0])) { - $line = str_ireplace($singleTest, '' . $singleTest . '', $line); - $error_message = JText::_('COM_JEDCHECKER_ERROR_FRAMEWORK_' . strtoupper($testObject->group)) . ':
' . $line . '
'; + $regex = '(?<=\W|^)' . $regex; + } + + if (ctype_alpha($singleTest[strlen($singleTest) - 1])) + { + $regex .= '(?=\W|$)'; + } + + if (preg_match('/' . $regex . '/i', $line)) + { + $origLine = str_ireplace($singleTest, '' . $singleTest . '', htmlspecialchars($origLine)); + $error_message = JText::_('COM_JEDCHECKER_ERROR_FRAMEWORK_' . strtoupper($testObject->group)) . ':
' . $origLine . '
'; switch ($testObject->kind) { - case 'error':$this->report->addError($file, $error_message, $line_number); + case 'error': + $this->report->addError($file, $error_message, $line_number); break; - case 'warning':$this->report->addWarning($file, $error_message, $line_number); + case 'warning': + $this->report->addWarning($file, $error_message, $line_number); break; - case 'compatibility':$this->report->addCompat($file, $error_message, $line_number); + case 'compatibility': + $this->report->addCompat($file, $error_message, $line_number); break; default: // Case 'notice': @@ -154,6 +267,7 @@ class JedcheckerRulesFramework extends JEDcheckerRule break; } } + // If you scored 10 errors on a single file, that's enough for now. if ($error_count > 10) {