Repository navigation
fix: preserve independent MIME words while tolerating split UTF-8 - #182
charan-rathore wants to merge 2 commits 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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
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
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.
charan-rathore
changed the title
Summary
Preserve independent decoding of valid MIME encoded words, including each word's BOM. Continue ignoring display whitespace between adjacent words.
Malformed split UTF-8 is optional tolerance, not an RFC 2047 requirement: collect only words that cannot decode independently, then concatenate each malformed group once. If that group is still invalid, retain per-word replacement behavior. Other charsets and valid following words keep their decoder boundaries. This removes the repeated accumulated-buffer copies identified in review.
Validation
0e94145bfe85908d6826fdf282afd07e7bdbe71cbefore editing.No live Gmail account, provider write, full workspace test run, mobile export or application build was used for this review fix. Those paths remain for CI/review.