Skip to content

Escape control characters in Strings.escapeJSON#6344

Merged
ernestognw merged 8 commits intoOpenZeppelin:masterfrom
ernestognw:audit/n-03
Feb 12, 2026
Merged

Escape control characters in Strings.escapeJSON#6344
ernestognw merged 8 commits intoOpenZeppelin:masterfrom
ernestognw:audit/n-03

Conversation

@ernestognw
Copy link
Member

Fixes #????

PR Checklist

  • Tests
  • Documentation
  • Changeset entry (run npx changeset add)

@ernestognw ernestognw requested a review from a team as a code owner February 11, 2026 03:56
@openzeppelin-security-agent
Copy link

The latest updates on your security scan. Learn more about OpenZeppelin Platform.

Project Scan Issues Details Updated
Openzeppelin Contracts 🟡 Queued View Feb 11, 2026, 3:56 AM (UTC)

@changeset-bot
Copy link

changeset-bot bot commented Feb 11, 2026

⚠️ No Changeset found

Latest commit: edc2e03

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@ernestognw ernestognw added this to the 5.6 milestone Feb 11, 2026
@ernestognw ernestognw changed the title Escape controll characters in Strings.escapeJSON Escape control characters in Strings.escapeJSON Feb 11, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 11, 2026

Walkthrough

The changes update the Strings.escapeJSON function to extend control character escaping to cover the full range U+0000 to U+001F per RFC-4627. Previously, only seven specific control characters were escaped. The implementation removes a bitmask-based lookup constant and replaces it with explicit control character detection that outputs Unicode escape sequences (\u00XX) for control characters. The test suite is expanded to verify escaping behavior for all ASCII control characters in the range U+0000 to U+001F. A breaking change entry is added to the changelog documenting the extended escaping behavior.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description is a template with placeholder content and unchecked items, but it does reference the PR's purpose of escaping control characters in the context of the PR title and objectives. Fill in the placeholder issue number and provide a detailed description of what changes were made, why they were necessary, and any testing or documentation updates completed.
✅ Passed checks (2 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title 'Escape controll characters in Strings.escapeJSON' directly describes the main change—expanding control character escaping in the escapeJSON function—and accurately reflects the primary objective of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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


bytes16 private constant HEX_DIGITS = "0123456789abcdef";
uint8 private constant ADDRESS_LENGTH = 20;
uint256 private constant SPECIAL_CHARS_LOOKUP =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not keep that lookup table, and extend it to cover all control chars?

uint256 private constant SPECIAL_CHARS_LOOKUP =
        (0xffffffff) | // first 32 bytes corresponding to the control characters
        (1 << 0x22) | // double quote
        (1 << 0x5c); // backslash

(1 << 0x0a) | // newline
(1 << 0x0c) | // form feed
(1 << 0x0d) | // carriage return
0xffffffff | // first 32 bytes corresponding to the control characters
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
0xffffffff | // first 32 bytes corresponding to the control characters
0xffffffff | // first 4 bytes corresponding to the control characters

or

Suggested change
0xffffffff | // first 32 bytes corresponding to the control characters
0xffffffff | // first 32 bits corresponding to the control characters

Copy link
Member Author

@ernestognw ernestognw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, can't approve my own PR

Amxx
Amxx previously approved these changes Feb 12, 2026
@ernestognw ernestognw merged commit 361cecc into OpenZeppelin:master Feb 12, 2026
17 of 18 checks passed
Amxx added a commit that referenced this pull request Feb 12, 2026
Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
Signed-off-by: Hadrien Croubois <hadrien.croubois@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants