fix: disable HTML escaping in protobuf JSON serialization across REST and JSON-RPC transports #947
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| package org.a2aproject.sdk.util; | ||
|
|
||
| /** | ||
| * Utilities for removing HTML escaping applied by Gson's {@link com.google.gson.stream.JsonWriter} | ||
| * when {@code htmlSafe} is enabled (the default). | ||
| */ | ||
| public final class HtmlEscapeUtils { | ||
|
|
||
| private HtmlEscapeUtils() { | ||
| } | ||
|
|
||
| /** | ||
| * Removes HTML escaping applied by Gson's {@link com.google.gson.stream.JsonWriter} when | ||
| * {@code htmlSafe} is enabled (the default). Restores literal | ||
| * {@code <}, {@code >}, {@code &}, {@code =}, and {@code '}. | ||
| * <p> | ||
| * Gson also escapes U+2028 (line separator) and U+2029 (paragraph separator) in HTML-safe | ||
| * mode, but those are left as-is because they are valid JSON encodings that preserve the | ||
| * original characters without data corruption. | ||
| * | ||
| * @param json the JSON string potentially containing HTML-escaped sequences | ||
| * @return the JSON string with literal characters restored | ||
| */ | ||
| public static String removeHtmlEscaping(String json) { | ||
| return json.replace("\\u003c", "<") | ||
| .replace("\\u003e", ">") | ||
| .replace("\\u0026", "&") | ||
| .replace("\\u003d", "=") | ||
| .replace("\\u0027", "'"); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| package org.a2aproject.sdk.util; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| public class HtmlEscapeUtilsTest { | ||
|
|
||
| @Test | ||
| public void removeHtmlEscaping_restoresAngleBrackets() { | ||
| assertEquals("<event-topic>", HtmlEscapeUtils.removeHtmlEscaping("\\u003cevent-topic\\u003e")); | ||
| } | ||
|
|
||
| @Test | ||
| public void removeHtmlEscaping_restoresAmpersand() { | ||
| assertEquals("foo&bar", HtmlEscapeUtils.removeHtmlEscaping("foo\\u0026bar")); | ||
| } | ||
|
|
||
| @Test | ||
| public void removeHtmlEscaping_restoresEquals() { | ||
| assertEquals("a=b", HtmlEscapeUtils.removeHtmlEscaping("a\\u003db")); | ||
| } | ||
|
|
||
| @Test | ||
| public void removeHtmlEscaping_restoresApostrophe() { | ||
| assertEquals("it's", HtmlEscapeUtils.removeHtmlEscaping("it\\u0027s")); | ||
| } | ||
|
|
||
| @Test | ||
| public void removeHtmlEscaping_handlesMultipleEscapes() { | ||
| assertEquals("<tag>&</tag>", | ||
| HtmlEscapeUtils.removeHtmlEscaping("\\u003ctag\\u003e\\u0026\\u003c/tag\\u003e")); | ||
| } | ||
|
|
||
| @Test | ||
| public void removeHtmlEscaping_leavesRegularJsonUntouched() { | ||
| String json = "{\"key\": \"value\", \"num\": 42}"; | ||
| assertEquals(json, HtmlEscapeUtils.removeHtmlEscaping(json)); | ||
| } | ||
|
|
||
| @Test | ||
| public void removeHtmlEscaping_handlesEmptyString() { | ||
| assertEquals("", HtmlEscapeUtils.removeHtmlEscaping("")); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,6 +48,11 @@ | |
| <groupId>com.google.api.grpc</groupId> | ||
| <artifactId>proto-google-common-protos</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>com.google.protobuf</groupId> | ||
| <artifactId>protobuf-java-util</artifactId> | ||
| <scope>provided</scope> | ||
| </dependency> | ||
|
|
||
| <!-- Annotation dependency for generated code --> | ||
| <dependency> | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| package org.a2aproject.sdk.compat03.grpc.utils; | ||
|
|
||
| import com.google.protobuf.InvalidProtocolBufferException; | ||
| import com.google.protobuf.MessageOrBuilder; | ||
| import com.google.protobuf.util.JsonFormat; | ||
| import org.a2aproject.sdk.util.HtmlEscapeUtils; | ||
| import org.jspecify.annotations.Nullable; | ||
|
|
||
| /** | ||
| * Protobuf-to-JSON serialization without HTML escaping. | ||
| * <p> | ||
| * {@link JsonFormat#printer()} delegates to Gson's {@link com.google.gson.stream.JsonWriter} | ||
| * which HTML-escapes {@code <}, {@code >}, and {@code &} by default. This utility | ||
| * removes those escape sequences via {@link HtmlEscapeUtils#removeHtmlEscaping(String)}. | ||
| * <p> | ||
| * Intentionally duplicated from {@code org.a2aproject.sdk.grpc.utils.ProtoJsonUtils} | ||
| * to maintain v0.3 module isolation (compat-0.3/spec-grpc cannot depend on spec-grpc). | ||
| */ | ||
| public final class ProtoJsonUtils_v0_3 { | ||
|
|
||
| private ProtoJsonUtils_v0_3() { | ||
| } | ||
|
|
||
| /** | ||
| * Serializes a protobuf message to JSON using the supplied printer, | ||
| * then removes HTML escaping. | ||
| * | ||
| * @param printer the configured {@link JsonFormat.Printer} (callers choose options | ||
| * such as {@code alwaysPrintFieldsWithNoPresence()} or | ||
| * {@code omittingInsignificantWhitespace()}) | ||
| * @param proto the protobuf message to serialize, or {@code null} | ||
| * @return JSON string without HTML-escaped characters, or empty string if proto is null | ||
| */ | ||
| public static String toJson(JsonFormat.Printer printer, @Nullable MessageOrBuilder proto) throws InvalidProtocolBufferException { | ||
| if (proto == null) { | ||
| return ""; | ||
| } | ||
| return HtmlEscapeUtils.removeHtmlEscaping(printer.print(proto)); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| package org.a2aproject.sdk.compat03.grpc.utils; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertFalse; | ||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||
|
|
||
| import java.util.Collections; | ||
|
|
||
| import com.google.protobuf.util.JsonFormat; | ||
|
|
||
| import org.a2aproject.sdk.compat03.spec.Message_v0_3; | ||
| import org.a2aproject.sdk.compat03.spec.MessageSendParams_v0_3; | ||
| import org.a2aproject.sdk.compat03.spec.TextPart_v0_3; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| public class ProtoJsonUtils_v0_3_Test { | ||
|
|
||
| @Test | ||
| public void toJson_doesNotHtmlEscapeAngleBrackets() throws Exception { | ||
| MessageSendParams_v0_3 params = new MessageSendParams_v0_3( | ||
| new Message_v0_3.Builder() | ||
| .role(Message_v0_3.Role.USER) | ||
| .parts(Collections.singletonList(new TextPart_v0_3("<event-topic>"))) | ||
| .contextId("context-1234") | ||
| .messageId("message-1234") | ||
| .build(), | ||
| null, null | ||
| ); | ||
| var proto = ProtoUtils_v0_3.ToProto.sendMessageRequest(params); | ||
|
|
||
| String json = ProtoJsonUtils_v0_3.toJson(JsonFormat.printer(), proto); | ||
|
|
||
| assertTrue(json.contains("<event-topic>"), | ||
| "JSON should preserve literal '<event-topic>' but got: " + json); | ||
| assertFalse(json.contains("\\u003c"), | ||
| "JSON must not contain HTML-escaped '<' (\\u003c) but got: " + json); | ||
| assertFalse(json.contains("\\u003e"), | ||
| "JSON must not contain HTML-escaped '>' (\\u003e) but got: " + json); | ||
| } | ||
|
|
||
| @Test | ||
| public void toJson_doesNotHtmlEscapeAmpersand() throws Exception { | ||
| MessageSendParams_v0_3 params = new MessageSendParams_v0_3( | ||
| new Message_v0_3.Builder() | ||
| .role(Message_v0_3.Role.USER) | ||
| .parts(Collections.singletonList(new TextPart_v0_3("foo&bar"))) | ||
| .contextId("context-1234") | ||
| .messageId("message-1234") | ||
| .build(), | ||
| null, null | ||
| ); | ||
| var proto = ProtoUtils_v0_3.ToProto.sendMessageRequest(params); | ||
|
|
||
| String json = ProtoJsonUtils_v0_3.toJson(JsonFormat.printer(), proto); | ||
|
|
||
| assertTrue(json.contains("foo&bar"), | ||
| "JSON should preserve literal '&' but got: " + json); | ||
| assertFalse(json.contains("\\u0026"), | ||
| "JSON must not contain HTML-escaped '&' (\\u0026) but got: " + json); | ||
| } | ||
|
|
||
| @Test | ||
| public void toJson_respectsAlwaysPrintFieldsWithNoPresence() throws Exception { | ||
| MessageSendParams_v0_3 params = new MessageSendParams_v0_3( | ||
| new Message_v0_3.Builder() | ||
| .role(Message_v0_3.Role.USER) | ||
| .parts(Collections.singletonList(new TextPart_v0_3("hello"))) | ||
| .contextId("context-1234") | ||
| .messageId("message-1234") | ||
| .build(), | ||
| null, null | ||
| ); | ||
| var proto = ProtoUtils_v0_3.ToProto.sendMessageRequest(params); | ||
|
|
||
| String withDefaults = ProtoJsonUtils_v0_3.toJson( | ||
| JsonFormat.printer().alwaysPrintFieldsWithNoPresence(), proto); | ||
| String withoutDefaults = ProtoJsonUtils_v0_3.toJson(JsonFormat.printer(), proto); | ||
|
|
||
| assertTrue(withDefaults.length() > withoutDefaults.length(), | ||
| "alwaysPrintFieldsWithNoPresence should produce more fields, got:\n with: " + withDefaults + "\n without: " + withoutDefaults); | ||
| } | ||
|
|
||
| @Test | ||
| public void toJson_returnsEmptyStringForNull() throws Exception { | ||
| String result = ProtoJsonUtils_v0_3.toJson(JsonFormat.printer(), null); | ||
|
|
||
| assertEquals("", result); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| import com.google.gson.JsonSyntaxException; | ||
| import com.google.protobuf.InvalidProtocolBufferException; | ||
| import com.google.protobuf.util.JsonFormat; | ||
| import org.a2aproject.sdk.compat03.grpc.utils.ProtoJsonUtils_v0_3; | ||
| import org.a2aproject.sdk.compat03.grpc.utils.ProtoUtils_v0_3; | ||
| import jakarta.enterprise.context.ApplicationScoped; | ||
| import jakarta.inject.Inject; | ||
|
|
@@ -267,7 +268,7 @@ private void validate(String json) { | |
|
|
||
| private HTTPRestResponse createSuccessResponse(int statusCode, com.google.protobuf.Message.Builder builder) { | ||
| try { | ||
| String jsonBody = ProtoJsonUtils_v0_3.toJson(JsonFormat.printer(), builder); | ||
| return new HTTPRestResponse(statusCode, "application/json", jsonBody); | ||
| } catch (InvalidProtocolBufferException e) { | ||
| return createErrorResponse(new InternalError_v0_3("Failed to serialize response: " + e.getMessage())); | ||
|
|
@@ -307,7 +308,8 @@ public void onSubscribe(Flow.Subscription subscription) { | |
| @Override | ||
| public void onNext(StreamingEventKind_v0_3 item) { | ||
| try { | ||
| String payload = ProtoJsonUtils_v0_3.toJson( | ||
| JsonFormat.printer().omittingInsignificantWhitespace(), ProtoUtils_v0_3.ToProto.taskOrMessageStream(item)); | ||
| tube.send(payload); | ||
| if (subscription != null) { | ||
| subscription.request(1); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -576,7 +576,8 @@ public static String toJsonRPCRequest(@Nullable String requestId, String method, | |
| output.name("method").value(method); | ||
| } | ||
| if (payload != null) { | ||
| String resultValue = ProtoJsonUtils.toJson( | ||
| JsonFormat.printer().alwaysPrintFieldsWithNoPresence().omittingInsignificantWhitespace(), payload); | ||
| output.name("params").jsonValue(resultValue); | ||
| } | ||
| output.endObject(); | ||
|
|
@@ -593,7 +594,8 @@ public static String toJsonRPCResultResponse(@Nullable Object requestId, com.goo | |
| output.beginObject(); | ||
| output.name("jsonrpc").value("2.0"); | ||
| JsonUtil.writeJsonRpcId(output, requestId); | ||
| String resultValue = ProtoJsonUtils.toJson( | ||
| JsonFormat.printer().alwaysPrintFieldsWithNoPresence().omittingInsignificantWhitespace(), builder); | ||
| output.name("result").jsonValue(resultValue); | ||
| output.endObject(); | ||
| return result.toString(); | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling
printProtoAsJsoninsidelog.finecauses the expensive double-serialization and parsing of the protobuf payload to run eagerly on every failed request, even whenFINElogging is disabled. Additionally, ifprintProtoAsJsonthrows anInvalidProtocolBufferExceptionduring 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.