refactor: refactor message inspectors into own classes

lib/postal.rb

@@ -15,6 +15,8 @@ module Postal
     autoload :Job
     autoload :MessageDB
     autoload :MessageInspection
+    autoload :MessageInspector
+    autoload :MessageInspectors
     autoload :MessageParser
     autoload :MessageRequeuer
     autoload :MXLookup
@@ -37,6 +39,7 @@ def self.eager_load!
     super
     Postal::MessageDB.eager_load!
     Postal::SMTPServer.eager_load!
+    Postal::MessageInspectors.eager_load!
   end
 
 end

lib/postal/message_db/message.rb

@@ -498,14 +498,16 @@ def rcpt_to_return_path?
       # Inspect this message
       #
       def inspect_message
-        if result = MessageInspection.new(self.raw_message, self.scope&.to_sym)
-          # Update the messages table with the results of our inspection
-          update(:inspected => 1, :spam_score => result.filtered_spam_score,  :threat => result.threat?, :threat_details => result.threat_message)
-          # Add any spam details into the spam checks database
-          self.database.insert_multi(:spam_checks, [:message_id, :code, :score, :description], result.filtered_spam_checks.map { |d| [self.id, d.code, d.score, d.description]})
-          # Return the result
-          result
-        end
+        result = MessageInspection.scan(self, self.scope&.to_sym)
+
+        # Update the messages table with the results of our inspection
+        update(:inspected => 1, :spam_score => result.spam_score,  :threat => result.threat?, :threat_details => result.threat_message)
+
+        # Add any spam details into the spam checks database
+        self.database.insert_multi(:spam_checks, [:message_id, :code, :score, :description], result.spam_checks.map { |d| [self.id, d.code, d.score, d.description] })
+
+        # Return the result
+        result
       end
 
       #

lib/postal/message_inspection.rb

@@ -1,140 +1,41 @@
-require 'timeout'
-require 'socket'
-require 'json'
-
 module Postal
   class MessageInspection
 
-    SPAM_EXCLUSIONS = {
-      :outgoing => ['NO_RECEIVED', 'NO_RELAYS', 'ALL_TRUSTED', 'FREEMAIL_FORGED_REPLYTO', 'RDNS_DYNAMIC', 'CK_HELO_GENERIC', /^SPF\_/, /^HELO\_/, /DKIM_/, /^RCVD_IN_/],
-      :incoming => []
-    }
+    attr_reader :message
+    attr_reader :scope
+    attr_reader :spam_checks
+    attr_accessor :threat
+    attr_accessor :threat_message
 
-    def initialize(message, scope = :incoming)
+    def initialize(message, scope)
       @message = message
       @scope = scope
-      @threat = false
-      @spam_score = 0.0
       @spam_checks = []
-
-      if Postal.config.spamd.enabled?
-        scan_for_spam
-      end
-
-      if Postal.config.clamav.enabled?
-        scan_for_threats
-      end
+      @threat = false
     end
 
     def spam_score
-      @spam_score
-    end
-
-    def spam_checks
-      @spam_checks
-    end
+      return 0 if @spam_checks.empty?
 
-    def filtered_spam_checks
-      @filtered_spam_checks ||= @spam_checks.reject do |check|
-        SPAM_EXCLUSIONS[@scope].any? do |item|
-          item == check.code || (item.is_a?(Regexp) && item =~ check.code)
-        end
-      end
-    end
-
-    def filtered_spam_score
-      filtered_spam_checks.inject(0.0) do |total, check|
-        total += check.score || 0.0
-      end.round(2)
+      @spam_checks.sum(&:score)
     end
 
     def threat?
-      @threat
-    end
-
-    def threat_message
-      @threat_message
+      @threat == true
     end
 
