From 1e3622c49abeeb0ca8a7cc1224e84f2fc74cc938 Mon Sep 17 00:00:00 2001
From: Charlie Smurthwaite <charlie.smurthwaite@krystal.uk>
Date: Wed, 22 Mar 2023 13:49:48 +0000
Subject: [PATCH] Consistently treat tinyint(1) fields in message database as
 booleans (#2380)

* Update mysql2 query call to cast booleans

* Treat messages:held field as boolean

* Treat messages:inspected field as boolean

* Treat messages:spam field as boolean

* Treat messages:threat field as boolean

* Treat messages:bounce field as boolean

* Treat messages:received_with_ssl field as boolean

* Treat deliveries:sent_with_ssl field as boolean
---
 api/controllers/send_api_controller.rb   |  2 +-
 api/structures/delivery_api_structure.rb |  2 +-
 api/structures/message_api_structure.rb  |  8 ++++----
 app/controllers/messages_controller.rb   |  2 +-
 app/jobs/unqueue_message_job.rb          | 24 ++++++++++++------------
 app/models/outgoing_message_prototype.rb |  2 +-
 app/models/server.rb                     |  2 +-
 app/views/messages/_deliveries.html.haml |  2 +-
 app/views/messages/activity.html.haml    |  2 +-
 app/views/messages/show.html.haml        |  6 +++---
 lib/postal/bounce_message.rb             |  2 +-
 lib/postal/http_sender.rb                |  4 ++--
 lib/postal/message_db/database.rb        |  2 +-
 lib/postal/message_db/message.rb         | 12 ++++++------
 lib/postal/message_inspection.rb         |  4 ----
 lib/postal/smtp_sender.rb                |  2 +-
 script/insert-bounce.rb                  |  2 +-
 17 files changed, 38 insertions(+), 42 deletions(-)

diff --git a/api/controllers/send_api_controller.rb b/api/controllers/send_api_controller.rb
index 20da531..8de60b6 100644
--- a/api/controllers/send_api_controller.rb
+++ b/api/controllers/send_api_controller.rb
@@ -99,7 +99,7 @@ controller :send do
         message.scope = "outgoing"
         message.domain_id = authenticated_domain.id
         message.credential_id = identity.id
-        message.bounce = params.bounce ? 1 : 0
+        message.bounce = params.bounce
         message.save
         result[:message_id] = message.message_id if result[:message_id].nil?
         result[:messages][rcpt_to] = { id: message.id, token: message.token }
diff --git a/api/structures/delivery_api_structure.rb b/api/structures/delivery_api_structure.rb
index a92505b..d685f69 100644
--- a/api/structures/delivery_api_structure.rb
+++ b/api/structures/delivery_api_structure.rb
@@ -3,7 +3,7 @@ structure :delivery do
   basic :status
   basic :details
   basic :output, value: proc { o.output&.strip }
-  basic :sent_with_ssl, value: proc { o.sent_with_ssl == 1 }
+  basic :sent_with_ssl, value: proc { o.sent_with_ssl }
   basic :log_id
   basic :time, value: proc { o.time&.to_f }
   basic :timestamp, value: proc { o.timestamp.to_f }
diff --git a/api/structures/message_api_structure.rb b/api/structures/message_api_structure.rb
index 3794188..5e97e5a 100644
--- a/api/structures/message_api_structure.rb
+++ b/api/structures/message_api_structure.rb
@@ -6,7 +6,7 @@ structure :message do
     {
       status: o.status,
       last_delivery_attempt: o.last_delivery_attempt ? o.last_delivery_attempt.to_f : nil,
-      held: o.held == 1,
+      held: o.held,
       hold_expiry: o.hold_expiry ? o.hold_expiry.to_f : nil
     }
   end
@@ -29,10 +29,10 @@ structure :message do
 
   expansion(:inspection) do
     {
-      inspected: o.inspected == 1,
-      spam: o.spam == 1,
+      inspected: o.inspected,
+      spam: o.spam,
       spam_score: o.spam_score.to_f,
-      threat: o.threat == 1,
+      threat: o.threat,
       threat_details: o.threat_details
     }
   end
diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb
index aee5491..1d267a9 100644
--- a/app/controllers/messages_controller.rb
+++ b/app/controllers/messages_controller.rb
@@ -153,7 +153,7 @@ class MessagesController < ApplicationController
 
   def get_messages(scope)
     if scope == "held"
-      options = { where: { held: 1 } }
+      options = { where: { held: true } }
     else
       options = { where: { scope: scope, spam: false }, order: :timestamp, direction: "desc" }
 
diff --git a/app/jobs/unqueue_message_job.rb b/app/jobs/unqueue_message_job.rb
index a44e2b9..dadfc43 100644
--- a/app/jobs/unqueue_message_job.rb
+++ b/app/jobs/unqueue_message_job.rb
@@ -87,7 +87,7 @@ class UnqueueMessageJob < Postal::Job
               #
               # If this is a bounce, we need to handle it as such
               #
-              if queued_message.message.bounce == 1
+              if queued_message.message.bounce
                 log "#{log_prefix} Message is a bounce"
                 original_messages = queued_message.message.original_messages
                 unless original_messages.empty?
@@ -120,17 +120,17 @@ class UnqueueMessageJob < Postal::Job
               #
               # Inspect incoming messages
               #
-              if queued_message.message.inspected == 0
+              unless queued_message.message.inspected
                 log "#{log_prefix} Inspecting message"
                 queued_message.message.inspect_message
-                if queued_message.message.inspected == 1
+                if queued_message.message.inspected
                   is_spam = queued_message.message.spam_score > queued_message.server.spam_threshold
-                  queued_message.message.update(spam: 1) if is_spam
+                  queued_message.message.update(spam: true) if is_spam
                   queued_message.message.append_headers(
-                    "X-Postal-Spam: #{queued_message.message.spam == 1 ? 'yes' : 'no'}",
+                    "X-Postal-Spam: #{queued_message.message.spam ? 'yes' : 'no'}",
                     "X-Postal-Spam-Threshold: #{queued_message.server.spam_threshold}",
                     "X-Postal-Spam-Score: #{queued_message.message.spam_score}",
-                    "X-Postal-Threat: #{queued_message.message.threat == 1 ? 'yes' : 'no'}"
+                    "X-Postal-Threat: #{queued_message.message.threat ? 'yes' : 'no'}"
                   )
                   log "#{log_prefix} Message inspected successfully. Headers added."
                 end
@@ -162,7 +162,7 @@ class UnqueueMessageJob < Postal::Job
               if route = queued_message.message.route
 
                 # If the route says we're holding quananteed mail and this is spam, we'll hold this
-                if route.spam_mode == "Quarantine" && queued_message.message.spam == 1 && !queued_message.manual?
+                if route.spam_mode == "Quarantine" && queued_message.message.spam && !queued_message.manual?
                   queued_message.message.create_delivery("Held", details: "Message placed into quarantine.")
                   queued_message.destroy
                   log "#{log_prefix} Route says to quarantine spam message. Holding."
@@ -170,7 +170,7 @@ class UnqueueMessageJob < Postal::Job
                 end
 
                 # If the route says we're holding quananteed mail and this is spam, we'll hold this
-                if route.spam_mode == "Fail" && queued_message.message.spam == 1 && !queued_message.manual?
+                if route.spam_mode == "Fail" && queued_message.message.spam && !queued_message.manual?
                   queued_message.message.create_delivery("HardFail", details: "Message is spam and the route specifies it should be failed.")
                   queued_message.destroy
                   log "#{log_prefix} Route says to fail spam message. Hard failing."
@@ -325,18 +325,18 @@ class UnqueueMessageJob < Postal::Job
               end
 
               # Inspect outgoing messages when there's a threshold set for the server
-              if queued_message.message.inspected == 0 && queued_message.server.outbound_spam_threshold
+              if !queued_message.message.inspected && queued_message.server.outbound_spam_threshold
                 log "#{log_prefix} Inspecting message"
                 queued_message.message.inspect_message
-                if queued_message.message.inspected == 1
+                if queued_message.message.inspected
                   if queued_message.message.spam_score >= queued_message.server.outbound_spam_threshold
-                    queued_message.message.update(spam: 1)
+                    queued_message.message.update(spam: true)
                   end
                   log "#{log_prefix} Message inspected successfully"
                 end
               end
 
-              if queued_message.message.spam == 1
+              if queued_message.message.spam
                 queued_message.message.create_delivery("HardFail", details: "Message is likely spam. Threshold is #{queued_message.server.outbound_spam_threshold} and the message scored #{queued_message.message.spam_score}.")
                 queued_message.destroy
                 log "#{log_prefix} Message is spam (#{queued_message.message.spam_score}). Hard failing."
diff --git a/app/models/outgoing_message_prototype.rb b/app/models/outgoing_message_prototype.rb
index b904fff..1cf1a1f 100644
--- a/app/models/outgoing_message_prototype.rb
+++ b/app/models/outgoing_message_prototype.rb
@@ -191,7 +191,7 @@ class OutgoingMessagePrototype
     message.tag = tag
     message.credential_id = credential&.id
     message.received_with_ssl = true
-    message.bounce = @bounce ? 1 : 0
+    message.bounce = @bounce
     message.save
     { id: message.id, token: message.token }
   end
diff --git a/app/models/server.rb b/app/models/server.rb
index 614922f..88e9a68 100644
--- a/app/models/server.rb
+++ b/app/models/server.rb
@@ -134,7 +134,7 @@ class Server < ApplicationRecord
   end
 
   def held_messages
-    @held_messages ||= message_db.messages(where: { held: 1 }, count: true)
+    @held_messages ||= message_db.messages(where: { held: true }, count: true)
   end
 
   def throughput_stats
diff --git a/app/views/messages/_deliveries.html.haml b/app/views/messages/_deliveries.html.haml
index cac94f3..707948b 100644
--- a/app/views/messages/_deliveries.html.haml
+++ b/app/views/messages/_deliveries.html.haml
@@ -35,7 +35,7 @@
           .deliveryList__time
             = delivery.timestamp.to_s(:long)
           .deliveryList__status
-            - if delivery.sent_with_ssl == 1
+            - if delivery.sent_with_ssl
               = image_tag 'icons/lock.svg', :class => 'deliveryList__secure'
             %span.label.label--large{:class => "label--messageStatus-#{delivery.status.underscore}"}= delivery.status.underscore.humanize
         - if delivery.details
diff --git a/app/views/messages/activity.html.haml b/app/views/messages/activity.html.haml
index 64910a0..fba7341 100644
--- a/app/views/messages/activity.html.haml
+++ b/app/views/messages/activity.html.haml
@@ -41,5 +41,5 @@
         %p.messageActivity__extra
           - if @message.credential
             Received using the #{@message.credential.name} #{@message.credential.type} credential.
-          - if @message.received_with_ssl == 1
+          - if @message.received_with_ssl
             Connection secured with SSL.
diff --git a/app/views/messages/show.html.haml b/app/views/messages/show.html.haml
index dc5ffd8..36f7da0 100644
--- a/app/views/messages/show.html.haml
+++ b/app/views/messages/show.html.haml
@@ -56,7 +56,7 @@
                 = link_to @message.domain.name, [organization, @server, :domains], :class => "u-link"
               - else
                 Unknown Domain
-      - if @message.threat == 1
+      - if @message.threat
         %dl.messagePropertiesPage__property
           %dt Threat
           %dd= @message.threat_details
@@ -66,8 +66,8 @@
       - unless @message.received_with_ssl.nil?
         %dl.messagePropertiesPage__property
           %dt Transport Security
-          - if @message.received_with_ssl == 1
-            %dd.messagePropertiesPage__property--locked Received over a SSL connection
+          - if @message.received_with_ssl
+            %dd.messagePropertiesPage__property--locked Received over an SSL connection
           - else
             %dd Not received with SSL
 
diff --git a/lib/postal/bounce_message.rb b/lib/postal/bounce_message.rb
index 3834c61..4c12a29 100644
--- a/lib/postal/bounce_message.rb
+++ b/lib/postal/bounce_message.rb
@@ -24,7 +24,7 @@ module Postal
       message.mail_from = @message.route.description
       message.domain_id = @message.domain&.id
       message.raw_message = raw_message
-      message.bounce = 1
+      message.bounce = true
       message.bounce_for_id = @message.id
       message.save
       message.id
diff --git a/lib/postal/http_sender.rb b/lib/postal/http_sender.rb
index 6b66217..c6a23ee 100644
--- a/lib/postal/http_sender.rb
+++ b/lib/postal/http_sender.rb
@@ -74,8 +74,8 @@ module Postal
           timestamp: message.timestamp.to_f,
           size: message.size,
           spam_status: message.spam_status,
-          bounce: message.bounce == 1,
-          received_with_ssl: message.received_with_ssl == 1,
+          bounce: message.bounce,
+          received_with_ssl: message.received_with_ssl,
           to: message.headers["to"]&.last,
           cc: message.headers["cc"]&.last,
           from: message.headers["from"]&.last,
diff --git a/lib/postal/message_db/database.rb b/lib/postal/message_db/database.rb
index 1c8561e..d9734ed 100644
--- a/lib/postal/message_db/database.rb
+++ b/lib/postal/message_db/database.rb
@@ -316,7 +316,7 @@ module Postal
 
       def query_on_connection(connection, query)
         start_time = Time.now.to_f
-        result = connection.query(query)
+        result = connection.query(query, cast_booleans: true)
         time = Time.now.to_f - start_time
         logger.debug "  \e[4;34mMessageDB Query (#{time.round(2)}s) \e[0m  \e[33m#{query}\e[0m"
         if time > 0.5 && query =~ /\A(SELECT|UPDATE|DELETE) /
diff --git a/lib/postal/message_db/message.rb b/lib/postal/message_db/message.rb
index ea87b80..1127cbc 100644
--- a/lib/postal/message_db/message.rb
+++ b/lib/postal/message_db/message.rb
@@ -123,7 +123,7 @@ module Postal
       def create_delivery(status, options = {})
         delivery = Delivery.create(self, options.merge(status: status))
         hold_expiry = status == "Held" ? Postal.config.general.maximum_hold_expiry_days.days.from_now.to_f : nil
-        update(status: status, last_delivery_attempt: delivery.timestamp.to_f, held: status == "Held" ? 1 : 0, hold_expiry: hold_expiry)
+        update(status: status, last_delivery_attempt: delivery.timestamp.to_f, held: status == "Held", hold_expiry: hold_expiry)
         delivery
       end
 
@@ -371,9 +371,9 @@ module Postal
       # Return the spam status
       #
       def spam_status
-        return "NotChecked" unless inspected == 1
+        return "NotChecked" unless inspected
 
-        spam == 1 ? "Spam" : "NotSpam"
+        spam ? "Spam" : "NotSpam"
       end
 
       #
@@ -445,7 +445,7 @@ module Postal
       # Should bounces be sent for this message?
       #
       def send_bounces?
-        bounce != 1 && mail_from.present?
+        !bounce && mail_from.present?
       end
 
       #
@@ -471,7 +471,7 @@ module Postal
       #  Return a message object that this message is a reply to
       #
       def original_messages
-        return nil unless bounce == 1
+        return nil unless bounce
 
         other_message_ids = raw_message.scan(/\X-Postal-MsgID:\s*([a-z0-9]+)/i).flatten
         if other_message_ids.empty?
@@ -495,7 +495,7 @@ module Postal
         result = MessageInspection.scan(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)
+        update(inspected: true, spam_score: result.spam_score, threat: result.threat, threat_details: result.threat_message)
 
         # Add any spam details into the spam checks database
         database.insert_multi(:spam_checks, [:message_id, :code, :score, :description], result.spam_checks.map { |d| [id, d.code, d.score, d.description] })
diff --git a/lib/postal/message_inspection.rb b/lib/postal/message_inspection.rb
index 5f24723..402844e 100644
--- a/lib/postal/message_inspection.rb
+++ b/lib/postal/message_inspection.rb
@@ -20,10 +20,6 @@ module Postal
       @spam_checks.sum(&:score)
     end
 
-    def threat?
-      @threat == true
-    end
-
     def scan
       MessageInspector.inspectors.each do |inspector|
         inspector.inspect_message(self)
diff --git a/lib/postal/smtp_sender.rb b/lib/postal/smtp_sender.rb
index 1ae9457..c2fd4b1 100644
--- a/lib/postal/smtp_sender.rb
+++ b/lib/postal/smtp_sender.rb
@@ -133,7 +133,7 @@ module Postal
       end
 
       begin
-        if message.bounce == 1
+        if message.bounce
           mail_from = ""
         elsif message.domain.return_path_status == "OK"
           mail_from = "#{message.server.token}@#{message.domain.return_path_domain}"
diff --git a/script/insert-bounce.rb b/script/insert-bounce.rb
index 87cca6b..81d4c2e 100755
--- a/script/insert-bounce.rb
+++ b/script/insert-bounce.rb
@@ -30,6 +30,6 @@ message.scope = "incoming"
 message.rcpt_to = "#{server.token}@#{Postal.config.dns.return_path}"
 message.mail_from = "MAILER-DAEMON@smtp.infra.atech.io"
 message.raw_message = template
-message.bounce = 1
+message.bounce = true
 message.save
 puts "Added message with id #{message.id}"