maptool icon indicating copy to clipboard operation
maptool copied to clipboard

[Bug]: execFunction breaks nested jsons for other clients

Open aRTy42 opened this issue 2 years ago • 3 comments

Describe the Bug

It works fine in version 1.10.4. Starting with 1.11.2, handing over a nested json to execFunction results in a messed up object on other clients. For the client running the macro it is fine.

Instead of ["one","two","three"] the output becomes
{"elements":[{"value":"one"},{"value":"two"},{"value":"three"}]}.

To Reproduce

  1. create a UDF called testUDF with this macro code
[h: broadcast(getPlayerName())]
[h: broadcast(arg(0))]
  1. create a macro button with this code:
[h: array = json.append("", "one", "two", "three")]
[h: execFunction("testUDF", json.append("", array), 1, "all")]
  1. Start a server as player1.
  2. Connect to the server as player2.
  3. If player1 clicks the macro button the chat output is
player1
["one","two","three"]
player2
{"elements":[{"value":"one"},{"value":"two"},{"value":"three"}]}
  1. If player2 clicks the macro button, the roles are reversed, meaning whoever clicks the macro gets the correct array, the other recipients get the broken version.

execLink does not show the same behaviour.

Campaign file: exec_nestedJson.zip

Expected Behaviour

All clients should receive the original array.

Screenshots

No response

MapTool Info

Working fine in version 1.10.4
Not working in versions 1.11.2, 1.11.4 and 1.11.5
(Campaign file created with version: 1.11.4)

Desktop

Windows

Additional Context

Started as a question from FullBleed on Discord.
channel: macros-questions-in-threads-only > "[BUG] execFunction changes to json"

aRTy42 avatar Mar 10 '22 18:03 aRTy42

This one bit me pretty hard... thanks for the report.

I looked over the changelogs in the 1.11.x series and didn't see any mention of changes to execFunction specifically, so I do wonder if there might be other problems. I haven't had a chance to test execLink yet...

FullBleed avatar Mar 11 '22 00:03 FullBleed

Correction: execLink does not have the same issue. I'm not sure what happened earlier, but I guess I misclicked at some point which made me believe that it did.
Updated the attached campaign file above.

aRTy42 avatar Mar 11 '22 12:03 aRTy42

This looks to be caused by a fix that prevented Hessian from deserializing arbitrary types, and the GSON types aren't on the "allow" list. Usually trying to deserialize such a type would cause a failure, but there must be something special about how the GSON types gets serialized/deserialized that allowed those to be deserialized as a HashMap instead.

This won't happen in upcoming 1.12 as we recently removed the Hessian dependency in favour of protobuf.

If we would rather want to fix up the 1.11.x series, the change is pretty trivial:

diff --git a/src/main/java/net/rptools/clientserver/hessian/HessianSecurity.java b/src/main/java/net/rptools/clientserver/hessian/HessianSecurity.java
index 643245928..9779c19c6 100644
--- a/src/main/java/net/rptools/clientserver/hessian/HessianSecurity.java
+++ b/src/main/java/net/rptools/clientserver/hessian/HessianSecurity.java
@@ -70,6 +70,11 @@ public HessianSecurity() {
 
     allow.add("net.rptools.lib.MD5Key");
 
+    allow.add("com.google.gson.JsonNull");
+    allow.add("com.google.gson.JsonArray");
+    allow.add("com.google.gson.JsonElement");
+    allow.add("com.google.gson.JsonPrimitive");
+
     Set<String> deny = new HashSet<>();
     deny.add("*");

kwvanderlinde avatar May 29 '22 17:05 kwvanderlinde

Closing as this behaviour is no longer seen since 1.12 and the move away from Hessian.

kwvanderlinde avatar May 16 '23 17:05 kwvanderlinde