From e6dea074c39350c865334bfd7877ba4f2d119138 Mon Sep 17 00:00:00 2001 From: Shaheen Gandhi Date: Tue, 17 Mar 2026 16:31:55 -0700 Subject: [PATCH] Use Jackson JSON serialization in CallManager Replace all manual JSON string concatenation with Jackson ObjectNode construction and ObjectMapper serialization. Use BigInteger for call IDs to properly represent unsigned 64-bit values in JSON. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../signal/manager/helper/CallManager.java | 124 +++++++++--------- .../manager/helper/CallManagerTest.java | 86 +++--------- 2 files changed, 81 insertions(+), 129 deletions(-) diff --git a/lib/src/main/java/org/asamk/signal/manager/helper/CallManager.java b/lib/src/main/java/org/asamk/signal/manager/helper/CallManager.java index 7fd6b4ed..d876dcd8 100644 --- a/lib/src/main/java/org/asamk/signal/manager/helper/CallManager.java +++ b/lib/src/main/java/org/asamk/signal/manager/helper/CallManager.java @@ -10,6 +10,7 @@ import org.asamk.signal.manager.internal.SignalDependencies; import org.asamk.signal.manager.storage.SignalAccount; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -27,6 +28,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.attribute.PosixFilePermissions; import java.security.SecureRandom; +import java.math.BigInteger; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -116,9 +118,11 @@ public class CallManager implements AutoCloseable { var turnServers = getTurnServers(); // Send createOutgoingCall + proceed via control channel - var peerIdStr = recipientAddress.toString(); - sendControlMessage(state, "{\"type\":\"createOutgoingCall\",\"callId\":" + callIdJson(callId) - + ",\"peerId\":\"" + escapeJson(peerIdStr) + "\"}"); + var createMsg = mapper.createObjectNode(); + createMsg.put("type", "createOutgoingCall"); + createMsg.put("callId", callIdUnsigned(callId)); + createMsg.put("peerId", recipientAddress.toString()); + sendControlMessage(state, writeJson(createMsg)); sendProceed(state, callId, turnServers); // Schedule ring timeout @@ -272,18 +276,16 @@ public class CallManager implements AutoCloseable { } // Send receivedOffer to subprocess - var opaqueB64 = java.util.Base64.getEncoder().encodeToString(opaque); - var senderIdKeyB64 = java.util.Base64.getEncoder().encodeToString(remoteIdentityKey); - var receiverIdKeyB64 = java.util.Base64.getEncoder().encodeToString(localIdentityKey); - var peerIdStr = senderAddress.toString(); - sendControlMessage(state, "{\"type\":\"receivedOffer\",\"callId\":" + callIdJson(callId) - + ",\"peerId\":\"" + escapeJson(peerIdStr) + "\"" - + ",\"senderDeviceId\":1" - + ",\"opaque\":\"" + opaqueB64 + "\"" - + ",\"age\":0" - + ",\"senderIdentityKey\":\"" + senderIdKeyB64 + "\"" - + ",\"receiverIdentityKey\":\"" + receiverIdKeyB64 + "\"" - + "}"); + var offerMsg = mapper.createObjectNode(); + offerMsg.put("type", "receivedOffer"); + offerMsg.put("callId", callIdUnsigned(callId)); + offerMsg.put("peerId", senderAddress.toString()); + offerMsg.put("senderDeviceId", 1); + offerMsg.put("opaque", java.util.Base64.getEncoder().encodeToString(opaque)); + offerMsg.put("age", 0); + offerMsg.put("senderIdentityKey", java.util.Base64.getEncoder().encodeToString(remoteIdentityKey)); + offerMsg.put("receiverIdentityKey", java.util.Base64.getEncoder().encodeToString(localIdentityKey)); + sendControlMessage(state, writeJson(offerMsg)); // Send proceed with TURN servers sendProceed(state, callId, turnServers); @@ -309,15 +311,13 @@ public class CallManager implements AutoCloseable { byte[] remoteIdentityKey = getRemoteIdentityKey(state); // Forward raw opaque to subprocess - var opaqueB64 = java.util.Base64.getEncoder().encodeToString(opaque); - var senderIdKeyB64 = java.util.Base64.getEncoder().encodeToString(remoteIdentityKey); - var receiverIdKeyB64 = java.util.Base64.getEncoder().encodeToString(localIdentityKey); - sendControlMessage(state, "{\"type\":\"receivedAnswer\"" - + ",\"opaque\":\"" + opaqueB64 + "\"" - + ",\"senderDeviceId\":1" - + ",\"senderIdentityKey\":\"" + senderIdKeyB64 + "\"" - + ",\"receiverIdentityKey\":\"" + receiverIdKeyB64 + "\"" - + "}"); + var answerMsg = mapper.createObjectNode(); + answerMsg.put("type", "receivedAnswer"); + answerMsg.put("opaque", java.util.Base64.getEncoder().encodeToString(opaque)); + answerMsg.put("senderDeviceId", 1); + answerMsg.put("senderIdentityKey", java.util.Base64.getEncoder().encodeToString(remoteIdentityKey)); + answerMsg.put("receiverIdentityKey", java.util.Base64.getEncoder().encodeToString(localIdentityKey)); + sendControlMessage(state, writeJson(answerMsg)); state.state = CallInfo.State.CONNECTING; fireCallEvent(state, null); @@ -333,8 +333,11 @@ public class CallManager implements AutoCloseable { } // Forward to subprocess as receivedIce - var b64 = java.util.Base64.getEncoder().encodeToString(opaque); - sendControlMessage(state, "{\"type\":\"receivedIce\",\"candidates\":[\"" + b64 + "\"]}"); + var iceMsg = mapper.createObjectNode(); + iceMsg.put("type", "receivedIce"); + var candidates = iceMsg.putArray("candidates"); + candidates.add(java.util.Base64.getEncoder().encodeToString(opaque)); + sendControlMessage(state, writeJson(iceMsg)); logger.debug("Forwarded ICE candidate to tunnel for call {}", callId); } @@ -364,24 +367,21 @@ public class CallManager implements AutoCloseable { } private void sendProceed(CallState state, long callId, List turnServers) { - var sb = new StringBuilder(); - sb.append("{\"type\":\"proceed\",\"callId\":").append(callIdJson(callId)); - sb.append(",\"hideIp\":false"); - sb.append(",\"iceServers\":["); - for (int i = 0; i < turnServers.size(); i++) { - if (i > 0) sb.append(","); - var ts = turnServers.get(i); - sb.append("{\"username\":\"").append(escapeJson(ts.username())).append("\""); - sb.append(",\"password\":\"").append(escapeJson(ts.password())).append("\""); - sb.append(",\"urls\":["); - for (int j = 0; j < ts.urls().size(); j++) { - if (j > 0) sb.append(","); - sb.append("\"").append(escapeJson(ts.urls().get(j))).append("\""); + var proceedMsg = mapper.createObjectNode(); + proceedMsg.put("type", "proceed"); + proceedMsg.put("callId", callIdUnsigned(callId)); + proceedMsg.put("hideIp", false); + var iceServers = proceedMsg.putArray("iceServers"); + for (var ts : turnServers) { + var server = iceServers.addObject(); + server.put("username", ts.username()); + server.put("password", ts.password()); + var urls = server.putArray("urls"); + for (var url : ts.urls()) { + urls.add(url); } - sb.append("]}"); } - sb.append("]}"); - sendControlMessage(state, sb.toString()); + sendControlMessage(state, writeJson(proceedMsg)); } private void spawnMediaTunnel(CallState state) { @@ -476,15 +476,13 @@ public class CallManager implements AutoCloseable { new SecureRandom().nextBytes(tokenBytes); state.controlToken = java.util.Base64.getEncoder().encodeToString(tokenBytes); - var sb = new StringBuilder(); - sb.append("{"); - sb.append("\"call_id\":").append(callIdJson(state.callId)); - sb.append(",\"is_outgoing\":").append(state.isOutgoing); - sb.append(",\"control_socket_path\":\"").append(escapeJson(state.controlSocketPath)).append("\""); - sb.append(",\"control_token\":\"").append(state.controlToken).append("\""); - sb.append(",\"local_device_id\":1"); - sb.append("}"); - return sb.toString(); + var config = mapper.createObjectNode(); + config.put("call_id", callIdUnsigned(state.callId)); + config.put("is_outgoing", state.isOutgoing); + config.put("control_socket_path", state.controlSocketPath); + config.put("control_token", state.controlToken); + config.put("local_device_id", 1); + return writeJson(config); } private void connectToControlSocket(CallState state) { @@ -503,7 +501,10 @@ public class CallManager implements AutoCloseable { new OutputStreamWriter(Channels.newOutputStream(channel), StandardCharsets.UTF_8), true); // Send authentication token - state.controlWriter.println("{\"type\":\"auth\",\"token\":\"" + state.controlToken + "\"}"); + var authMsg = mapper.createObjectNode(); + authMsg.put("type", "auth"); + authMsg.put("token", state.controlToken); + state.controlWriter.println(writeJson(authMsg)); logger.info("Connected to control socket for call {}", state.callId); // Flush any pending control messages @@ -636,7 +637,9 @@ public class CallManager implements AutoCloseable { if (state.acceptPending && state.controlWriter != null) { state.acceptPending = false; logger.debug("Sending deferred accept for call {}", state.callId); - state.controlWriter.println("{\"type\":\"accept\"}"); + var acceptMsg = mapper.createObjectNode(); + acceptMsg.put("type", "accept"); + state.controlWriter.println(writeJson(acceptMsg)); } } @@ -752,14 +755,17 @@ public class CallManager implements AutoCloseable { return serializedKey; } - /** Format call ID as unsigned for JSON (tunnel binary expects u64). */ - private static String callIdJson(long callId) { - return Long.toUnsignedString(callId); + /** Convert signed long call ID to unsigned BigInteger (tunnel binary expects u64). */ + private static BigInteger callIdUnsigned(long callId) { + return new BigInteger(Long.toUnsignedString(callId)); } - private static String escapeJson(String s) { - if (s == null) return ""; - return s.replace("\\", "\\\\").replace("\"", "\\\"").replace("\n", "\\n").replace("\r", "\\r"); + private static String writeJson(ObjectNode node) { + try { + return mapper.writeValueAsString(node); + } catch (com.fasterxml.jackson.core.JsonProcessingException e) { + throw new RuntimeException("Failed to serialize JSON", e); + } } private void endCall(final long callId, final String reason) { diff --git a/lib/src/test/java/org/asamk/signal/manager/helper/CallManagerTest.java b/lib/src/test/java/org/asamk/signal/manager/helper/CallManagerTest.java index 6b77e286..70cc45ff 100644 --- a/lib/src/test/java/org/asamk/signal/manager/helper/CallManagerTest.java +++ b/lib/src/test/java/org/asamk/signal/manager/helper/CallManagerTest.java @@ -5,7 +5,6 @@ import org.asamk.signal.manager.api.RecipientAddress; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.ValueSource; import java.lang.invoke.MethodHandle; @@ -16,7 +15,6 @@ import java.nio.file.Path; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -28,8 +26,7 @@ class CallManagerTest { // --- Reflection helpers for private static methods --- private static final MethodHandle GET_RAW_IDENTITY_KEY_BYTES; - private static final MethodHandle CALL_ID_JSON; - private static final MethodHandle ESCAPE_JSON; + private static final MethodHandle CALL_ID_UNSIGNED; private static final MethodHandle GENERATE_CALL_ID; static { @@ -39,11 +36,8 @@ class CallManagerTest { GET_RAW_IDENTITY_KEY_BYTES = lookup.findStatic(CallManager.class, "getRawIdentityKeyBytes", MethodType.methodType(byte[].class, byte[].class)); - CALL_ID_JSON = lookup.findStatic(CallManager.class, "callIdJson", - MethodType.methodType(String.class, long.class)); - - ESCAPE_JSON = lookup.findStatic(CallManager.class, "escapeJson", - MethodType.methodType(String.class, String.class)); + CALL_ID_UNSIGNED = lookup.findStatic(CallManager.class, "callIdUnsigned", + MethodType.methodType(BigInteger.class, long.class)); GENERATE_CALL_ID = lookup.findStatic(CallManager.class, "generateCallId", MethodType.methodType(long.class)); @@ -57,12 +51,8 @@ class CallManagerTest { return (byte[]) GET_RAW_IDENTITY_KEY_BYTES.invokeExact(serializedKey); } - private static String callIdJson(long callId) throws Throwable { - return (String) CALL_ID_JSON.invokeExact(callId); - } - - private static String escapeJson(String s) throws Throwable { - return (String) ESCAPE_JSON.invokeExact(s); + private static BigInteger callIdUnsigned(long callId) throws Throwable { + return (BigInteger) CALL_ID_UNSIGNED.invokeExact(callId); } private static long generateCallId() throws Throwable { @@ -143,78 +133,34 @@ class CallManagerTest { } // ======================================================================== - // callIdJson tests + // callIdUnsigned tests // ======================================================================== @Test - void callIdJson_zero() throws Throwable { - assertEquals("0", callIdJson(0L)); + void callIdUnsigned_zero() throws Throwable { + assertEquals(BigInteger.ZERO, callIdUnsigned(0L)); } @Test - void callIdJson_positiveLong() throws Throwable { - assertEquals("8230211930154373276", callIdJson(8230211930154373276L)); + void callIdUnsigned_positiveLong() throws Throwable { + assertEquals(new BigInteger("8230211930154373276"), callIdUnsigned(8230211930154373276L)); } @Test - void callIdJson_negativeLongBecomesUnsigned() throws Throwable { + void callIdUnsigned_negativeLongBecomesUnsigned() throws Throwable { // -1L as unsigned is 2^64 - 1 = 18446744073709551615 - assertEquals("18446744073709551615", callIdJson(-1L)); + assertEquals(new BigInteger("18446744073709551615"), callIdUnsigned(-1L)); } @Test - void callIdJson_longMinValueBecomesUnsigned() throws Throwable { + void callIdUnsigned_longMinValueBecomesUnsigned() throws Throwable { // Long.MIN_VALUE as unsigned is 2^63 = 9223372036854775808 - assertEquals("9223372036854775808", callIdJson(Long.MIN_VALUE)); + assertEquals(new BigInteger("9223372036854775808"), callIdUnsigned(Long.MIN_VALUE)); } @Test - void callIdJson_longMaxValue() throws Throwable { - assertEquals("9223372036854775807", callIdJson(Long.MAX_VALUE)); - } - - // ======================================================================== - // escapeJson tests - // ======================================================================== - - @Test - void escapeJson_null() throws Throwable { - assertEquals("", escapeJson(null)); - } - - @Test - void escapeJson_empty() throws Throwable { - assertEquals("", escapeJson("")); - } - - @Test - void escapeJson_noSpecialChars() throws Throwable { - assertEquals("hello world", escapeJson("hello world")); - } - - @Test - void escapeJson_backslash() throws Throwable { - assertEquals("path\\\\to\\\\file", escapeJson("path\\to\\file")); - } - - @Test - void escapeJson_doubleQuote() throws Throwable { - assertEquals("say \\\"hello\\\"", escapeJson("say \"hello\"")); - } - - @Test - void escapeJson_newline() throws Throwable { - assertEquals("line1\\nline2", escapeJson("line1\nline2")); - } - - @Test - void escapeJson_carriageReturn() throws Throwable { - assertEquals("line1\\rline2", escapeJson("line1\rline2")); - } - - @Test - void escapeJson_allSpecialChars() throws Throwable { - assertEquals("a\\\\b\\\"c\\nd\\re", escapeJson("a\\b\"c\nd\re")); + void callIdUnsigned_longMaxValue() throws Throwable { + assertEquals(new BigInteger("9223372036854775807"), callIdUnsigned(Long.MAX_VALUE)); } // ========================================================================