Skip to content

Document SMTP FromHeader manual usage in package godoc#219

Merged
veverkap merged 2 commits into
mainfrom
copilot/add-helper-for-smtp-fromheader
May 7, 2026
Merged

Document SMTP FromHeader manual usage in package godoc#219
veverkap merged 2 commits into
mainfrom
copilot/add-helper-for-smtp-fromheader

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 7, 2026

smtp.Params.FromHeader is easy to misread as something smtp.Send will apply automatically. In practice, Send only uses Params.From for the SMTP envelope; callers must embed the RFC 5322 From: header themselves in msg.

  • Clarify package contract

    • Add a package-level warning that Send does not construct or rewrite message headers.
    • Call out the split between SMTP envelope sender (Params.From) and RFC 5322 header sender (Params.FromHeader).
  • Make Params.FromHeader harder to misuse

    • Expand the field godoc to state that Validate() formats FromHeader, but Send does not copy it into the message.
    • Explicitly tell callers to write From: + Params.FromHeader into msg.
  • Document Send behavior where callers will look first

    • Update Send godoc to state that it uses Params.From only for MAIL FROM.
    • Note that From, and any other RFC 5322 headers, remain the caller’s responsibility.

Example:

params, err := cfg.Validate()
if err != nil {
	return err
}

msg := []byte(
	"From: " + params.FromHeader + "\r\n" +
	"To: user@example.com\r\n" +
	"Subject: Welcome\r\n" +
	"\r\n" +
	"Hello\r\n",
)

err = smtp.Send(ctx, params, "user@example.com", msg)

Greptile Summary

This is a documentation-only PR that clarifies the contract between smtp.Send and Params.FromHeader. The existing code is unchanged; only godoc comments are updated to prevent callers from assuming Send writes RFC 5322 headers automatically.

  • Adds a package-level WARNING explaining that Send only sets the SMTP envelope sender via MAIL FROM and does not inject any RFC 5322 headers into msg.
  • Expands the Params.FromHeader field doc and the Send function doc to repeat the same contract and show the explicit string concatenation callers must perform.

Confidence Score: 5/5

Documentation-only change with no modifications to runtime behavior — safe to merge.

No executable code was changed. The three updated godoc blocks accurately describe the existing behavior: Send uses only Params.From for MAIL FROM and passes msg to the server verbatim, exactly as the implementation shows. The added guidance is consistent across the package doc, field doc, and function doc.

No files require special attention.

Important Files Changed

Filename Overview
smtp/smtp.go Documentation-only change; no functional code was modified. Three godoc sites updated with consistent contract warnings about envelope vs. RFC 5322 header responsibilities.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant Validate as cfg.Validate()
    participant Send as smtp.Send()
    participant SMTP as SMTP Server

    Caller->>Validate: "Config{Host, Port, From, ...}"
    Validate-->>Caller: "Params{From (bare email), FromHeader (RFC 5322), ...}"

    Note over Caller: Caller must build msg with<br/>"From: "+params.FromHeader+"\r\n"<br/>and all other RFC 5322 headers

    Caller->>Send: Send(ctx, params, to, msg)
    Send->>SMTP: MAIL FROM: params.From (envelope only)
    Send->>SMTP: RCPT TO: to
    Send->>SMTP: DATA: msg (as-is, headers untouched)
    SMTP-->>Send: 250 OK
    Send-->>Caller: nil / error
Loading

Reviews (2): Last reviewed commit: "docs(smtp): use Go doc-link syntax and i..." | Re-trigger Greptile

Agent-Logs-Url: https://github.com/amalgamated-tools/goauth/sessions/99ea3150-fa82-4f5c-886b-44f45e694f18

Co-authored-by: veverkap <22348+veverkap@users.noreply.github.com>
Comment thread smtp/smtp.go Outdated
Comment thread smtp/smtp.go Outdated
Copy link
Copy Markdown
Contributor

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 clarifies the smtp package contract around sender identity by documenting that Send only uses Params.From for the SMTP envelope sender (MAIL FROM) and does not construct or apply RFC 5322 headers like From:.

Changes:

  • Add a package-level warning explaining the envelope-vs-header split (Params.From vs Params.FromHeader).
  • Expand Params.FromHeader godoc to warn that callers must add the From: header themselves.
  • Update Send godoc to explicitly state it does not add/rewrite RFC 5322 headers.
Show a summary per file
File Description
smtp/smtp.go Adds/updates godoc warnings to prevent misuse of Params.FromHeader and clarify Send header responsibilities.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (1)

smtp/smtp.go:124

  • Similar to the package warning: "embed Params.FromHeader in msg" is ambiguous because FromHeader does not include the From: field name. Reword this godoc to explicitly say callers must include a From: header line (typically "From: "+params.FromHeader+"\r\n") in msg themselves.
// Send uses Params.From only for the SMTP envelope sender (MAIL FROM). It does
// not add or rewrite RFC 5322 message headers, including From, so callers must
// embed Params.FromHeader in msg themselves.
  • Files reviewed: 1/1 changed files
  • Comments generated: 1

Comment thread smtp/smtp.go Outdated
…mples

Apply Go 1.19+ doc-link syntax ([Params.From], [Params.FromHeader], [Send])
so pkg.go.dev renders clickable cross-references. Also clarify all three
warning comments to show the full header line with "\r\n" terminator, making
it unambiguous that callers must write "From: "+Params.FromHeader+"\r\n".
@veverkap veverkap merged commit bb3315c into main May 7, 2026
8 checks passed
@veverkap veverkap deleted the copilot/add-helper-for-smtp-fromheader branch May 7, 2026 21:39
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