Apply suggestions from code review

Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
This commit is contained in:
Georg Lauterbach 2024-01-09 14:03:15 +01:00 committed by GitHub
parent f67362ed74
commit 46702bf98a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 33 additions and 25 deletions

View File

@ -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))

View File

@ -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}"
}

View File

@ -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/<TEST FILE>`,
# and provide `<TEST FILE>` as an argument to this function.
# To send a custom email:
# 1. Create a file at `test/files/<TEST FILE>`
# 2. Provide `<TEST FILE>` 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/<TEST FILE>`,
# and provide `<TEST FILE>` as an argument to this function.
# To send a custom email:
# 1. Create a file at `test/files/<TEST FILE>`
# 2. Provide `<TEST FILE>` 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
#

View File

@ -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'