Skip to content

fix: correct sender format issues fix #1347 - #2424

Merged
Calcium-Ion merged 1 commit into
QuantumNous:mainfrom
ion1ze:main
Dec 12, 2025
Merged

fix: correct sender format issues fix #1347#2424
Calcium-Ion merged 1 commit into
QuantumNous:mainfrom
ion1ze:main

Conversation

@ion1ze

@ion1ze ion1ze commented Dec 12, 2025

Copy link
Copy Markdown
Contributor
  • Adjust sender field format, add space to separate nickname and email address
  • Ensure email header format complies with standard RFC specifications
  • Fix potential email client sending exceptions (Tencent Cloud)

Resolves #1347

Summary by CodeRabbit

  • Bug Fixes
    • Fixed email header formatting to ensure proper standards compliance in message delivery.

✏️ Tip: You can customize this high-level summary in your review settings.

- Adjust sender field format, add space to separate nickname and email address
- Ensure email header format complies with standard RFC specifications
- Fix potential email client sending exceptions (Tencent Cloud)
@coderabbitai

coderabbitai Bot commented Dec 12, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

A minor formatting adjustment to the SMTP From header in the email module adds a space between the display name and email address, changing the format from From: %s<%s>\r\n to From: %s <%s>\r\n with no functional changes to the email sending logic.

Changes

Cohort / File(s) Summary
SMTP Header Formatting
common/email.go
Added space in From header format string for improved RFC compliance

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐰 A space slips in, so small and neat,
Between the name and address sweet,
The header's dressed in proper style,
RFC smiles, and mail flows mild! ✉️

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly references the issue being fixed (#1347) and clearly indicates a sender format issue correction, accurately reflecting the main change.
Linked Issues check ✅ Passed The code change addresses the Tencent Cloud sender authentication error by correcting SMTP header format to comply with RFC standards, matching issue #1347 objectives.
Out of Scope Changes check ✅ Passed The change is narrowly scoped to fixing the sender format issue in the SMTP header, directly addressing the linked issue with no extraneous modifications.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
common/email.go (1)

46-49: Security risk: TLS certificate verification is disabled.

InsecureSkipVerify: true disables certificate validation, making SMTP connections vulnerable to man-in-the-middle attacks. This could expose credentials (SMTPToken) and email content.

Consider applying this diff to enable proper certificate verification:

 		tlsConfig := &tls.Config{
-			InsecureSkipVerify: true,
 			ServerName:         SMTPServer,
 		}

If certificate verification must be optional, make it configurable via an environment variable or configuration setting rather than hardcoding it to true.

🧹 Nitpick comments (1)
common/email.go (1)

13-20: Optional: eliminate duplicate string split operation.

strings.Split(SMTPFrom, "@") is called twice—once on line 14 and again on line 18. Consider reusing the split variable.

Apply this diff to eliminate the duplication:

 func generateMessageID() (string, error) {
 	split := strings.Split(SMTPFrom, "@")
 	if len(split) < 2 {
 		return "", fmt.Errorf("invalid SMTP account")
 	}
-	domain := strings.Split(SMTPFrom, "@")[1]
+	domain := split[1]
 	return fmt.Sprintf("<%d.%s@%s>", time.Now().UnixNano(), GetRandomString(12), domain), nil
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4e69c98 and c992919.

📒 Files selected for processing (1)
  • common/email.go (1 hunks)
🔇 Additional comments (1)
common/email.go (1)

35-35: Good fix for email header compatibility.

Adding the space between the display name and angle-bracketed email address follows the more standard and readable form of RFC 5322-compliant email headers (Display Name email@address.com). While RFC 5322 allows optional whitespace here, the improved formatting should enhance compatibility with strict mail providers like Tencent Cloud and resolve the authentication issue reported in #1347.

@Calcium-Ion
Calcium-Ion merged commit 2ac6a5b into QuantumNous:main Dec 12, 2025
1 check passed
Ridter pushed a commit to Ridter/new-api that referenced this pull request Dec 13, 2025
ennnnny pushed a commit to ennnnny/new-api that referenced this pull request Mar 17, 2026
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.

无法使用腾讯邮箱推送服务

2 participants