Repository navigation
fix: keep surrogate numeric character references from failing the mail list - #183
charan-rathore wants to merge 1 commit into
Conversation
…l list 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: 9a70e5f345
ℹ️ 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".
| ? Number.parseInt(entity.slice(2), 16) | ||
| : Number.parseInt(entity.slice(1), 10); | ||
| return code >= 0 && code <= 0x10ffff ? String.fromCodePoint(code) : original; | ||
| // Surrogates are not scalar values: fromCodePoint throws on them, and one |
There was a problem hiding this comment.
On the supported Node runtime, String.fromCodePoint(0xd800) returns an unpaired UTF-16 code unit; it does not throw (only nonintegral or out-of-range values do). Consequently, before this patch the fixture reaches Promise.all successfully and the new test fails only at its changed body assertion, so this guard/test cannot demonstrate or fix the claimed whole-inbox rejection. If that rejection is the production regression, reproduce the operation that actually throws (or reframe this as output normalization) so the real failure is not left unresolved.
Useful? React with 👍 / 👎.
This branch has not been deployed
1 participant
Summary
decodeSnippetdecodes numeric character references withString.fromCodePoint, which throws aRangeErrorfor surrogate code points (0xD800-0xDFFF). The range check only excluded values above0x10FFFF. A sender can put the literal text�in a message - someone discussing HTML entities, for example - and when that message falls back to its snippet, the throw rejects thePromise.allinlistMail, so one message breaks the whole inbox list. The same unpaired-surrogate input class is already handled for attachment filenames and URI encoding (wellFormed); the snippet decoder missed it.Fix
Treat surrogate code points as invalid alongside the out-of-range ones, leaving the reference literal - the same fallback the decoder already uses for values it cannot represent.
Test
a numeric character reference to a surrogate stays literal instead of failing the listfails on main with the uncaughtRangeErrorand passes with the range narrowed. The fulltests/google.test.tssuite passes (38/38), andtsc --noEmitandbiome checkare clean.