fix: resolve 4 bugs in termui - #3622
Conversation
github-actions
Bot
added
area:examples
📝 WalkthroughWalkthroughThe PR updates parsing behavior in three examples and adds rejection logging to the form Promise workflow. ChangesInput parsing corrections
Form error logging
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.6)packages/ui/src/Form.tsFile contains syntax errors that prevent linting: Line 142: Expected a statement but instead found '.catch(err => console.error("Promise.all failed:", err))'. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/rss-reader/src/index.tsx`:
- Around line 28-31: Update the entity-decoding callback to reject hexadecimal
entities containing more than eight digits before parsing, and require parsed
numeric values to be within 0x000000–0x10FFFF before calling
String.fromCodePoint. Apply the same upper-bound validation to decimal numeric
entities, returning the original match for invalid or out-of-range values so RSS
parsing continues safely.
In `@packages/ui/src/Form.ts`:
- Around line 141-142: Fix the Promise.all handling in Form by attaching the
rejection handler directly to the Promise.all expression, or replace it with a
try/catch around the existing await and rethrow after logging. Remove the
standalone .catch expression so packages/ui/src/Form.ts parses and compiles
successfully.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: eae2489b-0900-4d26-823b-4c1315302bc8
📒 Files selected for processing (4)
examples/chat-app/src/index.tsxexamples/rss-reader/src/index.tsxexamples/widget-gallery/src/index.tspackages/ui/src/Form.ts
| return value.replace(/&(#x?[0-9a-fA-F]+|[a-zA-Z]+);/g, (match, entity: string) => { | ||
| if (entity.startsWith('#x')) { | ||
| const codePoint = Number.parseInt(entity.slice(2), 16); | ||
| const codePoint = Number.parseInt(entity.slice(2, 10), 16); | ||
| return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : match; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does JavaScript String.fromCodePoint throw RangeError for values greater than 0x10FFFF?
💡 Result:
Yes, the JavaScript String.fromCodePoint method throws a RangeError if any of the provided arguments are greater than 0x10FFFF [1][2]. According to the ECMAScript specification, String.fromCodePoint iterates through its arguments and performs a check to ensure each is a valid Unicode code point [3][4]. A value is considered invalid if it is not an integral number, is less than 0, or is greater than 0x10FFFF [3][4]. If any of these conditions are met, the method throws a RangeError exception [1][3]. This behavior is consistent across all modern JavaScript environments as defined by the ECMAScript standard [3][5][6][4].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/fromCodePoint
- 2: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Not_a_valid_code_point
- 3: https://tc39.es/ecma262/2025/multipage/text-processing.html
- 4: https://tc39.es/ecma262/2026/multipage/text-processing.html
- 5: https://tc39.es/ecma262/2024/multipage/text-processing.html
- 6: https://tc39.es/ecma262/2022/multipage/text-processing.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the referenced file and decoding implementation.
if [ -f examples/rss-reader/src/index.tsx ]; then
wc -l examples/rss-reader/src/index.tsx
sed -n '1,80p' examples/rss-reader/src/index.tsx | cat -n
else
echo "File examples/rss-reader/src/index.tsx not found"
fd -i 'index\.tsx$' . | sed -n '1,40p'
fi
# Find any related tests or usages that may affect severity.
printf '\nSearch for decodeEntity/replace/unescape references:\n'
rg -n "decodeEntity|unescape|&`#x`?|`#x`|[0-9a-fA-F]+;|String\.fromCodePoint|parseInt" examples/rss-reader/src examples/rss-reader 2>/dev/null || true
# Read-only behavioral probe matching the relevant parser slice semantics.
node - <<'JS'
function decodeCurrent(value) {
return value.replace(/&(`#x`?[0-9a-fA-F]+|[a-zA-Z]+);/g, (match, entity) => {
if (entity.startsWith('`#x`')) {
const codePoint = Number.parseInt(entity.slice(2, 10), 16);
return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : match;
}
return match;
});
}
const inputs = [
'&',
'&`#x61`;',
'&`#x10ffff`;',
'&`#x110000`;',
'&`#x110000ffffff`;',
'&`#110000`;',
];
for (const input of inputs) {
try {
console.log(JSON.stringify(input), '=>', JSON.stringify(decodeCurrent(input)));
} catch (error) {
console.log(JSON.stringify(input), 'THREW', error.name + ': ' + error.message);
}
}
JSRepository: Karanjot786/TermUI
Length of output: 5558
Reject overlong and invalid hexadecimal entities.
entity.slice(2, 10) can turn longer malformed entities into valid values or pass values above 0x10ffff to String.fromCodePoint, which throws RangeError and stops RSS parsing. Skip entities with more than eight hexadecimal digits before parsing, validate codePoint <= 0x10ffff before conversion, and apply the same range check to decimal numeric entities.
Proposed fix
if (entity.startsWith('`#x`')) {
- const codePoint = Number.parseInt(entity.slice(2, 10), 16);
- return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : match;
+ const digits = entity.slice(2);
+ if (digits.length > 8) return match;
+ const codePoint = Number.parseInt(digits, 16);
+ return codePoint <= 0x10ffff ? String.fromCodePoint(codePoint) : match;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/rss-reader/src/index.tsx` around lines 28 - 31, Update the
entity-decoding callback to reject hexadecimal entities containing more than
eight digits before parsing, and require parsed numeric values to be within
0x000000–0x10FFFF before calling String.fromCodePoint. Apply the same
upper-bound validation to decimal numeric entities, returning the original match
for invalid or out-of-range values so RSS parsing continues safely.
|
|
||
| .catch(err => console.error("Promise.all failed:", err)); No newline at end of file |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Attach the rejection handler to the Promise expression.
Line 142 is a standalone .catch(...) expression. Biome reports a parse error, so packages/ui/src/Form.ts cannot compile. Attach .catch(...) directly to Promise.all(...), or use try/catch and rethrow after logging.
try {
const results = await Promise.all(existingPromises);
// Existing result handling.
} catch (err) {
console.error("Promise.all failed:", err);
throw err;
}🧰 Tools
🪛 Biome (2.5.6)
[error] 142-142: Expected a statement but instead found '.catch(err => console.error("Promise.all failed:", err))'.
(parse)
🪛 GitHub Actions: CI / 0_build-and-test.txt
[error] 142-142: @termuijs/ui build failed during tsup/esbuild: unexpected '.' at the standalone '.catch(err => console.error("Promise.all failed:", err));'. TypeScript also reported TS1128, TS1005, and TS2304 syntax errors. Command: bun run build.
🪛 GitHub Actions: CI / build-and-test
[error] 142-142: The @termuijs/ui build failed during tsup/esbuild: unexpected '.' in .catch(err => console.error("Promise.all failed:", err));. TypeScript also reported syntax errors (TS1128, TS1005, TS2304). Command: bun run build.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ui/src/Form.ts` around lines 141 - 142, Fix the Promise.all handling
in Form by attaching the rejection handler directly to the Promise.all
expression, or replace it with a try/catch around the existing await and rethrow
after logging. Remove the standalone .catch expression so
packages/ui/src/Form.ts parses and compiles successfully.
Source: Linters/SAST tools
1 participant
Description
This PR fixes real bugs found in the codebase:
parseInt: without10, strings like'0x1F'or'08'parse in unintended bases.trim()to''misses whitespace-only input;.trim().length === 0is explicit.Promise.all: an unhandled rejection in any input promise previously crashed silently.parseInt: without10, strings like'0x1F'or'08'parse in unintended bases.Type of Change
How Has This Been Tested?
Checklist
Related Issue
Ref: #3621
Summary by CodeRabbit