-    private
-
-    def scan_for_spam
-      data = nil
-      Timeout.timeout(15) do
-        tcp_socket = TCPSocket.new(Postal.config.spamd.host, Postal.config.spamd.port)
-        tcp_socket.write("REPORT SPAMC/1.2\r\n")
-        tcp_socket.write("Content-length: #{@message.bytesize}\r\n")
-        tcp_socket.write("\r\n")
-        tcp_socket.write(@message)
-        tcp_socket.close_write
-        data = tcp_socket.read
-      end
-
-      spam_checks = []
-      total = 0.0
-      rules = data ? data.split(/^---(.*)\r?\n/).last.split(/\r?\n/) : []
-      while line = rules.shift
-        if line =~ /\A([\- ]?[\d\.]+)\s+(\w+)\s+(.*)/
-          total += $1.to_f
-          spam_checks << SPAMCheck.new($2, $1.to_f, $3)
-        else
-          spam_checks.last.description << " " + line.strip
-        end
+    def scan
+      MessageInspector.inspectors.each do |inspector|
+        inspector.inspect_message(self)
       end
-
-      @spam_score = total.round(1)
-      @spam_checks = spam_checks
-
-    rescue Timeout::Error
-      @spam_checks = [SPAMCheck.new("TIMEOUT", 0, "Timed out when scanning for spam")]
-    rescue => e
-      logger.error "Error talking to spamd: #{e.class} (#{e.message})"
-      logger.error e.backtrace[0,5]
-      @spam_checks = [SPAMCheck.new("ERROR", 0, "Error when scanning for spam")]
-    ensure
-      tcp_socket.close rescue nil
     end
 
-    def scan_for_threats
-      @threat = false
-
-      data = nil
-      Timeout.timeout(10) do
-        tcp_socket = TCPSocket.new(Postal.config.clamav.host, Postal.config.clamav.port)
-        tcp_socket.write("zINSTREAM\0")
-        tcp_socket.write([@message.bytesize].pack("N"))
-        tcp_socket.write(@message)
-        tcp_socket.write([0].pack("N"))
-        tcp_socket.close_write
-        data = tcp_socket.read
-      end
-
-      if data && data =~ /\Astream\:\s+(.*?)[\s\0]+?/
-        if $1.upcase == 'OK'
-          @threat = false
-          @threat_message = "No threats found"
-        else
-          @threat = true
-          @threat_message = $1
-        end
-      else
-        @threat = false
-        @threat_message = "Could not scan message"
+    class << self
+      def scan(message, scope)
+        inspection = new(message, scope)
+        inspection.scan
+        inspection
       end
-    rescue Timeout::Error
-      @threat = false
-      @threat_message = "Timed out scanning for threats"
-    rescue => e
-      logger.error "Error talking to clamav: #{e.class} (#{e.message})"
-      logger.error e.backtrace[0,5]
-      @threat = false
-      @threat_message = "Error when scanning for threats"
-    ensure
-      tcp_socket.close rescue nil
-    end
-
-    def logger
-      Postal.logger_for(:message_inspection)
     end
 
   end

lib/postal/message_inspector.rb

@@ -0,0 +1,38 @@
+module Postal
+  class MessageInspector
+
+    def initialize(config)
+      @config = config
+    end
+
+    # Inspect a message and update the inspection with the results
+    # as appropriate.
+    def inspect_message(message, scope, inspection)
+    end
+
+    private
+
+    def logger
+      Postal.logger_for(:message_inspection)
+    end
+
+    class << self
+      # Return an array of all inspectors that are available for this
+      # installation.
+      def inspectors
+        Array.new.tap do |inspectors|
+
+          if Postal.config.spamd&.enabled
+            inspectors << MessageInspectors::SpamAssassin.new(Postal.config.spamd)
+          end
+
+          if Postal.config.clamav&.enabled
+            inspectors << MessageInspectors::Clamav.new(Postal.config.clamav)
+          end
+
+        end
+      end
+    end
+
+  end
+end

lib/postal/message_inspectors.rb

@@ -0,0 +1,9 @@
+module Postal
+  module MessageInspectors
+    extend ActiveSupport::Autoload
+    eager_autoload do
+      autoload :Clamav
+      autoload :SpamAssassin
+    end
+  end
+end

