Repository navigation
fix: join adjacent MIME encoded words before decoding multibyte text - #182
charan-rathore wants to merge 1 commit into
Conversation
Signed-off-by: Charan Rathore <180254320+charan-rathore@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b24bc9b75
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "latin1", | ||
| ); | ||
| if (pending !== null && normalized === pendingCharset) { | ||
| pending = Buffer.concat([pending, bytes]); |
There was a problem hiding this comment.
When an incoming Gmail Subject or From contains thousands of adjacent same-charset encoded words, this loop copies the entire accumulated prefix for every word, making decodeHeader quadratic in header size. The mapMessage path accepts unrestricted header strings up to the whole-response limit, so a sufficiently large folded header can cause seconds of CPU time and heavy allocation churn whenever the message is listed or opened; collect the byte chunks and call Buffer.concat once in flush() instead.
Useful? React with 👍 / 👎.
jerelvelarde
left a comment
There was a problem hiding this comment.
Reviewed 8b24bc9b753227cf2fc875225be7475f51dbe66d.
Recovering malformed split UTF-8 headers may be useful, but joining every same-charset word changes correctly independent decoding and introduces quadratic allocation. RFC 2047 section 5 requires complete characters within each encoded word; section 6.2 removes intervening display whitespace, not decoder boundaries (https://www.rfc-editor.org/rfc/rfc2047.html#section-5). Reframe any split-byte repair as optional tolerance, not a standards requirement. Security: the earlier quadratic-copy finding remains actionable on untrusted header strings; no new credential or write path is involved. Verification: 37 existing Google tests passed; an added independent-word regression fails on this head and passes on current main. No live Gmail account was used.
Local execution used read-only, network-disabled Linux containers with synthetic data and Node 24.17.0. Root dependency fixtures came from main’s manifest rather than the lockfile; exact-head GitHub CI covers locked full checks where green. No production credentials were supplied.
| "latin1", | ||
| ); | ||
| if (pending !== null && normalized === pendingCharset) { | ||
| pending = Buffer.concat([pending, bytes]); |
There was a problem hiding this comment.
[P2] Preserve independent encoded-word decoding. With Subject =?UTF-16?B?//5BAA==?= =?UTF-16?B?//5CAA==?=, the existing decoder produces AB; this implementation produces A\uFEFFB because the second word’s BOM is no longer handled at a decoder boundary. Reproduced through GoogleClient.getThread on this head and current main. The same issue affects independently BOM-prefixed UTF-8 words. Keep normal per-word decoding, and if repairing malformed split UTF-8 is desired, scope that fallback so it cannot change valid independent words.
| "latin1", | ||
| ); | ||
| if (pending !== null && normalized === pendingCharset) { | ||
| pending = Buffer.concat([pending, bytes]); |
There was a problem hiding this comment.
[P2] Avoid repeatedly copying the accumulated header. The existing review’s resource-exhaustion finding remains: Buffer.concat([pending, bytes]) recopies all prior bytes per word, making a long same-charset run quadratic. Header values have no individual size limit in messageSchema, only the much larger overall JSON-response cap. Preserve linear work with chunk collection and a single concatenate per bounded group, or a streaming decoder, and add a large-header regression.
Summary
A sender that wraps a long header at the RFC 2047 encoded-word length limit can break a multibyte character across two adjacent encoded words.
decodeHeaderglued the words' whitespace together (#130) but still decoded each word on its own, so the split bytes each became U+FFFD:=?UTF-8?B?...w6?==?UTF-8?B?qQ==?=rendered ascaf��instead ofcafé.Fix
Per RFC 2047 §6.2, adjacent encoded words are concatenated before decoding. The decoder now matches a run of adjacent words, joins the raw bytes of consecutive same-charset words, and decodes each joined group once. Unknown charsets keep the existing fallback of printing the original words verbatim, and words separated by plain text still decode independently.
Test
mail joins adjacent encoded words that split a multibyte charactercovers base64 and quoted-printable splits, including a fold-separated pair; it fails on main with replacement characters and passes with the join. The fulltests/google.test.tssuite passes (37/37), andtsc --noEmitandbiome checkare clean.