refactor(gmail): replace hand-rolled email construction with mail-builder#491
refactor(gmail): replace hand-rolled email construction with mail-builder#491malob wants to merge 2 commits intogoogleworkspace:mainfrom
Conversation
…lder Replace custom MessageBuilder, RFC 2047 encoding, header sanitization, and address encoding (including googleworkspace#482) with the mail-builder crate (Stalwart Labs, 0 runtime deps). Each command builds a mail_builder::MessageBuilder directly. Introduce structured types throughout: - Mailbox type (parsed display name + email) replaces raw string passing - sanitize_control_chars strips ASCII control characters (CRLF, null, tab, etc.) at the parse boundary — defense-in-depth for mail-builder's structured header types, superseding sanitize_header_value, sanitize_component, and encode_address_header from googleworkspace#482 - OriginalMessage fields use Option<T> instead of empty-string sentinels - parse_original_message returns Result with validation (threadId, From, Message-ID) - Pre-parsed Config types (SendConfig, ForwardConfig, ReplyConfig) with Vec<Mailbox> — parse at the boundary, not downstream - parse_forward_args and parse_send_args return Result with --to validation, consistent with parse_reply_args - parse_optional_mailboxes helper normalizes Some(vec![]) to None for optional address fields (--cc, --bcc, --from) - Envelope types borrow from Config + OriginalMessage with lifetimes - Message IDs stored bare (no angle brackets), parsed once at boundary - References stored as Vec<String> instead of space-separated string - ThreadingHeaders bundles In-Reply-To + References with debug_assert for bare-ID convention - Shared CLI arg builders (common_mail_args, common_reply_args) eliminate duplicated --cc/--bcc/--html/--dry-run definitions Additional improvements: - finalize_message returns Result instead of panicking via .expect() - Mailbox::parse_list filters empty-email entries (trailing comma edge case) - format_email_link percent-encodes mailto hrefs to prevent parameter injection - Forward date handling: omits Date line when absent instead of showing empty "Date: " - Dry-run auth: log skipped auth as diagnostic instead of silently discarding errors - Restore --html tips in after_help strings (gmail_quote CSS, cid: image warnings, HTML fragment advice) lost in release PR googleworkspace#434 - Update execute_method call for upload_content_type parameter (googleworkspace#429) Delete: MessageBuilder, encode_header_value, sanitize_header_value, encode_address_header, sanitize_component, extract_email, extract_display_name, split_mailbox_list, build_references.
Consistent with +reply, +reply-all, and +forward which already support --from. Uses the same parse_optional_mailboxes path and apply_optional_headers plumbing.
🦋 Changeset detectedLatest commit: e0cd622 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
googleworkspace-bot
added
area: distribution
area: core
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the email construction process within the Gmail helpers by integrating the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is an excellent and comprehensive refactoring that significantly improves the Gmail helper functionality. Replacing the manual email construction with the mail-builder crate is a major step forward for correctness, maintainability, and security. The introduction of the Mailbox struct for typed address handling, along with the consistent use of Result for error propagation, makes the code much more robust and easier to follow. The attention to security, including the addition of control character sanitization and tests for header injection, is commendable. The new --from flag on the +send command is also a great addition for consistency. Overall, this is a very high-quality pull request.
2 participants
Description
Replace hand-rolled MessageBuilder, RFC 2047 encoding, and header sanitization with the
mail-buildercrate (Stalwart Labs, 24KB, 1 optional runtime dep), and add--fromflag to+sendfor send-as alias consistency with+reply,+reply-all, and+forward.Why mail-builder
The Gmail helpers have grown from simple text emails to supporting HTML mode, CC/BCC, reply-all with recipient dedup, forwarding with threading, and send-as aliases. Each feature added more hand-rolled RFC 5322 logic: MIME headers, content-type selection, RFC 2047 encoding for non-ASCII names (#482), header injection prevention, and address parsing/formatting.
Adding attachment support (#247) would require multipart/mixed MIME construction — boundaries, part encoding, content-disposition headers — on top of an already complex custom implementation.
mail-buildergives us correct RFC 2047 encoding, Content-Transfer-Encoding, and MIME structure for free, plus a clean path to--attachvia native multipart support.Commit 1 — Refactor
Types:
Mailbox(parsed display name + email) replaces raw string passing.OriginalMessagefields useOption<T>instead of empty-string sentinels. Config types useVec<Mailbox>. Message IDs stored bare (no angle brackets), parsed once at the boundary.Message construction: Each command builds a
mail_builder::MessageBuilderdirectly via shared helpers (apply_optional_headers,set_threading_headers,finalize_message). The pattern is consistent across all three commands.Security:
sanitize_control_charsinMailbox::parsestrips ASCII control characters (CRLF, null, tab) at the parse boundary. This supersedessanitize_header_value,sanitize_component, andencode_address_headerfrom #482 — mail-builder's structured address types prevent header injection structurally, and parse-boundary sanitization provides defense-in-depth. End-to-end injection tests verify CRLF in--from/--cccannot create spurious headers.Behavioral changes:
parse_original_messagenow returnsResult— rejects messages missing required headers (threadId, From, Message-ID) instead of silently proceeding with empty fieldsCommit 2 —
--fromon+sendAdds the
--fromflag to+send, consistent with+reply,+reply-all, and+forward. Uses the sameparse_optional_mailboxespath andapply_optional_headersplumbing.Note on #482
This PR supersedes the RFC 2047 address header encoding merged in #482. mail-builder handles RFC 2047 automatically via structured
Addresstypes, andsanitize_control_charsinMailbox::parsestrips all ASCII control characters at the parse boundary (covering the same CRLF, null, and tab injection vectors assanitize_component/sanitize_header_value). One behavioral difference: #482'sencode_address_headeralso truncated bare emails at the first non-email character as post-CRLF-stripping cleanup; we rely on mail-builder's angle-bracket wrapping and Gmail's API validation to reject malformed addresses instead of silently truncating. Seetest_mailbox_parse_strips_*andtest_send_crlf_injection_*.Note on #395
The open attachments PR hand-rolls multipart MIME on the old MessageBuilder. This refactor provides a cleaner foundation for that feature using mail-builder's native multipart support.
Checklist:
AGENTS.mdguidelines (no generatedgoogle-*crates).cargo fmt --allto format the code perfectly.cargo clippy -- -D warningsand resolved all warnings.pnpx changeset) to document my changes.