fix: disable HTML escaping in protobuf JSON serialization across REST and JSON-RPC transports#947
fix: disable HTML escaping in protobuf JSON serialization across REST and JSON-RPC transports#947ehsavoie wants to merge 1 commit into
Conversation
… 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>
There was a problem hiding this comment.
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.
| private static String printProtoAsJson(@Nullable MessageOrBuilder proto) throws InvalidProtocolBufferException { | ||
| String protoJson = JsonFormat.printer().print(checkNotNullParam("proto", proto)); | ||
| return NON_HTML_SAFE_GSON.toJson(JsonParser.parseString(protoJson)); | ||
| } |
There was a problem hiding this comment.
The printProtoAsJson method is annotated with @Nullable, but it immediately calls checkNotNullParam("proto", proto), which will throw an exception if proto is null.
In sendPostRequest, this method is called to log the payload when a POST request fails:
log.fine("Error on POST processing " + printProtoAsJson((MessageOrBuilder) payloadAndHeaders.getPayload()));
If the payload is null (which is possible depending on the request or interceptors), this logging statement will throw a NullPointerException or IllegalArgumentException, masking the actual REST error and making debugging extremely difficult.
Please handle the null case gracefully by returning an empty string or a placeholder.
private static String printProtoAsJson(@Nullable MessageOrBuilder proto) throws InvalidProtocolBufferException {
if (proto == null) {
return "";
}
String protoJson = JsonFormat.printer().print(proto);
return NON_HTML_SAFE_GSON.toJson(JsonParser.parseString(protoJson));
}| private static String printProtoAsJson(@Nullable MessageOrBuilder proto) throws InvalidProtocolBufferException { | ||
| String protoJson = JsonFormat.printer().print(checkNotNullParam("proto", proto)); | ||
| return NON_HTML_SAFE_GSON.toJson(JsonParser.parseString(protoJson)); | ||
| } |
There was a problem hiding this comment.
The printProtoAsJson method is annotated with @Nullable, but it immediately calls checkNotNullParam("proto", proto), which will throw an exception if proto is null.
In sendPostRequest, this method is called to log the payload when a POST request fails:
log.fine("Error on POST processing " + printProtoAsJson((MessageOrBuilder) payloadAndHeaders.getPayload()));
If the payload is null, this logging statement will throw a NullPointerException or IllegalArgumentException, masking the actual REST error and making debugging extremely difficult.
Please handle the null case gracefully by returning an empty string or a placeholder.
private static String printProtoAsJson(@Nullable MessageOrBuilder proto) throws InvalidProtocolBufferException {
if (proto == null) {
return "";
}
String protoJson = JsonFormat.printer().print(proto);
return NON_HTML_SAFE_GSON.toJson(JsonParser.parseString(protoJson));
}
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 🦕