Skip to content

harden JsonEscaper by removing sprintf and clarifying byte handling#44474

Open
rootvector2 wants to merge 1 commit into
envoyproxy:mainfrom
rootvector2:json-escaper-hardening
Open

harden JsonEscaper by removing sprintf and clarifying byte handling#44474
rootvector2 wants to merge 1 commit into
envoyproxy:mainfrom
rootvector2:json-escaper-hardening

Conversation

@rootvector2
Copy link
Copy Markdown

Additional Description:

  • Replaces C-style sprintf writes in JSON escaping with explicit fixed-position hex encoding for control bytes.
  • Uses unsigned-byte comparison for control character detection to avoid signed char ambiguity on non-ASCII input.
  • Preserves existing escaping behavior while reducing error-prone string write patterns.
  • Adds regression coverage for high-bit bytes (0x80, 0xFF) and mixed high-bit/control-byte input.
  • AI usage disclosure: GitHub Copilot assistance was used to draft this change and description; the author manually reviewed the code and tests.

Risk Level:
Low. The change is localized to JSON escaping utility logic and test coverage was expanded for edge cases.

Testing:

  • Added JsonEscapeTest coverage for high-bit byte pass-through.
  • Added JsonEscapeTest coverage for mixed high-bit and control-byte escaping.
  • Attempted targeted test: bazel test --config=clang //test/common/common:logger_test
  • Local run was blocked by environment setup (missing JDK and CC toolchain resolution). Full validation expected in CI.

Docs Changes:
None.

Release Notes:
None.

Platform Specific Features:
None.

[Optional Runtime guard:]
Not needed.

[Optional Fixes #Issue]
N/A

[Optional Fixes commit #PR or SHA]
N/A

[Optional Deprecated:]
N/A

@repokitteh-read-only
Copy link
Copy Markdown

Hi @rootvector2, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #44474 was opened by rootvector2.

see: more, trace.

@rootvector2 rootvector2 changed the title enhance JSON escape tests for high and mixed byte values harden JsonEscaper by removing sprintf and clarifying byte handling Apr 15, 2026
@phlax
Copy link
Copy Markdown
Member

phlax commented Apr 16, 2026

@rootvector2 please fix DCO

@rootvector2 rootvector2 force-pushed the json-escaper-hardening branch from 54a36fb to e3d0176 Compare April 16, 2026 17:25
@rootvector2 rootvector2 temporarily deployed to external-contributors April 16, 2026 17:25 — with GitHub Actions Inactive
@agrawroh
Copy link
Copy Markdown
Member

@rootvector2 Please fix the failing triggers.

/wait

Signed-off-by: rootvector2 <dxbnaveed.k@gmail.com>
@rootvector2 rootvector2 force-pushed the json-escaper-hardening branch from e3d0176 to 735104c Compare May 3, 2026 09:09
@rootvector2 rootvector2 requested a deployment to external-contributors May 3, 2026 09:09 — with GitHub Actions Waiting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants