diff --git a/CHANGELOG.md b/CHANGELOG.md index 12f07561..9f440e31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,10 @@ The most noteworthy change of this release is the update of the container's base - **Tests:** - Refactored helper methods for sending e-mails with specific `Message-ID` headers and the helpers for retrieving + filtering logs, which together help isolate logs relevant to specific mail when multiple mails have been processed within a single test. ([#3786](https://github.com/docker-mailserver/docker-mailserver/pull/3786)) +### Fixes + +- DMS config files that are parsed line by line are now more robust to parse by detecting and fixing line-endings ([#3819](https://github.com/docker-mailserver/docker-mailserver/pull/3819)) + ## [v13.3.1](https://github.com/docker-mailserver/docker-mailserver/releases/tag/v13.3.1) ### Fixes diff --git a/target/scripts/helpers/accounts.sh b/target/scripts/helpers/accounts.sh index 7499a2ec..31ded04a 100644 --- a/target/scripts/helpers/accounts.sh +++ b/target/scripts/helpers/accounts.sh @@ -19,16 +19,9 @@ function _create_accounts() { _create_masters if [[ -f ${DATABASE_ACCOUNTS} ]]; then - _log 'trace' "Checking file line endings" - sed -i 's|\r||g' "${DATABASE_ACCOUNTS}" - _log 'trace' "Regenerating postfix user list" echo "# WARNING: this file is auto-generated. Modify ${DATABASE_ACCOUNTS} to edit the user list." > /etc/postfix/vmailbox - # checking that ${DATABASE_ACCOUNTS} ends with a newline - # shellcheck disable=SC1003 - sed -i -e '$a\' "${DATABASE_ACCOUNTS}" - chown dovecot:dovecot "${DOVECOT_USERDB_FILE}" chmod 640 "${DOVECOT_USERDB_FILE}" @@ -158,15 +151,8 @@ function _create_masters() { 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' "${DATABASE_DOVECOT_MASTERS}" - _log 'trace' "Regenerating dovecot masters list" - # checking that ${DATABASE_DOVECOT_MASTERS} ends with a newline - # shellcheck disable=SC1003 - sed -i -e '$a\' "${DATABASE_DOVECOT_MASTERS}" - chown dovecot:dovecot "${DOVECOT_MASTERDB_FILE}" chmod 640 "${DOVECOT_MASTERDB_FILE}" diff --git a/target/scripts/helpers/aliases.sh b/target/scripts/helpers/aliases.sh index 04a56da3..b0f2fa1a 100644 --- a/target/scripts/helpers/aliases.sh +++ b/target/scripts/helpers/aliases.sh @@ -12,11 +12,6 @@ function _handle_postfix_virtual_config() { local DATABASE_VIRTUAL=/tmp/docker-mailserver/postfix-virtual.cf if [[ -f ${DATABASE_VIRTUAL} ]]; then - # fixing old virtual user file - if grep -q ",$" "${DATABASE_VIRTUAL}"; then - sed -i -e "s|, |,|g" -e "s|,$||g" "${DATABASE_VIRTUAL}" - fi - cp -f "${DATABASE_VIRTUAL}" /etc/postfix/virtual else _log 'debug' "'${DATABASE_VIRTUAL}' not provided - no mail alias/forward created" diff --git a/target/scripts/helpers/utils.sh b/target/scripts/helpers/utils.sh index f7095bf3..c848e13e 100644 --- a/target/scripts/helpers/utils.sh +++ b/target/scripts/helpers/utils.sh @@ -17,9 +17,44 @@ function _escape_for_sed() { # Returns input after filtering out lines that are: # empty, white-space, comments (`#` as the first non-whitespace character) function _get_valid_lines_from_file() { + _convert_crlf_to_lf_if_necessary "${1}" + _append_final_newline_if_missing "${1}" + grep --extended-regexp --invert-match "^\s*$|^\s*#" "${1}" || true } +# This is to sanitize configs from users that unknowingly introduced CRLF: +function _convert_crlf_to_lf_if_necessary() { + if [[ $(file "${1}") =~ 'CRLF' ]]; then + _log 'warn' "File '${1}' contains CRLF line-endings" + + if [[ -w ${1} ]]; then + _log 'debug' 'Converting CRLF to LF' + sed -i 's|\r||g' "${1}" + else + _log 'warn' "File '${1}' is not writable - cannot change CRLF to LF" + fi + fi +} + +# This is to sanitize configs from users that unknowingly removed the end-of-file LF: +function _append_final_newline_if_missing() { + # Correctly detect a missing final newline and fix it: + # https://stackoverflow.com/questions/38746/how-to-detect-file-ends-in-newline#comment82380232_25749716 + # https://unix.stackexchange.com/questions/31947/how-to-add-a-newline-to-the-end-of-a-file/441200#441200 + # https://unix.stackexchange.com/questions/159557/how-to-non-invasively-test-for-write-access-to-a-file + if [[ $(tail -c1 "${1}" | wc -l) -eq 0 ]]; then + # Avoid fixing when the destination is read-only: + if [[ -w ${1} ]]; then + printf '\n' >> "${1}" + + _log 'info' "File '${1}' was missing a final newline - this has been fixed" + else + _log 'warn' "File '${1}' is missing a final newline - it is not writable, hence it was not fixed - the last line will not be processed!" + fi + fi +} + # 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.d/postfix.sh b/target/scripts/startup/setup.d/postfix.sh index 5aec8636..05052faa 100644 --- a/target/scripts/startup/setup.d/postfix.sh +++ b/target/scripts/startup/setup.d/postfix.sh @@ -109,8 +109,9 @@ function _setup_postfix_late() { function __postfix__setup_override_configuration() { __postfix__log 'debug' 'Overriding / adjusting configuration with user-supplied values' - if [[ -f /tmp/docker-mailserver/postfix-main.cf ]]; then - cat /tmp/docker-mailserver/postfix-main.cf >>/etc/postfix/main.cf + local OVERRIDE_CONFIG_POSTFIX_MAIN='/tmp/docker-mailserver/postfix-main.cf' + if [[ -f ${OVERRIDE_CONFIG_POSTFIX_MAIN} ]]; then + cat "${OVERRIDE_CONFIG_POSTFIX_MAIN}" >>/etc/postfix/main.cf _adjust_mtime_for_postfix_maincf # do not directly output to 'main.cf' as this causes a read-write-conflict @@ -118,20 +119,19 @@ function __postfix__setup_override_configuration() { mv /tmp/postfix-main-new.cf /etc/postfix/main.cf _adjust_mtime_for_postfix_maincf - __postfix__log 'trace' "Adjusted '/etc/postfix/main.cf' according to '/tmp/docker-mailserver/postfix-main.cf'" + __postfix__log 'trace' "Adjusted '/etc/postfix/main.cf' according to '${OVERRIDE_CONFIG_POSTFIX_MAIN}'" else - __postfix__log 'trace' "No extra Postfix settings loaded because optional '/tmp/docker-mailserver/postfix-main.cf' was not provided" + __postfix__log 'trace' "No extra Postfix settings loaded because optional '${OVERRIDE_CONFIG_POSTFIX_MAIN}' was not provided" fi - if [[ -f /tmp/docker-mailserver/postfix-master.cf ]]; then + local OVERRIDE_CONFIG_POSTFIX_MASTER='/tmp/docker-mailserver/postfix-master.cf' + if [[ -f ${OVERRIDE_CONFIG_POSTFIX_MASTER} ]]; then while read -r LINE; do - if [[ ${LINE} =~ ^[0-9a-z] ]]; then - postconf -P "${LINE}" - fi - done < /tmp/docker-mailserver/postfix-master.cf - __postfix__log 'trace' "Adjusted '/etc/postfix/master.cf' according to '/tmp/docker-mailserver/postfix-master.cf'" + [[ ${LINE} =~ ^[0-9a-z] ]] && postconf -P "${LINE}" + done < <(_get_valid_lines_from_file "${OVERRIDE_CONFIG_POSTFIX_MASTER}") + __postfix__log 'trace' "Adjusted '/etc/postfix/master.cf' according to '${OVERRIDE_CONFIG_POSTFIX_MASTER}'" else - __postfix__log 'trace' "No extra Postfix settings loaded because optional '/tmp/docker-mailserver/postfix-master.cf' was not provided" + __postfix__log 'trace' "No extra Postfix settings loaded because optional '${OVERRIDE_CONFIG_POSTFIX_MASTER}' was not provided" fi } diff --git a/test/helper/setup.bash b/test/helper/setup.bash index 0dd57bd6..ed2e4e32 100644 --- a/test/helper/setup.bash +++ b/test/helper/setup.bash @@ -102,7 +102,6 @@ function _init_with_defaults() { # The config volume cannot be read-only as some data needs to be written at container startup # - # - two sed failures (unknown lines) # - dovecot-quotas.cf (setup-stack.sh:_setup_dovecot_quotas) # - postfix-aliases.cf (setup-stack.sh:_setup_postfix_aliases) # TODO: Check how many tests need write access. Consider using `docker create` + `docker cp` for easier cleanup. diff --git a/test/tests/parallel/set3/scripts/helper_functions.bats b/test/tests/parallel/set3/scripts/helper_functions.bats index 332de448..518f8717 100644 --- a/test/tests/parallel/set3/scripts/helper_functions.bats +++ b/test/tests/parallel/set3/scripts/helper_functions.bats @@ -70,3 +70,45 @@ SOURCE_BASE_PATH="${REPOSITORY_ROOT:?Expected REPOSITORY_ROOT to be set}/target/ assert_failure assert_output --partial "ENV var name must be provided to _env_var_expect_integer" } + +@test '(utils.sh) _convert_crlf_to_lf_if_necessary' { + # shellcheck source=../../../../../target/scripts/helpers/log.sh + source "${SOURCE_BASE_PATH}/log.sh" + # shellcheck source=../../../../../target/scripts/helpers/utils.sh + source "${SOURCE_BASE_PATH}/utils.sh" + + # Create a temporary file in the BATS test-case folder: + local TMP_DMS_CONFIG=$(mktemp -p "${BATS_TEST_TMPDIR}" -t 'dms_XXX.cf') + # A file with mixed line-endings including CRLF: + echo -en 'line one\nline two\r\n' > "${TMP_DMS_CONFIG}" + + # Confirm CRLF detected: + run file "${TMP_DMS_CONFIG}" + assert_output --partial 'CRLF' + + # Helper method detects and fixes: + _convert_crlf_to_lf_if_necessary "${TMP_DMS_CONFIG}" + run file "${TMP_DMS_CONFIG}" + refute_output --partial 'CRLF' +} + +@test '(utils.sh) _append_final_newline_if_missing' { + # shellcheck source=../../../../../target/scripts/helpers/log.sh + source "${SOURCE_BASE_PATH}/log.sh" + # shellcheck source=../../../../../target/scripts/helpers/utils.sh + source "${SOURCE_BASE_PATH}/utils.sh" + + # Create a temporary file in the BATS test-case folder: + local TMP_DMS_CONFIG=$(mktemp -p "${BATS_TEST_TMPDIR}" -t 'dms_XXX.cf') + # A file missing a final newline: + echo -en 'line one\nline two' > "${TMP_DMS_CONFIG}" + + # Confirm missing newline: + run bash -c "tail -c 1 '${TMP_DMS_CONFIG}' | wc -l" + assert_output '0' + + # Helper method detects and fixes: + _append_final_newline_if_missing "${TMP_DMS_CONFIG}" + run bash -c "tail -c 1 '${TMP_DMS_CONFIG}' | wc -l" + assert_output '1' +}