Skip to content

Conversation

@FredrikAppelros
Copy link
Contributor

Only use the email address in the MAIL FROM command to avoid SMTP errors.

@FredrikAppelros FredrikAppelros requested a review from umputun as a code owner April 1, 2025 23:18
@FredrikAppelros
Copy link
Contributor Author

Hey! I created this as a fix to being able to use a display name for the from address in Remark42. Without this I am getting 501 errors from the SMTP server when using a from address on the format My Display Name <from@example.com>.

@umputun umputun requested review from Copilot and paskal April 2, 2025 04:56
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for handling email display names by ensuring only the email address is used in the MAIL FROM command to avoid SMTP errors.

  • Added a new test (TestEmail_SendWithDisplayName) to validate display name handling.
  • Modified the MAIL FROM logic to extract just the email address using a new helper, extractEmailAddress.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
email_test.go Introduces tests for verifying proper extraction of the email address from a display name format.
email.go Implements extraction logic to filter out the display name when calling the SMTP client.

Only use the email address in the MAIL FROM command to avoid SMTP errors.
…ests

- Replace regex with net/mail.ParseAddress for RFC 5322 compliance
- Apply extractEmailAddress to recipients (params.To) for consistency
- Add strings.TrimSpace to handle whitespace in pasted addresses
- Add table-driven unit tests covering various email formats
- Fix import ordering and normalize comments
@paskal
Copy link
Contributor

paskal commented Dec 3, 2025

LGTM, will merge and update remark42 soon.

@umputun umputun merged commit 16337d0 into go-pkgz:master Dec 3, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants