MDL-50887 antivirus_clamav: Split scanning logic and results processing.

This refactoring will make possible to assert scan results processing behaviour in unit testing.
ralferlebach · Feb 29, 2016 · 5b6a0f2 · 5b6a0f2
1 parent 56e915b
commit 5b6a0f2
Showing 1 changed file with 39 additions and 26 deletions.
diff --git a/lib/antivirus/clamav/classes/scanner.php b/lib/antivirus/clamav/classes/scanner.php
@@ -43,9 +43,6 @@ public function is_configured() {
     /**
      * Scan file, throws exception in case of infected file.
      *
-     * Please note that the scanning engine must be able to access the file,
-     * permissions of the file are not modified here!
-     *
      * @param string $file Full path to the file.
      * @param string $filename Name of the file (could be different from physical file if temp file is used).
      * @param bool $deleteinfected whether infected file needs to be deleted.
@@ -57,30 +54,12 @@ public function scan_file($file, $filename, $deleteinfected) {
 
         if (!is_readable($file)) {
             // This should not happen.
+            debugging('File is not readable.');
             return;
         }
 
-        $this->config->pathtoclam = trim($this->config->pathtoclam);
-
-        if (!file_exists($this->config->pathtoclam) or !is_executable($this->config->pathtoclam)) {
-            // Misconfigured clam, notify admins.
-            $notice = get_string('invalidpathtoclam', 'antivirus_clamav', $this->config->pathtoclam);
-            $this->message_admins($notice);
-            return;
-        }
-
-        $clamparam = ' --stdout ';
-        // If we are dealing with clamdscan, clamd is likely run as a different user
-        // that might not have permissions to access your file.
-        // To make clamdscan work, we use --fdpass parameter that passes the file
-        // descriptor permissions to clamd, which allows it to scan given file
-        // irrespective of directory and file permissions.
-        if (basename($this->config->pathtoclam) == 'clamdscan') {
-            $clamparam .= '--fdpass ';
-        }
-        // Execute scan.
-        $cmd = escapeshellcmd($this->config->pathtoclam).$clamparam.escapeshellarg($file);
-        exec($cmd, $output, $return);
+        // Execute the scan using preferable method.
+        list($return, $notice) = $this->scan_file_execute_commandline($file);
 
         if ($return == 0) {
             // Perfect, no problem found, file is clean.
@@ -93,8 +72,6 @@ public function scan_file($file, $filename, $deleteinfected) {
             throw new \core\antivirus\scanner_exception('virusfounduser', '', array('filename' => $filename));
         } else {
             // Unknown problem.
-            $notice = get_string('clamfailed', 'antivirus_clamav', $this->get_clam_error_code($return));
-            $notice .= "\n\n". implode("\n", $output);
             $this->message_admins($notice);
             if ($this->config->clamfailureonupload === 'actlikevirus') {
                 if ($deleteinfected) {
@@ -140,4 +117,40 @@ private function get_clam_error_code($returncode) {
         }
         return get_string('unknownerror', 'antivirus_clamav');
     }
+
+    /**
+     * Scan file using command line utility.
+     *
+     * @param string $file Full path to the file.
+     * @return array ($return, $notice) Execution return code and notification text.
+     */
+    public function scan_file_execute_commandline($file) {
+        $this->config->pathtoclam = trim($this->config->pathtoclam);
+
+        if (!file_exists($this->config->pathtoclam) or !is_executable($this->config->pathtoclam)) {
+            // Misconfigured clam, notify admins.
+            $notice = get_string('invalidpathtoclam', 'antivirus_clamav', $this->config->pathtoclam);
+            return array(-1, $notice);
+        }
+
+        $clamparam = ' --stdout ';
+        // If we are dealing with clamdscan, clamd is likely run as a different user
+        // that might not have permissions to access your file.
+        // To make clamdscan work, we use --fdpass parameter that passes the file
+        // descriptor permissions to clamd, which allows it to scan given file
+        // irrespective of directory and file permissions.
+        if (basename($this->config->pathtoclam) == 'clamdscan') {
+            $clamparam .= '--fdpass ';
+        }
+        // Execute scan.
+        $cmd = escapeshellcmd($this->config->pathtoclam).$clamparam.escapeshellarg($file);
+        exec($cmd, $output, $return);
+        $notice = '';
+        if ($return > 1) {
+            $notice = get_string('clamfailed', 'antivirus_clamav', $this->get_clam_error_code($return));
+            $notice .= "\n\n". implode("\n", $output);
+        }
+
+        return array($return, $notice);
+    }
 }