lib/postal/message_inspectors/clamav.rb

@@ -0,0 +1,45 @@
+module Postal
+  module MessageInspectors
+    class Clamav < MessageInspector
+
+      def inspect_message(inspection)
+        raw_message = inspection.message.raw_message
+
+        data = nil
+        Timeout.timeout(10) do
+          tcp_socket = TCPSocket.new(@config.host, @config.port)
+          tcp_socket.write("zINSTREAM\0")
+          tcp_socket.write([raw_message.bytesize].pack("N"))
+          tcp_socket.write(raw_message)
+          tcp_socket.write([0].pack("N"))
+          tcp_socket.close_write
+          data = tcp_socket.read
+        end
+
+        if data && data =~ /\Astream\:\s+(.*?)[\s\0]+?/
+          if $1.upcase == 'OK'
+            inspection.threat = false
+            inspection.threat_message = "No threats found"
+          else
+            inspection.threat = true
+            inspection.threat_message = $1
+          end
+        else
+          inspection.threat = false
+          inspection.threat_message = "Could not scan message"
+        end
+      rescue Timeout::Error
+        inspection.threat = false
+        inspection.threat_message = "Timed out scanning for threats"
+      rescue => e
+        logger.error "Error talking to clamav: #{e.class} (#{e.message})"
+        logger.error e.backtrace[0,5]
+        inspection.threat = false
+        inspection.threat_message = "Error when scanning for threats"
+      ensure
+        tcp_socket.close rescue nil
+      end
+
+    end
+  end
+end

lib/postal/message_inspectors/spam_assassin.rb

@@ -0,0 +1,54 @@
+module Postal
+  module MessageInspectors
+    class SpamAssassin < MessageInspector
+
+      EXCLUSIONS = {
+        :outgoing => ['NO_RECEIVED', 'NO_RELAYS', 'ALL_TRUSTED', 'FREEMAIL_FORGED_REPLYTO', 'RDNS_DYNAMIC', 'CK_HELO_GENERIC', /^SPF\_/, /^HELO\_/, /DKIM_/, /^RCVD_IN_/],
+        :incoming => []
+      }
+
+      def inspect_message(inspection)
+        data = nil
+        raw_message = inspection.message.raw_message
+        Timeout.timeout(15) do
+          tcp_socket = TCPSocket.new(@config.host, @config.port)
+          tcp_socket.write("REPORT SPAMC/1.2\r\n")
+          tcp_socket.write("Content-length: #{raw_message.bytesize}\r\n")
+          tcp_socket.write("\r\n")
+          tcp_socket.write(raw_message)
+          tcp_socket.close_write
+          data = tcp_socket.read
+        end
+
+        spam_checks = []
+        total = 0.0
+        rules = data ? data.split(/^---(.*)\r?\n/).last.split(/\r?\n/) : []
+        while line = rules.shift
+          if line =~ /\A([\- ]?[\d\.]+)\s+(\w+)\s+(.*)/
+            total += $1.to_f
+            spam_checks << SpamCheck.new($2, $1.to_f, $3)
+          else
+            spam_checks.last.description << " " + line.strip
+          end
+        end
+
+        checks = spam_checks.reject { |s| EXCLUSIONS[inspection.scope].include?(s.code) }
+        checks.each do |check|
+          inspection.spam_checks << check
+        end
+
+      rescue Timeout::Error
+        inspection.spam_checks << SpamCheck.new("TIMEOUT", 0, "Timed out when scanning for spam")
+
+      rescue => e
+        logger.error "Error talking to spamd: #{e.class} (#{e.message})"
+        logger.error e.backtrace[0,5]
+        inspection.spam_checks << SpamCheck.new("ERROR", 0, "Error when scanning for spam")
+
+      ensure
+        tcp_socket.close rescue nil
+      end
+
+    end
+  end
+end

lib/postal/spam_check.rb

@@ -1,5 +1,5 @@
 module Postal
-  class SPAMCheck
+  class SpamCheck
 
     attr_reader :code, :score, :description