fix: escape JavaScript line terminators in generated docs - #489
Conversation
|
Warning Review limit reached
Next review available in: 11 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
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 |
|
ⓘ Your Qodo trial ends soon. Ask your workspace admin to set up billing to keep reviews running after the trial. Manage billing |
PR Summary by QodoEscape JS line terminators in escJsAttr for generated HTML docs
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #489 +/- ##
=======================================
Coverage 93.88% 93.88%
=======================================
Files 173 173
Lines 14715 14715
=======================================
Hits 13815 13815
Misses 900 900 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| function escJs(str) { | ||
| // JSON.stringify is the JavaScript string-literal serializer. It handles | ||
| // quotes, backslashes, and ordinary line terminators; escape the two | ||
| // legacy Unicode line separators that it leaves literal. | ||
| if (!str) return '""'; | ||
| return JSON.stringify(String(str)) | ||
| .replace(/\\u2028/g, '\\\\u2028') | ||
| .replace(/\\u2029/g, '\\\\u2029'); | ||
| } | ||
|
|
||
| function escJsAttr(str) { | ||
| // A JS string literal nested inside an HTML attribute — two contexts, so | ||
| // two escapes in that order. innerHTML decodes the entities before the | ||
| // handler compiles, so the JS escapes must survive that decode: \' stays | ||
| // \', while " arrives as a plain quote which cannot close a | ||
| // single-quoted JS string. | ||
| return escAttr(String(str).replace(/\\\\/g, '\\\\\\\\').replace(/'/g, "\\\\'")); | ||
| // escJs returns a complete double-quoted JS string literal. HTML-escape | ||
| // that literal so it can be placed inside a double-quoted attribute; | ||
| // innerHTML decodes the entities before the inline handler compiles. | ||
| return escAttr(escJs(str)); | ||
| } | ||
| // END_ESCAPERS |
There was a problem hiding this comment.
Why this extra layer of indirection lol
![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: 03ea660df2
ℹ️ 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".
| // quotes, backslashes, and ordinary line terminators; escape the two | ||
| // legacy Unicode line separators that it leaves literal. Then HTML-escape | ||
| // the complete literal for the double-quoted attribute. | ||
| var literal = !str ? '""' : JSON.stringify(String(str)); |
There was a problem hiding this comment.
When an inline-handler value is numeric 0 or boolean false, this truthiness check serializes it as "" instead of preserving String(str) as the previous helper did. The docs model deliberately supports falsy manifest-derived names such as PluginDoc(name=0), so the generated plugin card now calls navigateTo("") and returns home rather than opening the plugin named 0; serialize all non-null values directly instead of using a truthiness test.
Useful? React with 👍 / 👎.
1 participant
Summary
Generated documentation builds inline event handlers with JavaScript string values nested inside HTML attributes. The old helper manually escaped a single-quoted JavaScript literal and omitted ECMAScript line terminators, which could leave invalid handlers in generated pages.
Use
JSON.stringifyas the JavaScript string-literal serializer, escape U+2028 and U+2029 for compatibility, then HTML-escape the resulting double-quoted literal. Update all inline-handler call sites to use the complete serialized literal.Fixes #488
Validation
.venv/bin/pytest tests/codex/test_docs_output_safety.py -q— 65 passedmake test— 3903 passed, 22 warningsmake lint— passedmake update— passed with no generated changesgit diff --check— passed