Maintenance: Improved storage of unprocessable / oversized emails.

.gitignore

@@ -52,10 +52,13 @@
 /public/assets/custom
 /public/assets/chat/node_modules
 /tmp/*
+!/tmp/README.md
 !/tmp/pids
 /tmp/pids/*
 !/tmp/pids/.keep
 /storage/fs
+/var/*
+!/var/README.md
 
 # doorkeeper (OAuth 2)
 /public/assets/doorkeeper

app/models/channel/email_parser.rb

@@ -12,6 +12,9 @@ class Channel::EmailParser
   EXCESSIVE_LINKS_MSG = __('This message cannot be displayed because it contains over 5,000 links. Download the raw message below and open it via an Email client if you still wish to view it.').freeze
   MESSAGE_STRUCT = Struct.new(:from_display_name, :subject, :msg_size).freeze
 
+  UNPROCESSABLE_MAIL_DIRECTORY = Rails.root.join('var/spool/unprocessable_mail')
+  OVERSIZED_MAIL_DIRECTORY = Rails.root.join('var/spool/oversized_mail')
+
 =begin
 
   parser = Channel::EmailParser.new
@@ -124,7 +127,7 @@ def process(channel, msg, exception = true)
     end
   rescue => e
     # store unprocessable email for bug reporting
-    filename = archive_mail('unprocessable_mail', msg)
+    filename = archive_mail(UNPROCESSABLE_MAIL_DIRECTORY, msg)
 
     message = "Can't process email, you will find it for bug reporting under #{filename}, please create an issue at https://github.com/zammad/zammad/issues"
 
@@ -496,16 +499,15 @@ def set_attributes_by_x_headers(item_object, header_name, mail, suffix = false)
 
 =begin
 
-process unprocessable_mails (tmp/unprocessable_mail/*.eml) again
+process unprocessable_mails (var/spool/unprocessable_mail/*.eml) again
 
   Channel::EmailParser.process_unprocessable_mails
 
 =end
 
   def self.process_unprocessable_mails(params = {})
-    path = Rails.root.join('tmp/unprocessable_mail')
     files = []
-    Dir.glob("#{path}/*.eml") do |entry|
+    Dir.glob("#{UNPROCESSABLE_MAIL_DIRECTORY}/*.eml") do |entry|
       ticket, _article, _user, _mail = Channel::EmailParser.new.process(params, File.binread(entry))
       next if ticket.blank?
 
@@ -524,7 +526,7 @@ def self.process_unprocessable_mails(params = {})
 =end
 
   def process_oversized_mail(channel, msg)
-    archive_mail('oversized_mail', msg)
+    archive_mail(OVERSIZED_MAIL_DIRECTORY, msg)
     postmaster_response(channel, msg)
   end
 
@@ -909,17 +911,12 @@ def get_attachments(file, attachments, mail)
   end
 
   # Archive the given message as tmp/folder/md5.eml
-  def archive_mail(folder, msg)
-    path = Rails.root.join('tmp', folder)
+  def archive_mail(path, msg)
     FileUtils.mkpath path
 
-    # MD5 hash the msg and save it as "md5.eml"
-    md5 = Digest::MD5.hexdigest(msg)
-    file_path = Rails.root.join('tmp', folder, "#{md5}.eml")
-
-    File.binwrite(file_path, msg)
-
-    file_path
+    path.join("#{Digest::MD5.hexdigest(msg)}.eml").tap do |file_path|
+      File.binwrite(file_path, msg)
+    end
   end
 
   # Auto reply as the postmaster to oversized emails with:

contrib/backup/functions

@@ -155,8 +155,7 @@ function backup_files () {
       chown -R zammad:zammad ${ZAMMAD_DIR}/storage/
     fi
 
-      tar -C / -czf ${BACKUP_DIR}/${TIMESTAMP}_zammad_files.tar.gz\
-        ${ZAMMAD_DIR#/}/storage/\
+      tar -C / -czf ${BACKUP_DIR}/${TIMESTAMP}_zammad_files.tar.gz ${ZAMMAD_DIR#/}/storage/ ${ZAMMAD_DIR#/}/var/
 
       state=$?
   fi

db/migrate/20230330122449_relocate_unprocessable_mails.rb

@@ -0,0 +1,21 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+class RelocateUnprocessableMails < ActiveRecord::Migration[6.1]
+  def change
+    # return if it's a new setup
+    return if !Setting.exists?(name: 'system_init_done')
+
+    relocate_files('unprocessable_mail')
+    relocate_files('oversized_mail')
+  end
+
+  def relocate_files(type)
+    old_dir = Rails.root.join('tmp', type)
+    return if !old_dir.exist? || old_dir.children.empty?
+
+    new_dir = Rails.root.join('var/spool', type)
+    FileUtils.mkdir_p(new_dir)
+    FileUtils.cp_r(old_dir.children, new_dir)
+    FileUtils.rm_r(old_dir)
+  end
+end

lib/monitoring_helper/health_checker/unprocessable_mail.rb

@@ -3,12 +3,11 @@
 module MonitoringHelper
   class HealthChecker
     class UnprocessableMail < Backend
-      DIRECTORY = Rails.root.join('tmp/unprocessable_mail')
 
       def run_health_check
-        return if !File.exist?(DIRECTORY)
+        return if !File.exist?(Channel::EmailParser::UNPROCESSABLE_MAIL_DIRECTORY)
 
-        count = Dir.glob("#{DIRECTORY}/*.eml").count
+        count = Dir.glob("#{Channel::EmailParser::UNPROCESSABLE_MAIL_DIRECTORY}/*.eml").count
 
         return if count.zero?
 

spec/db/migrate/relocate_unprocessable_mails_spec.rb

@@ -0,0 +1,87 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+
+UNPROCESSABLE_DIR_OLD = Rails.root.join('tmp/unprocessable_mail')
+UNPROCESSABLE_DIR_NEW = Rails.root.join('var/spool/unprocessable_mail')
+OVERSIZED_DIR_OLD = Rails.root.join('tmp/oversized_mail')
+OVERSIZED_DIR_NEW = Rails.root.join('var/spool/oversized_mail')
+OLD_DIRS = [UNPROCESSABLE_DIR_OLD, OVERSIZED_DIR_OLD].freeze
+NEW_DIRS = [UNPROCESSABLE_DIR_NEW, OVERSIZED_DIR_NEW].freeze
+DIRS = [UNPROCESSABLE_DIR_OLD, UNPROCESSABLE_DIR_NEW, OVERSIZED_DIR_OLD, OVERSIZED_DIR_NEW].freeze
+
+RSpec.describe RelocateUnprocessableMails, :aggregate_failures, type: :db_migration do
+
+  def remove_all_directories
+    DIRS.each { |dir| FileUtils.rm_r(dir) if File.exist?(dir) }
+  end
+
+  def create_old_directories
+    OLD_DIRS.each { |dir| FileUtils.mkdir_p(dir) }
+  end
+
+  def create_all_directories
+    DIRS.each { |dir| FileUtils.mkdir_p(dir) }
+  end
+
+  before do
+    remove_all_directories
+  end
+
+  after do
+    remove_all_directories
+  end
+
+  context 'when migrating' do
+    context 'without unprocessable mail directory' do
+      it 'does nothing' do
+        migrate
+        DIRS.each { |dir| expect(dir).not_to exist }
+      end
+    end
+
+    context 'with empty unprocessable mail directory' do
+      before do
+        create_old_directories
+      end
+
+      it 'does nothing' do
+        migrate
+        expect(OLD_DIRS).to all(exist)
+        NEW_DIRS.each { |dir| expect(dir).not_to exist }
+      end
+    end
+
+    context 'with unprocessable mails' do
+      before do
+        create_all_directories
+        %w[unprocessable_mail oversized_mail].each do |type|
+          files = [
+            "tmp/#{type}/1.eml",
+            "tmp/#{type}/2.eml",
+            "var/spool/#{type}/2.eml",
+            "var/spool/#{type}/3.eml",
+          ]
+          FileUtils.touch(files.map { |f| Rails.root.join(f) })
+        end
+      end
+
+      it 'removes source directories' do
+        migrate
+        [UNPROCESSABLE_DIR_OLD, OVERSIZED_DIR_OLD].each { |dir| expect(dir).not_to exist }
+      end
+
+      it 'migrates files and keeps existing' do
+        migrate
+        %w[unprocessable_mail oversized_mail].each do |type|
+          files = [
+            "var/spool/#{type}/1.eml",
+            "var/spool/#{type}/2.eml",
+            "var/spool/#{type}/3.eml",
+          ]
+          files.each { |f| expect(Pathname.new(f)).to exist }
+        end
+      end
+    end
+  end
+end

spec/lib/monitoring_helper/health_checker/unprocessable_mail_spec.rb

@@ -7,7 +7,8 @@
   let(:folder)    { SecureRandom.hex }
   let(:directory) { Rails.root.join('tmp', folder) }
 
-  before { stub_const("#{described_class}::DIRECTORY", directory) }
+  before { stub_const('Channel::EmailParser::UNPROCESSABLE_MAIL_DIRECTORY', directory) }
+  after { FileUtils.rm_r(directory) if File.exist?(directory) }
 
   describe '#check_health' do
     it 'does nothing if directory missing' do

spec/models/channel/driver/imap_spec.rb

@@ -283,7 +283,7 @@
       end
       let(:oversized_email_md5) { Digest::MD5.hexdigest(oversized_email) }
       let(:oversized_email_size) { format('%<MB>.2f', MB: oversized_email.size.to_f / 1024 / 1024) }
-      let(:oversized_eml_folder) { Rails.root.join('tmp/oversized_mail') }
+      let(:oversized_eml_folder) { Rails.root.join('var/spool/oversized_mail') }
       let(:oversized_eml_file) do
         Dir.entries(oversized_eml_folder).grep(%r{^#{oversized_email_md5}\.eml$}).map { |path| oversized_eml_folder.join(path) }.last
       end
@@ -317,7 +317,7 @@
 
         it 'creates email reply correctly' do
           # 1. verify that the oversized email has been saved locally to:
-          # /tmp/oversized_mail/yyyy-mm-ddThh:mm:ss-:md5.eml
+          # var/spool/oversized_mail/yyyy-mm-ddThh:mm:ss-:md5.eml
           expect(oversized_eml_file).to be_present
 
           # verify that the file is byte for byte identical to the sent message
@@ -327,12 +327,14 @@
           expect(oversized_email_reply).to be_present
 
           # parse the reply mail and verify the various headers
-          expect(parsed_oversized_email_reply).to include({
-                                                            from_email: email_address.email,
-            subject: '[undeliverable] Message too large',
-            'references' => "<#{cid}@zammad.test.com>",
-            'in-reply-to' => "<#{cid}@zammad.test.com>",
-                                                          })
+          expect(parsed_oversized_email_reply).to include(
+            {
+              from_email: email_address.email,
+              subject: '[undeliverable] Message too large',
+              'references' => "<#{cid}@zammad.test.com>",
+              'in-reply-to' => "<#{cid}@zammad.test.com>",
+            }
+          )
 
           # verify the reply mail body content
           expect(parsed_oversized_email_reply[:body]).to match(%r{^Dear Max Mustermann.*Oversized Email Message.*#{oversized_email_size} MB.*0.1 MB.*#{Setting.get('fqdn')}}sm)

spec/models/channel/email_parser/process_unprocessable_mail_spec.rb

@@ -0,0 +1,43 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe 'Channel::EmailParser#process_unprocessable_mail', aggregate_failures: true, type: :model do
+
+  context 'when receiving unprocessable mail' do
+    let(:mail) do
+      <<~MAIL
+        From: ME Bob <[email protected]>
+        To: [email protected]
+        Subject: some subject
+
+        Some Text
+      MAIL
+    end
+    let(:dir) { Channel::EmailParser::UNPROCESSABLE_MAIL_DIRECTORY }
+
+    before do
+      FileUtils.rm_r(dir) if dir.exist?
+      stub_const('Channel::EmailParser::PROZESS_TIME_MAX', -1)
+      begin
+        Channel::EmailParser.new.process({}, mail)
+      rescue RuntimeError
+        # expected
+      end
+    end
+
+    after do
+      FileUtils.rm_r(dir) if dir.exist?
+    end
+
+    it 'saves the unprocessable email into a file' do
+      expect(dir.join('ce61e7319bcc4297c1d7dfea2fbc87dd.eml')).to exist
+    end
+
+    it 'allows reprocessing of the stored email' do
+      stub_const('Channel::EmailParser::PROZESS_TIME_MAX', nil)
+      expect { Channel::EmailParser.process_unprocessable_mails }.to change(Ticket, :count).by(1)
+      expect(dir.join('ce61e7319bcc4297c1d7dfea2fbc87dd.eml')).not_to exist
+    end
+  end
+end

tmp/README.md

@@ -0,0 +1,2 @@
+This folder holds volatile data and may thus be mounted on volatile file systems
+like tmpfs / ramdisk. It does not need to be shared in cluster setups.

var/README.md

@@ -0,0 +1,6 @@
+This folder stores persistent application data like unprocessable emails,
+and must be shared in cluster setups to be available for all nodes.
+
+Possible subfolders (depending on system configuration and usage):
+- `var/spool/unprocessable_mail` - stores emails that could not properly be imported
+- `var/spool/oversized_mail` - stores emails that were rejected as too big