From 46702bf98ad807c56f3179803cda34c732c41c9e Mon Sep 17 00:00:00 2001 From: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com> Date: Tue, 9 Jan 2024 14:03:15 +0100 Subject: [PATCH] Apply suggestions from code review Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com> --- CHANGELOG.md | 7 +++- target/scripts/build/packages.sh | 5 ++- test/helper/sending.bash | 40 ++++++++++--------- .../parallel/set1/spam_virus/rspamd_full.bats | 6 +-- 4 files changed, 33 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 82bfb223..d217a898 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,12 @@ All notable changes to this project will be documented in this file. The format ### Updates - **Tests**: - - existing tests were streamlined and simplified, which is a follow-up of [#3732](https://github.com/docker-mailserver/docker-mailserver/pull/3732) + - Refactored mail sending ([#3747](https://github.com/docker-mailserver/docker-mailserver/pull/3747)): + - This change is a follow-up to [#3732](https://github.com/docker-mailserver/docker-mailserver/pull/3732) from DMS v13.2. + - `swaks` version is now the latest from Github releases instead of the Debian package. + - `_nc_wrapper`, `_send_mail` and related helpers expect the `.txt` filepath extension again. + - `sending.bash` helper methods were refactored to better integrate `swaks` and accommodate different usage contexts. + - `test/files/emails/existing/` files were removed similar to previous removal of SMTP auth files as they became redundant with `swaks`. - **Internal:** - tests: Replace `wc -l` with `grep -c` ([#3752](https://github.com/docker-mailserver/docker-mailserver/pull/3752)) - Postfix is now configured with `smtputf8_enable = no` in our default `main.cf` config (_instead of during container startup_). ([#3750](https://github.com/docker-mailserver/docker-mailserver/pull/3750)) diff --git a/target/scripts/build/packages.sh b/target/scripts/build/packages.sh index 4e9cb93c..3e45edb8 100644 --- a/target/scripts/build/packages.sh +++ b/target/scripts/build/packages.sh @@ -196,8 +196,9 @@ function _install_utils() { curl -sL https://github.com/01mf02/jaq/releases/latest/download/jaq-v1.2.0-x86_64-unknown-linux-musl -o /usr/bin/jaq && chmod +x /usr/bin/jaq _log 'trace' 'Installing swaks' - local SWAKS_RELEASE='swaks-20240103.0' - curl -sSfL "https://github.com/jetmore/swaks/releases/download/v20240103.0/${SWAKS_RELEASE}.tar.gz" | tar -xz + local SWAKS_VERSION='20240103.0' + local SWAKS_RELEASE="swaks-${SWAKS_VERSION}" + curl -sSfL "https://github.com/jetmore/swaks/releases/download/v${SWAKS_VERSION}/${SWAKS_RELEASE}.tar.gz" | tar -xz mv "${SWAKS_RELEASE}/swaks" /usr/local/bin rm -r "${SWAKS_RELEASE}" } diff --git a/test/helper/sending.bash b/test/helper/sending.bash index 4a6b2600..86c525c1 100644 --- a/test/helper/sending.bash +++ b/test/helper/sending.bash @@ -7,13 +7,12 @@ # ! ATTENTION: This file is loaded by `common.sh` - do not load it yourself! # ! ATTENTION: This file requires helper functions from `common.sh`! -# Parse the arguments given to `_send_email` and `_send_email_unchecked` -# and print the command that is to be exuted in these functions. How these -# functions then handle the result of the invocation depends on the function. +# Parse the arguments given to `_send_email` and `_send_email_unchecked`. +# Outputs the `swaks` command to later be executed by the calling function. # # ## Note # -# This function is internal and should not be used in tests. +# This is an internal support function, it should not be used directly by tests. function __parse_swaks_arguments() { [[ -v CONTAINER_NAME ]] || return 1 @@ -50,9 +49,9 @@ function __parse_swaks_arguments() { done if [[ ${DATA_WAS_SUPPLIED} -eq 0 ]]; then - # Fallback template without implicit `Message-Id` + `X-Mailer` headers: - # NOTE: It is better to let Postfix generate the `Message-Id` header and append that - # as it contains the queue ID for tracking logs and is returned to in swaks output. + # Fallback template (without the implicit `Message-Id` + `X-Mailer` headers from swaks): + # NOTE: It is better to let Postfix generate and append the `Message-Id` header itself, + # as it will contain the Queue ID for tracking in logs (which is also returned in swaks output). ADDITIONAL_SWAKS_OPTIONS+=('--data') ADDITIONAL_SWAKS_OPTIONS+=("'Date: %DATE%\nTo: %TO_ADDRESS%\nFrom: %FROM_ADDRESS%\nSubject: test %DATE%\n%NEW_HEADERS%\n%BODY%\n'") fi @@ -63,12 +62,14 @@ function __parse_swaks_arguments() { # Sends a mail from the container named by the environment variable `CONTAINER_NAME` # to the same or another container. # -# To send a custom email, create a file at `test/files/`, -# and provide `` as an argument to this function. +# To send a custom email: +# 1. Create a file at `test/files/` +# 2. Provide `` as an argument to this function. # # Parameters include all options that one can supply to `swaks` itself. -# The `--data` parameter expects either, a relative path from `emails/` or -# "inline" data, e.g., `Date: 1 Jan 2024\nSubject: This is a test`. +# The `--data` parameter expects a value of either: +# - A relative path from `test/files/emails/` +# - An "inline" data string (e.g., `Date: 1 Jan 2024\nSubject: This is a test`) # # ## Output # @@ -82,10 +83,9 @@ function __parse_swaks_arguments() { # This function will send the email in an "asynchronous" fashion, # it will return without waiting for the Postfix mail queue to be emptied. # -# This functions performs **no** implicit `assert_success` to check whether -# the e-mail transaction was successful. If this is not desirable, use -# `_send_email`. If you anticipate the sending to succeed, use `_send_email` -# instead. +# This functions performs **no** implicit `assert_success` to check whether the +# e-mail transaction was successful. If this is not desirable, use `_send_email`. +# If you anticipate the sending to succeed, use `_send_email` instead. function _send_email_unchecked() { local COMMAND_STRING=$(__parse_swaks_arguments "${@}") _run_in_container_bash "${COMMAND_STRING}" @@ -98,12 +98,14 @@ function _send_email_unchecked() { # Sends a mail from the container named by the environment variable `CONTAINER_NAME` # to the same or another container. # -# To send a custom email, create a file at `test/files/`, -# and provide `` as an argument to this function. +# To send a custom email: +# 1. Create a file at `test/files/` +# 2. Provide `` as an argument to this function. # # Parameters include all options that one can supply to `swaks` itself. -# The `--data` parameter expects either, a relative path from `emails/` or -# "inline" data, e.g., `Date: 1 Jan 2024\nSubject: This is a test`. +# The `--data` parameter expects a value of either: +# - A relative path from `test/files/emails/` +# - An "inline" data string (e.g., `Date: 1 Jan 2024\nSubject: This is a test`) # # ## Output # diff --git a/test/tests/parallel/set1/spam_virus/rspamd_full.bats b/test/tests/parallel/set1/spam_virus/rspamd_full.bats index 7578724b..d21bfc44 100644 --- a/test/tests/parallel/set1/spam_virus/rspamd_full.bats +++ b/test/tests/parallel/set1/spam_virus/rspamd_full.bats @@ -46,14 +46,14 @@ function setup_file() { # We will send 4 emails: # 1. The first one should pass just fine _send_email_and_get_id MAIL_ID_PASS - # 2. The second one should be rejected due to spam (GTube pattern) + # 2. The second one should be rejected due to spam (GTUBE pattern) _send_email_and_get_id MAIL_ID_SPAM --unchecked \ --body 'XJS*C4JDBQADN1.NSBN3*2IDNEN*GTUBE-STANDARD-ANTI-UBE-TEST-EMAIL*C.34X' - # 3. Te third one should be rejected due to a virus (ClamAV Eicar pattern) + # 3. The third one should be rejected due to a virus (ClamAV EICAR pattern) # shellcheck disable=SC2016 _send_email_and_get_id MAIL_ID_VIRUS --unchecked \ --body 'X5O!P%@AP[4\PZX54(P^)7CC)7}$EICAR-STANDARD-ANTIVIRUS-TEST-FILE!$H+H*' - # 4. The fourth one will receive an added header (GTube pattern) + # 4. The fourth one will receive an added header (GTUBE pattern) _send_email_and_get_id MAIL_ID_HEADER \ --body 'YJS*C4JDBQADN1.NSBN3*2IDNEN*GTUBE-STANDARD-ANTI-UBE-TEST-EMAIL*C.34X'