Skip to content

fix(common): preserve escaped backslashes when removing HTML escaping - #1290

Closed
deoxyszc wants to merge 1 commit into
a2aproject:mainfrom
deoxyszc:fix/preserve-literal-unicode-escapes
Closed

deoxyszc wants to merge 1 commit into
a2aproject:mainfrom
deoxyszc:fix/preserve-literal-unicode-escapes

Conversation

@deoxyszc

@deoxyszc deoxyszc commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Problem

HtmlEscapeUtils.removeHtmlEscaping() does not distinguish an active JSON
Unicode escape from the same characters preceded by an escaped backslash.
Its global replacements can turn valid protobuf JSON into invalid JSON.

For example, when a text or data part contains the literal text literal \u003e:

JsonFormat output: {"result":"literal \\u003e"}
Current output:   {"result":"literal \>"}

The second output cannot be parsed as JSON. The same issue affects the other
HTML escapes, including apostrophes, and can break both ordinary responses and
streaming artifact-update envelopes. It is not necessary for the caller to
supply malformed JSON: the input can be an ordinary protobuf string value.

Related implementation history: #947 and
#954. This PR does not reopen or close those issues/PRs.

Change

Replace the global substitutions with a single scan that consumes existing
escapes as pairs. Only active Unicode escapes for <, >, &, = and '
are decoded; escaped backslashes and other escape sequences remain unchanged.

The public API, supported characters, uppercase/lowercase handling and the
no-HTML-escape fast path are retained. There are no new dependencies and no
transport-specific workarounds.

Regression coverage

  • Each supported escape with one through six preceding backslashes, covering
    literal escape text and actual HTML characters preceded by backslashes.
  • Existing quote, newline, tab, unrelated Unicode and U+2028/U+2029 escapes.
  • Protobuf JSON round trips of string values and field names, with both default
    and whitespace-omitting printers.
  • A JSON-RPC StreamResponse artifact update carrying a DataPart-style tool
    result, parsed back to the original protobuf message.
  • The v0.3 protobuf serialization wrapper, which shares the utility.

The new utility, protobuf round-trip and streaming-envelope regressions fail
against the original implementation. Existing HTML-unescaping tests continue
to pass with the fix.

Validation

On Java 21, the complete affected-module test/package build passed:

mvn -B -q \
  -pl transport/jsonrpc,transport/rest,client/transport/rest,compat-0.3/transport/jsonrpc,compat-0.3/transport/rest,compat-0.3/client/transport/rest \
  -am package

Surefire reports across the selected modules and reactor dependencies:
1,983 tests, 0 failures, 0 errors, 6 skipped (1,977 passed).
This is not a claim that the full repository integration/TCK matrix was run.

Only decode active Unicode escapes so literal escape text remains valid JSON. Cover backslash parity and protobuf/JSON-RPC streaming round trips, including v0.3 compatibility.

Co-authored-by: Copilot <[email protected]>
@kabir

kabir commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator

Hi @deoxyszc thank you for the PR. It seems it conflicts with some recently merged code. Can you please rebase?

@deoxyszc

deoxyszc commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor Author

Thanks @kabir. I checked the changes merged in #1293 and verified its implementation against the regression tests from this PR on current main (fc3e9bdf), without modifying any production code.

All 83 tests in HtmlEscapeUtilsTest, ProtoJsonUtilsTest, JSONRPCUtilsTest and ProtoJsonUtils_v0_3_Test passed, including the added cases covering all supported escapes with one through six preceding backslashes, literal escape text in protobuf field names and values, JSON-RPC streaming artifact/tool-result envelopes, and v0.3 serialization.

This confirms #1293 already fixes the same corruption reported here, so this PR is redundant. I am closing it. Thanks for getting the fix merged!

@deoxyszc deoxyszc closed this Nov 6, 2026
@kabir

kabir commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator

@deoxyszc If you're saying this PR has more test coverage than what we have at the moment, please feel free to open a PR with those tests

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.

2 participants