Skip to content

Conversation

imorland
Copy link
Member

@imorland imorland commented Oct 3, 2025

**Fixes #4222 **

Changes proposed in this pull request:
Introduces a reusable trait ValidatesMailSettings which is used by default in SmtpDriver and MailgunDriver to ensure that leading or trailing whitespace throws a validation error, which is displayed in the mail settings UI.

I'll also create an accompanying PR to this for 2.x

For SmtpDriver, we now check for whitespace on

  • mail_host
  • mail_port
  • mail_username *
  • mail_password *

* still allows for empty string

For MailgunDriver

  • mail_mailgun_secret

mail_mailgun_domain is already covered by the regex rule in place

Reviewers should focus on:

Screenshot
image

Necessity

  • Has the problem that is being solved here been clearly explained?
  • If applicable, have various options for solving this problem been considered?
  • For core PRs, does this need to be in core, or could it be in an extension?
  • Are we willing to maintain this for years / potentially forever?

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Backend changes: tests are green (run composer test).
  • Core developer confirmed locally this works as intended.
  • Tests have been added, or are not appropriate here.

Required changes:

  • Related documentation PR: (Remove if irrelevant)

@imorland imorland added this to the 1.8.11 milestone Oct 3, 2025
@imorland imorland requested a review from a team as a code owner October 3, 2025 09:07
@imorland imorland force-pushed the im/mail-validation branch from daeae37 to 38d1dc4 Compare October 3, 2025 09:08
@imorland imorland changed the title fix: validate mail settings for whitespace [1.x] fix: validate mail settings for whitespace Oct 3, 2025
@imorland imorland merged commit 91addb2 into 1.x Oct 3, 2025
680 of 681 checks passed
@imorland imorland deleted the im/mail-validation branch October 3, 2025 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Email Configuration Fails Due to Leading Whitespace in Database Settings
1 participant