From 54904aa02ca9066f4cd8508b26085d1b09ef2130 Mon Sep 17 00:00:00 2001
From: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
Date: Tue, 7 Jun 2022 01:07:30 +1200
Subject: [PATCH] chore(housekeeping): Normalize how config files filter out
 unwanted lines (#2619)

* chore(`aliases.sh`): Filepath to local var `DATABASE_VIRTUAL`

* chore(`accounts.sh`): Filepath to local var `DATABASE_ACCOUNTS`

* chore(`accounts.sh`): Filepath to local var `DATABASE_VIRTUAL`

* chore(`accounts.sh`): Filepath to local var `DATABASE_DOVECOT_MASTERS`

* chore(`bin/open-dkim`): Filepaths to local vars (accounts,virtual,vhost)

* chore(`relay.sh`): Filepath to local var `DATABASE_SASL_PASSWD`

* chore: Rename method

Prior PR feedback suggested a better helper method name.

* chore: Normalize filtering config lines as input for iterating

* chore: Remove `_is_comment` helper method

No longer serving a purpose with more appropriate filter method for pre-processing the entire config file.
---
 target/bin/listalias                  |  2 +-
 target/bin/listdovecotmasteruser      |  2 +-
 target/bin/listmailuser               |  2 +-
 target/bin/open-dkim                  | 29 +++++++++++---------
 target/scripts/helpers/accounts.sh    | 38 +++++++++++++--------------
 target/scripts/helpers/aliases.sh     | 14 +++++-----
 target/scripts/helpers/relay.sh       | 11 +++-----
 target/scripts/helpers/utils.sh       | 10 +------
 target/scripts/startup/setup-stack.sh |  3 +--
 9 files changed, 51 insertions(+), 60 deletions(-)

diff --git a/target/bin/listalias b/target/bin/listalias
index 53b19261..0eb90b40 100755
--- a/target/bin/listalias
+++ b/target/bin/listalias
@@ -8,5 +8,5 @@ DATABASE='/tmp/docker-mailserver/postfix-virtual.cf'
 [[ -f ${DATABASE} ]] || _exit_with_error "No 'postfix-virtual.cf' file"
 [[ -s ${DATABASE} ]] || _exit_with_error "Empty 'postfix-virtual.cf' - no aliases have been added"
 
-grep -v "^\s*$\|^\s*\#" "${DATABASE}"
+_get_valid_lines_from_file "${DATABASE}"
 exit 0
diff --git a/target/bin/listdovecotmasteruser b/target/bin/listdovecotmasteruser
index 6dd33ae9..d7766781 100755
--- a/target/bin/listdovecotmasteruser
+++ b/target/bin/listdovecotmasteruser
@@ -16,6 +16,6 @@ while read -r LINE
 do
   USER=$(echo "${LINE}" | cut -d'|' -f1)
   echo "* ${USER}"
-done < "${DATABASE}"
+done < <(_get_valid_lines_from_file "${DATABASE}")
 
 exit 0
diff --git a/target/bin/listmailuser b/target/bin/listmailuser
index 95dbc070..8dd7a0e4 100755
--- a/target/bin/listmailuser
+++ b/target/bin/listmailuser
@@ -47,7 +47,7 @@ do
   else
     echo
   fi
-done < <(_filter_to_valid_lines "${DATABASE}")
+done < <(_get_valid_lines_from_file "${DATABASE}")
 
 
 exit 0
diff --git a/target/bin/open-dkim b/target/bin/open-dkim
index cf0086c7..9c970812 100755
--- a/target/bin/open-dkim
+++ b/target/bin/open-dkim
@@ -98,23 +98,26 @@ do
   esac
 done
 
-touch /tmp/vhost.dkim.tmp
-
+DATABASE_ACCOUNTS='/tmp/docker-mailserver/postfix-accounts.cf'
+DATABASE_VIRTUAL='/tmp/docker-mailserver/postfix-virtual.cf'
+DATABASE_VHOST='/tmp/vhost'
+TMP_VHOST='/tmp/vhost.dkim.tmp'
+touch "${TMP_VHOST}"
 if [[ -z ${DOMAINS} ]]
 then
   # getting domains FROM mail accounts
-  if [[ -f /tmp/docker-mailserver/postfix-accounts.cf ]]
+  if [[ -f ${DATABASE_ACCOUNTS} ]]
   then
     # shellcheck disable=SC2034
     while IFS=$'|' read -r LOGIN PASS
     do
       DOMAIN=$(echo "${LOGIN}" | cut -d @ -f2)
-      echo "${DOMAIN}" >>/tmp/vhost.dkim.tmp
-    done < <(grep -v "^\s*$\|^\s*\#" /tmp/docker-mailserver/postfix-accounts.cf || true)
+      echo "${DOMAIN}" >>"${TMP_VHOST}"
+    done < <(_get_valid_lines_from_file "${DATABASE_ACCOUNTS}")
   fi
 
   # getting domains FROM mail aliases
-  if [[ -f /tmp/docker-mailserver/postfix-virtual.cf ]]
+  if [[ -f ${DATABASE_VIRTUAL} ]]
   then
     # shellcheck disable=SC2034
     while read -r FROM TO
@@ -122,17 +125,17 @@ then
       UNAME=$(echo "${FROM}" | cut -d @ -f1)
       DOMAIN=$(echo "${FROM}" | cut -d @ -f2)
 
-      [[ ${UNAME} != "${DOMAIN}" ]] && echo "${DOMAIN}" >>/tmp/vhost.dkim.tmp
-    done < <(grep -v "^\s*$\|^\s*\#" /tmp/docker-mailserver/postfix-virtual.cf || true)
+      [[ ${UNAME} != "${DOMAIN}" ]] && echo "${DOMAIN}" >>"${TMP_VHOST}"
+    done < <(_get_valid_lines_from_file "${DATABASE_VIRTUAL}")
   fi
 else
-  tr ',' '\n' <<< "${DOMAINS}" >/tmp/vhost.dkim.tmp
+  tr ',' '\n' <<< "${DOMAINS}" >"${TMP_VHOST}"
 fi
 
-sort < /tmp/vhost.dkim.tmp | uniq >/tmp/vhost
-rm /tmp/vhost.dkim.tmp
+sort < "${TMP_VHOST}" | uniq >"${DATABASE_VHOST}"
+rm "${TMP_VHOST}"
 
-if [[ ! -s /tmp/vhost ]]
+if [[ ! -s ${DATABASE_VHOST} ]]
 then
   _log 'warn' 'No entries found, no keys to make'
   exit 0
@@ -179,7 +182,7 @@ do
       echo "${SIGNINGTABLEENTRY}" >>/tmp/docker-mailserver/opendkim/SigningTable
     fi
   fi
-done < <(grep -vE '^(\s*$|#)' /tmp/vhost)
+done < <(_get_valid_lines_from_file "${DATABASE_VHOST}")
 
 # create TrustedHosts if missing
 if [[ -d /tmp/docker-mailserver/opendkim ]] && [[ ! -f /tmp/docker-mailserver/opendkim/TrustedHosts ]]
diff --git a/target/scripts/helpers/accounts.sh b/target/scripts/helpers/accounts.sh
index e0a0b229..c4c23a35 100644
--- a/target/scripts/helpers/accounts.sh
+++ b/target/scripts/helpers/accounts.sh
@@ -16,18 +16,19 @@ function _create_accounts
 
   [[ ${ENABLE_LDAP} -eq 1 ]] && return 0
 
+  local DATABASE_ACCOUNTS='/tmp/docker-mailserver/postfix-accounts.cf'
   _create_masters
-  if [[ -f /tmp/docker-mailserver/postfix-accounts.cf ]]
+  if [[ -f ${DATABASE_ACCOUNTS} ]]
   then
     _log 'trace' "Checking file line endings"
-    sed -i 's|\r||g' /tmp/docker-mailserver/postfix-accounts.cf
+    sed -i 's|\r||g' "${DATABASE_ACCOUNTS}"
 
     _log 'trace' "Regenerating postfix user list"
-    echo "# WARNING: this file is auto-generated. Modify /tmp/docker-mailserver/postfix-accounts.cf to edit the user list." > /etc/postfix/vmailbox
+    echo "# WARNING: this file is auto-generated. Modify ${DATABASE_ACCOUNTS} to edit the user list." > /etc/postfix/vmailbox
 
-    # checking that /tmp/docker-mailserver/postfix-accounts.cf ends with a newline
+    # checking that ${DATABASE_ACCOUNTS} ends with a newline
     # shellcheck disable=SC1003
-    sed -i -e '$a\' /tmp/docker-mailserver/postfix-accounts.cf
+    sed -i -e '$a\' "${DATABASE_ACCOUNTS}"
 
     chown dovecot:dovecot "${DOVECOT_USERDB_FILE}"
     chmod 640 "${DOVECOT_USERDB_FILE}"
@@ -92,7 +93,7 @@ function _create_accounts
       fi
 
       echo "${DOMAIN}" >>/tmp/vhost.tmp
-    done < <(grep -v "^\s*$\|^\s*\#" /tmp/docker-mailserver/postfix-accounts.cf)
+    done < <(_get_valid_lines_from_file "${DATABASE_ACCOUNTS}")
 
     _create_dovecot_alias_dummy_accounts
   fi
@@ -107,16 +108,15 @@ function _create_accounts
 # for more details on this method
 function _create_dovecot_alias_dummy_accounts
 {
-  if [[ -f /tmp/docker-mailserver/postfix-virtual.cf ]] && [[ ${ENABLE_QUOTAS} -eq 1 ]]
+  local DATABASE_VIRTUAL='/tmp/docker-mailserver/postfix-virtual.cf'
+
+  if [[ -f ${DATABASE_VIRTUAL} ]] && [[ ${ENABLE_QUOTAS} -eq 1 ]]
   then
     # adding aliases to Dovecot's userdb
     # ${REAL_FQUN} is a user's fully-qualified username
     local ALIAS REAL_FQUN DOVECOT_USERDB_LINE
     while read -r ALIAS REAL_FQUN
     do
-      # ignore comments
-      [[ ${ALIAS} == \#* ]] && continue
-
       # alias is assumed to not be a proper e-mail
       # these aliases do not need to be added to Dovecot's userdb
       [[ ! ${ALIAS} == *@* ]] && continue
@@ -130,7 +130,7 @@ function _create_dovecot_alias_dummy_accounts
       REAL_USERNAME=$(cut -d '@' -f 1 <<< "${REAL_FQUN}")
       REAL_DOMAINNAME=$(cut -d '@' -f 2 <<< "${REAL_FQUN}")
 
-      if ! grep -q "${REAL_FQUN}" /tmp/docker-mailserver/postfix-accounts.cf
+      if ! grep -q "${REAL_FQUN}" "${DATABASE_ACCOUNTS}"
       then
         _log 'debug' "Alias '${ALIAS}' is non-local (or mapped to a non-existing account) and will not be added to Dovecot's userdb"
         continue
@@ -141,7 +141,7 @@ function _create_dovecot_alias_dummy_accounts
       # ${REAL_ACC[0]} => real account name (e-mail address) == ${REAL_FQUN}
       # ${REAL_ACC[1]} => password hash
       # ${REAL_ACC[2]} => optional user attributes
-      IFS='|' read -r -a REAL_ACC < <(grep "${REAL_FQUN}" /tmp/docker-mailserver/postfix-accounts.cf)
+      IFS='|' read -r -a REAL_ACC < <(grep "${REAL_FQUN}" "${DATABASE_ACCOUNTS}")
 
       if [[ -z ${REAL_ACC[1]} ]]
       then
@@ -165,7 +165,7 @@ function _create_dovecot_alias_dummy_accounts
       else
         echo "${DOVECOT_USERDB_LINE}" >>"${DOVECOT_USERDB_FILE}"
       fi
-    done < /tmp/docker-mailserver/postfix-virtual.cf
+    done < <(_get_valid_lines_from_file "${DATABASE_VIRTUAL}")
   fi
 }
 
@@ -175,16 +175,17 @@ function _create_masters
 {
   : >"${DOVECOT_MASTERDB_FILE}"
 
-  if [[ -f /tmp/docker-mailserver/dovecot-masters.cf ]]
+  local DATABASE_DOVECOT_MASTERS='/tmp/docker-mailserver/dovecot-masters.cf'
+  if [[ -f ${DATABASE_DOVECOT_MASTERS} ]]
   then
     _log 'trace' "Checking file line endings"
-    sed -i 's|\r||g' /tmp/docker-mailserver/dovecot-masters.cf
+    sed -i 's|\r||g' "${DATABASE_DOVECOT_MASTERS}"
 
     _log 'trace' "Regenerating dovecot masters list"
 
-    # checking that /tmp/docker-mailserver/dovecot-masters.cf ends with a newline
+    # checking that ${DATABASE_DOVECOT_MASTERS} ends with a newline
     # shellcheck disable=SC1003
-    sed -i -e '$a\' /tmp/docker-mailserver/dovecot-masters.cf
+    sed -i -e '$a\' "${DATABASE_DOVECOT_MASTERS}"
 
     chown dovecot:dovecot "${DOVECOT_MASTERDB_FILE}"
     chmod 640 "${DOVECOT_MASTERDB_FILE}"
@@ -209,7 +210,6 @@ function _create_masters
       else
         echo "${DOVECOT_MASTERDB_LINE}" >>"${DOVECOT_MASTERDB_FILE}"
       fi
-
-    done < <(grep -v "^\s*$\|^\s*\#" /tmp/docker-mailserver/dovecot-masters.cf)
+    done < <(_get_valid_lines_from_file "${DATABASE_DOVECOT_MASTERS}")
   fi
 }
diff --git a/target/scripts/helpers/aliases.sh b/target/scripts/helpers/aliases.sh
index 1a4febc1..dc0c2359 100644
--- a/target/scripts/helpers/aliases.sh
+++ b/target/scripts/helpers/aliases.sh
@@ -11,15 +11,17 @@ function _handle_postfix_virtual_config
   : >/etc/postfix/virtual
   : >/etc/postfix/regexp
 
-  if [[ -f /tmp/docker-mailserver/postfix-virtual.cf ]]
+  local DATABASE_VIRTUAL=/tmp/docker-mailserver/postfix-virtual.cf
+
+  if [[ -f ${DATABASE_VIRTUAL} ]]
   then
     # fixing old virtual user file
-    if grep -q ",$" /tmp/docker-mailserver/postfix-virtual.cf
+    if grep -q ",$" "${DATABASE_VIRTUAL}"
     then
-      sed -i -e "s|, |,|g" -e "s|,$||g" /tmp/docker-mailserver/postfix-virtual.cf
+      sed -i -e "s|, |,|g" -e "s|,$||g" "${DATABASE_VIRTUAL}"
     fi
 
-    cp -f /tmp/docker-mailserver/postfix-virtual.cf /etc/postfix/virtual
+    cp -f "${DATABASE_VIRTUAL}" /etc/postfix/virtual
 
     # the `to` is important, don't delete it
     # shellcheck disable=SC2034
@@ -30,9 +32,9 @@ function _handle_postfix_virtual_config
 
       # if they are equal it means the line looks like: "user1     other@domain.tld"
       [[ ${UNAME} != "${DOMAIN}" ]] && echo "${DOMAIN}" >>/tmp/vhost.tmp
-    done < <(grep -v "^\s*$\|^\s*\#" /tmp/docker-mailserver/postfix-virtual.cf || true)
+    done < <(_get_valid_lines_from_file "${DATABASE_VIRTUAL}")
   else
-    _log 'debug' "'/tmp/docker-mailserver/postfix-virtual.cf' not provided - no mail alias/forward created"
+    _log 'debug' "'${DATABASE_VIRTUAL}' not provided - no mail alias/forward created"
   fi
 }
 
diff --git a/target/scripts/helpers/relay.sh b/target/scripts/helpers/relay.sh
index 8852bd44..fee045e0 100644
--- a/target/scripts/helpers/relay.sh
+++ b/target/scripts/helpers/relay.sh
@@ -92,16 +92,11 @@ function _relayhost_sasl
     echo "${SASL_PASSWD}" >> /etc/postfix/sasl_passwd
   fi
 
-  if [[ -f /tmp/docker-mailserver/postfix-sasl-password.cf ]]
+  local DATABASE_SASL_PASSWD='/tmp/docker-mailserver/postfix-sasl-password.cf'
+  if [[ -f ${DATABASE_SASL_PASSWD} ]]
   then
     # Add domain-specific auth from config file:
-    while read -r LINE
-    do
-      if ! _is_comment "${LINE}"
-      then
-        echo "${LINE}" >> /etc/postfix/sasl_passwd
-      fi
-    done < /tmp/docker-mailserver/postfix-sasl-password.cf
+    _get_valid_lines_from_file "${DATABASE_SASL_PASSWD}" >> /etc/postfix/sasl_passwd
 
     # Only relevant when providing this user config (unless users append elsewhere too)
     postconf 'smtp_sender_dependent_authentication = yes'
diff --git a/target/scripts/helpers/utils.sh b/target/scripts/helpers/utils.sh
index 13d39d12..4c6e5837 100644
--- a/target/scripts/helpers/utils.sh
+++ b/target/scripts/helpers/utils.sh
@@ -7,19 +7,11 @@ function _escape
 
 # Returns input after filtering out lines that are:
 # empty, white-space, comments (`#` as the first non-whitespace character)
-function _filter_to_valid_lines
+function _get_valid_lines_from_file
 {
   grep --extended-regexp --invert-match "^\s*$|^\s*#" "${1}" || true
 }
 
-# TODO: Only used by `relay.sh`, will be removed in future.
-# Similar to _filter_to_valid_lines, but only returns a status code
-# to indicate invalid line(s):
-function _is_comment
-{
-  grep -q -E "^\s*$|^\s*#" <<< "${1}"
-}
-
 # Provide the name of an environment variable to this function
 # and it will return its value stored in /etc/dms-settings
 function _get_dms_env_value
diff --git a/target/scripts/startup/setup-stack.sh b/target/scripts/startup/setup-stack.sh
index d100ddfb..1dbd84d3 100644
--- a/target/scripts/startup/setup-stack.sh
+++ b/target/scripts/startup/setup-stack.sh
@@ -1162,8 +1162,7 @@ function _setup_fetchmail_parallel
         # Just the server settings that need to be added to the specific rc.d file
         echo "${LINE}" >>"${FETCHMAILRCD}/fetchmail-${COUNTER}.rc"
       fi
-    # delete commented lines before parsing
-    done < <(sed '/^[[:space:]]*#/d' "${FETCHMAILRC}")
+    done < <(_get_valid_lines_from_file "${FETCHMAILRC}")
 
     rm "${DEFAULT_FILE}"
   }