Skip to content

Reject '+'-signed numeric character references in HTML paste - #1252

Open
Silvertree2010 wants to merge 1 commit into
storytold:mainfrom
Silvertree2010:fix/h1861-html-paste-decodes-numeric-entit
Open

Silvertree2010 wants to merge 1 commit into
storytold:mainfrom
Silvertree2010:fix/h1861-html-paste-decodes-numeric-entit

Conversation

@Silvertree2010

@Silvertree2010 Silvertree2010 commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Found while testing crates/engine/src/cmd/richclip.rs:301-304 (numeric_char, used by decode_text) (no existing issue): HTML paste decodes numeric entities with a '+' sign (&#+65; becomes 'A').

Cause

numeric_char parsed entity digits with str::parse::<u32>() and u32::from_str_radix(.., 16). Both accept a leading '+', so the malformed references &#+65; and &#x+41; were decoded as 'A' instead of being kept as typed.

Fix

numeric_char now returns None unless the digits are non-empty and every character is a valid digit for the radix (decimal, or hex after the x/X prefix). Malformed references then fall through to the existing keep-as-typed path in decode_text. The new test covers both '+' cases and checks that &#65; and &#x41; still decode to 'A'.

Verified

  • Regression test cmd::richclip::tests::signed_numeric_entities_are_kept_as_typed: fails on main without the fix and passes with it (cargo test -p deckcraft-engine --lib signed_numeric_entities_are_kept_as_typed).
  • cargo xtask ci passes except tests::east_asian_vertical_text_stands_ideographs_upright, which also fails on main without this change.

Not changed

The similar '+' issue in Rgba::from_hex is a separate parser and is not changed here.

🤖 Generated with AI by ApolloOffice, see the Co-Authored-By trailers for the models

Summary by CodeRabbit

  • Bug Fixes
    • Numeric character references with empty or invalid digits, including leading plus signs, are now kept as literal text instead of being decoded. Valid decimal and hexadecimal references continue to decode.

Found while testing crates/engine/src/cmd/richclip.rs:301-304 (numeric_char, used by decode_text)

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between 9aef21e and fc914a0.

📒 Files selected for processing: 1
  • crates/engine/src/cmd/richclip.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

📝 Walkthrough

The numeric character-reference parser now checks decimal and hexadecimal digits before parsing. A test covers signed references and valid decimal and hexadecimal references.

Changes: Numeric character references

Layer / File(s) Summary
Validate and test numeric references
crates/engine/src/cmd/richclip.rs
numeric_char rejects empty bodies and characters that are invalid for the selected radix. Tests check that signed references remain unchanged and valid decimal and hexadecimal references decode.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: nikitkara, fredapps

Merge Risk: ⚪ Minimal · up to fc914

No actionable merge-blocking risk is established.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: rejecting plus-signed numeric character references during HTML paste decoding.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant