Skip to content

fix: disable HTML escaping in protobuf JSON serialization across REST and JSON-RPC transports#947

Open
ehsavoie wants to merge 1 commit into
a2aproject:mainfrom
ehsavoie:issue_892
Open

fix: disable HTML escaping in protobuf JSON serialization across REST and JSON-RPC transports#947
ehsavoie wants to merge 1 commit into
a2aproject:mainfrom
ehsavoie:issue_892

Conversation

@ehsavoie

Copy link
Copy Markdown
Collaborator

JsonFormat.printer() uses Gson's JsonWriter with HTML escaping enabled by default, converting <, >, & to Unicode escape sequences. This corrupts message content containing those characters.

Fix by re-serializing through a Gson instance with disableHtmlEscaping() in RestTransport, RestHandler, JSONRPCUtils, and their v0.3 compat counterparts.

Fixes #892 🦕

@ehsavoie ehsavoie requested review from jmesnil and kabir June 22, 2026 19:44

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request addresses issue #892, where HTML special characters in protobuf messages were being escaped to Unicode sequences by JsonFormat.printer(). It introduces a non-HTML-escaping Gson instance (NON_HTML_SAFE_GSON) to re-serialize the proto JSON across several transport and handler classes, and adds corresponding unit tests. The review feedback points out a potential bug in both RestTransport.java and RestTransport_v0_3.java where the new printProtoAsJson helper method is annotated with @Nullable but immediately calls checkNotNullParam, which will throw an exception if the proto payload is null. Handling the null case gracefully is recommended to prevent runtime exceptions during error logging.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

… and JSON-RPC transports

JsonFormat.printer() uses Gson's JsonWriter with HTML escaping enabled by
default, converting <, >, & to Unicode escape sequences. This corrupts
message content containing those characters.

Fix by re-serializing through a Gson instance with disableHtmlEscaping() in
RestTransport, RestHandler, JSONRPCUtils, and their v0.3 compat counterparts.

This fixes a2aproject#892

Signed-off-by: Emmanuel Hugonnet <ehugonne@redhat.com>
@ehsavoie

Copy link
Copy Markdown
Collaborator Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request addresses issue #892, where HTML special characters in protobuf payloads were being escaped to Unicode sequences during JSON serialization. To resolve this, a custom Gson instance with HTML escaping disabled is introduced across various transport and handler classes to re-serialize the protobuf JSON. The feedback highlights performance and exception-handling issues in RestTransport and RestTransport_v0_3, where expensive serialization is executed eagerly inside log.fine statements even when fine logging is disabled, potentially masking the underlying REST errors if an exception is thrown.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines 440 to 443
if (!response.success()) {
log.fine("Error on POST processing " + JsonFormat.printer().print((MessageOrBuilder) payloadAndHeaders.getPayload()));
log.fine("Error on POST processing " + printProtoAsJson((MessageOrBuilder) payloadAndHeaders.getPayload()));
throw RestErrorMapper.mapRestError(response);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Calling printProtoAsJson inside log.fine causes the expensive double-serialization and parsing of the protobuf payload to run eagerly on every failed request, even when FINE logging is disabled. Additionally, if printProtoAsJson throws an InvalidProtocolBufferException during serialization, it will propagate that exception instead of the actual mapped REST error (RestErrorMapper.mapRestError(response)).

To fix both issues, wrap the logging statement in an if (log.isLoggable(java.util.logging.Level.FINE)) block and handle any potential serialization exceptions within it.

Suggested change
if (!response.success()) {
log.fine("Error on POST processing " + JsonFormat.printer().print((MessageOrBuilder) payloadAndHeaders.getPayload()));
log.fine("Error on POST processing " + printProtoAsJson((MessageOrBuilder) payloadAndHeaders.getPayload()));
throw RestErrorMapper.mapRestError(response);
}
if (!response.success()) {
if (log.isLoggable(java.util.logging.Level.FINE)) {
try {
log.fine("Error on POST processing " + printProtoAsJson((MessageOrBuilder) payloadAndHeaders.getPayload()));
} catch (InvalidProtocolBufferException e) {
log.log(java.util.logging.Level.FINE, "Failed to serialize payload for logging", e);
}
}
throw RestErrorMapper.mapRestError(response);
}

Comment on lines 382 to 385
if (!response.success()) {
log.fine("Error on POST processing " + JsonFormat.printer().print((MessageOrBuilder) payloadAndHeaders.getPayload()));
log.fine("Error on POST processing " + printProtoAsJson((MessageOrBuilder) payloadAndHeaders.getPayload()));
throw RestErrorMapper_v0_3.mapRestError(response);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Calling printProtoAsJson inside log.fine causes the expensive double-serialization and parsing of the protobuf payload to run eagerly on every failed request, even when FINE logging is disabled. Additionally, if printProtoAsJson throws an InvalidProtocolBufferException during serialization, it will propagate that exception instead of the actual mapped REST error (RestErrorMapper_v0_3.mapRestError(response)).

To fix both issues, wrap the logging statement in an if (log.isLoggable(java.util.logging.Level.FINE)) block and handle any potential serialization exceptions within it.

Suggested change
if (!response.success()) {
log.fine("Error on POST processing " + JsonFormat.printer().print((MessageOrBuilder) payloadAndHeaders.getPayload()));
log.fine("Error on POST processing " + printProtoAsJson((MessageOrBuilder) payloadAndHeaders.getPayload()));
throw RestErrorMapper_v0_3.mapRestError(response);
}
if (!response.success()) {
if (log.isLoggable(java.util.logging.Level.FINE)) {
try {
log.fine("Error on POST processing " + printProtoAsJson((MessageOrBuilder) payloadAndHeaders.getPayload()));
} catch (InvalidProtocolBufferException e) {
log.log(java.util.logging.Level.FINE, "Failed to serialize payload for logging", e);
}
}
throw RestErrorMapper_v0_3.mapRestError(response);
}

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.

[Bug]: Incorrect Json Escape

1 participant