fix(gmail): match message header names case-insensitively (#642)#869
fix(gmail): match message header names case-insensitively (#642)#869shigechika wants to merge 1 commit into
Conversation
parse_message_headers used exact-case string matching, so headers whose field names use non-canonical casing -- e.g. "CC" (common from Exchange/Outlook) or a lowercase "from" from some MTAs -- fell through and were silently dropped. This dropped CC recipients from +reply-all. Per RFC 5322 1.2.2 header field names are case-insensitive, and the sibling get_part_header already uses eq_ignore_ascii_case. Normalize the name to lowercase before matching, and add a regression test covering mixed-case FROM/to/CC/Reply-TO/subject/MESSAGE-ID/References. Fixes googleworkspace#642. Claude-Session: https://claude.ai/code/session_01EuRYoXhx7ozQWx19mAFLUS
🦋 Changeset detectedLatest commit: 78e0ae1 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 |
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 addresses an issue where email headers with non-canonical casing were being silently dropped, causing problems with features like reply-all. By normalizing header names to lowercase, the implementation now correctly handles headers regardless of their original casing, as per RFC 5322 standards. Highlights
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 the 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 counterproductive. 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request fixes an issue where Gmail message headers were matched case-sensitively, causing headers with non-canonical casing (such as uppercase "CC") to be silently dropped. The fix normalizes header names to lowercase before matching and adds a corresponding regression test. The review feedback points out that using to_ascii_lowercase() inside the loop allocates a new String on the heap for every header parsed, and suggests using eq_ignore_ascii_case in an if/else if chain to avoid these allocations.
| match name.to_ascii_lowercase().as_str() { | ||
| "from" => parsed.from = value.to_string(), | ||
| "reply-to" => append_address_list_header_value(&mut parsed.reply_to, value), | ||
| "to" => append_address_list_header_value(&mut parsed.to, value), | ||
| "cc" => append_address_list_header_value(&mut parsed.cc, value), | ||
| "subject" => parsed.subject = value.to_string(), | ||
| "date" => parsed.date = value.to_string(), | ||
| "message-id" => parsed.message_id = value.to_string(), | ||
| "references" => append_header_value(&mut parsed.references, value), | ||
| _ => {} | ||
| } |
There was a problem hiding this comment.
Using name.to_ascii_lowercase() inside the loop allocates a new String on the heap for every single header parsed. This can be avoided entirely by using eq_ignore_ascii_case in an if/else if chain, which performs case-insensitive comparison without any allocations.
if name.eq_ignore_ascii_case("From") {
parsed.from = value.to_string();
} else if name.eq_ignore_ascii_case("Reply-To") {
append_address_list_header_value(&mut parsed.reply_to, value);
} else if name.eq_ignore_ascii_case("To") {
append_address_list_header_value(&mut parsed.to, value);
} else if name.eq_ignore_ascii_case("Cc") {
append_address_list_header_value(&mut parsed.cc, value);
} else if name.eq_ignore_ascii_case("Subject") {
parsed.subject = value.to_string();
} else if name.eq_ignore_ascii_case("Date") {
parsed.date = value.to_string();
} else if name.eq_ignore_ascii_case("Message-ID") {
parsed.message_id = value.to_string();
} else if name.eq_ignore_ascii_case("References") {
append_header_value(&mut parsed.references, value);
}
Re-opening #847, which the stale bot auto-closed after 72h of inactivity before a maintainer could review it. No code changes since #847 — same fix, still needed (upstream/main still has the case-sensitive match as of this comment).
Summary
parse_message_headersmatched header names with exact-case string comparison, so headers whose field names use non-canonical casing — e.g."CC"(common from Exchange/Outlook) or a lowercase"from"from some MTAs — fell through and were silently dropped. This dropped CC recipients from+reply-all. Per RFC 5322 §1.2.2 header field names are case-insensitive (the siblingget_part_headeralready useseq_ignore_ascii_case). Normalize the name to lowercase before matching.Fixes #642.