From 8169c9031bb104f83919b67f86b7213db9a76ffc Mon Sep 17 00:00:00 2001 From: Shaheen Gandhi Date: Tue, 17 Mar 2026 16:41:55 -0700 Subject: [PATCH] Add subscribeCallEvents command for opt-in call event notifications Call events are no longer subscribed by default. JSON-RPC clients must explicitly call subscribeCallEvents to receive callEvent notifications and enable incoming call handling. This avoids sending unwanted call events to clients that don't use voice calling. Also adds unsubscribeCallEvents for cleanup, idempotent subscription guard, and updates CALL_TUNNEL.md to document the subscription step. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/CALL_TUNNEL.md | 10 + .../SignalJsonRpcDispatcherHandler.java | 95 ++++- .../jsonrpc/SubscribeCallEventsTest.java | 351 ++++++++++++++++++ 3 files changed, 445 insertions(+), 11 deletions(-) create mode 100644 src/test/java/org/asamk/signal/jsonrpc/SubscribeCallEventsTest.java diff --git a/docs/CALL_TUNNEL.md b/docs/CALL_TUNNEL.md index fb90806c..b60d188a 100644 --- a/docs/CALL_TUNNEL.md +++ b/docs/CALL_TUNNEL.md @@ -209,8 +209,14 @@ signal-cli signal-call-tunnel Remote Phone An external application (bot, UI, test script) interacts via JSON-RPC only. It never touches the control socket directly. +**Important:** Call event notifications are not sent by default. Clients must +call `subscribeCallEvents` before initiating or receiving calls. Without this, +incoming calls are silently ignored (no tunnel is spawned). + ``` JSON-RPC Client signal-cli daemon + | | + |-- subscribeCallEvents() ------------>| (required: enables call support) | | |-- startCall(recipient) ------------->| |<-- {callId, state, -| @@ -233,6 +239,8 @@ For incoming calls: ``` JSON-RPC Client signal-cli daemon + | | + |-- subscribeCallEvents() ------------>| (if not already subscribed) | | |<-- callEvent: RINGING_INCOMING ------| (includes callId, device names) | | @@ -248,6 +256,8 @@ JSON-RPC Client signal-cli daemon | (via platform audio APIs) | ``` +To stop receiving call events, call `unsubscribeCallEvents`. + --- ## State Machine diff --git a/src/main/java/org/asamk/signal/jsonrpc/SignalJsonRpcDispatcherHandler.java b/src/main/java/org/asamk/signal/jsonrpc/SignalJsonRpcDispatcherHandler.java index 1a7d3973..68a4ae21 100644 --- a/src/main/java/org/asamk/signal/jsonrpc/SignalJsonRpcDispatcherHandler.java +++ b/src/main/java/org/asamk/signal/jsonrpc/SignalJsonRpcDispatcherHandler.java @@ -64,11 +64,6 @@ public class SignalJsonRpcDispatcherHandler { c.addOnManagerRemovedHandler(this::unsubscribeReceive); } - for (var m : c.getManagers()) { - subscribeCallEvents(m); - } - c.addOnManagerAddedHandler(this::subscribeCallEvents); - handleConnection(); } @@ -79,8 +74,6 @@ public class SignalJsonRpcDispatcherHandler { subscribeReceive(m, true); } - subscribeCallEvents(m); - final var currentThread = Thread.currentThread(); m.addClosedListener(currentThread::interrupt); @@ -88,6 +81,10 @@ public class SignalJsonRpcDispatcherHandler { } private void subscribeCallEvents(final Manager manager) { + // Prevent duplicate subscriptions for the same manager + if (callEventHandlers.stream().anyMatch(p -> p.first().equals(manager))) { + return; + } Manager.CallEventListener listener = (callInfo, reason) -> { final var params = new ObjectNode(objectMapper.getNodeFactory()); params.set("account", params.textNode(manager.getSelfNumber())); @@ -106,6 +103,24 @@ public class SignalJsonRpcDispatcherHandler { callEventHandlers.add(new Pair<>(manager, listener)); } + private void unsubscribeCallEvents(final Manager manager) { + var iterator = callEventHandlers.iterator(); + while (iterator.hasNext()) { + var pair = iterator.next(); + if (pair.first().equals(manager)) { + pair.first().removeCallEventListener(pair.second()); + iterator.remove(); + } + } + } + + private void unsubscribeAllCallEvents() { + for (var pair : callEventHandlers) { + pair.first().removeCallEventListener(pair.second()); + } + callEventHandlers.clear(); + } + private static final AtomicInteger nextSubscriptionId = new AtomicInteger(0); private int subscribeReceive(final Manager manager, boolean internalSubscription) { @@ -169,10 +184,7 @@ public class SignalJsonRpcDispatcherHandler { } finally { receiveHandlers.forEach((_subscriptionId, handlers) -> handlers.forEach(this::unsubscribeReceiveHandler)); receiveHandlers.clear(); - for (var pair : callEventHandlers) { - pair.first().removeCallEventListener(pair.second()); - } - callEventHandlers.clear(); + unsubscribeAllCallEvents(); } } @@ -189,6 +201,12 @@ public class SignalJsonRpcDispatcherHandler { if ("unsubscribeReceive".equals(method)) { return new UnsubscribeReceiveCommand(); } + if ("subscribeCallEvents".equals(method)) { + return new SubscribeCallEventsCommand(); + } + if ("unsubscribeCallEvents".equals(method)) { + return new UnsubscribeCallEventsCommand(); + } return Commands.getCommand(method); } @@ -272,4 +290,59 @@ public class SignalJsonRpcDispatcherHandler { }; } } + + private class SubscribeCallEventsCommand implements JsonRpcSingleCommand, JsonRpcMultiCommand { + + @Override + public String getName() { + return "subscribeCallEvents"; + } + + @Override + public void handleCommand( + final Void request, + final Manager m, + final JsonWriter jsonWriter + ) throws CommandException { + subscribeCallEvents(m); + } + + @Override + public void handleCommand( + final Void request, + final MultiAccountManager c, + final JsonWriter jsonWriter + ) throws CommandException { + for (var m : c.getManagers()) { + subscribeCallEvents(m); + } + c.addOnManagerAddedHandler(SignalJsonRpcDispatcherHandler.this::subscribeCallEvents); + } + } + + private class UnsubscribeCallEventsCommand implements JsonRpcSingleCommand, JsonRpcMultiCommand { + + @Override + public String getName() { + return "unsubscribeCallEvents"; + } + + @Override + public void handleCommand( + final Void request, + final Manager m, + final JsonWriter jsonWriter + ) throws CommandException { + unsubscribeCallEvents(m); + } + + @Override + public void handleCommand( + final Void request, + final MultiAccountManager c, + final JsonWriter jsonWriter + ) throws CommandException { + unsubscribeAllCallEvents(); + } + } } diff --git a/src/test/java/org/asamk/signal/jsonrpc/SubscribeCallEventsTest.java b/src/test/java/org/asamk/signal/jsonrpc/SubscribeCallEventsTest.java new file mode 100644 index 00000000..0e072eb6 --- /dev/null +++ b/src/test/java/org/asamk/signal/jsonrpc/SubscribeCallEventsTest.java @@ -0,0 +1,351 @@ +package org.asamk.signal.jsonrpc; + +import org.asamk.signal.manager.Manager; +import org.asamk.signal.manager.MultiAccountManager; +import org.asamk.signal.manager.RegistrationManager; +import org.asamk.signal.manager.ProvisioningManager; +import org.asamk.signal.manager.api.*; +import org.asamk.signal.output.JsonWriter; + +import org.junit.jupiter.api.Test; + +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.net.URI; +import java.time.Duration; +import java.util.*; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Tests for the subscribeCallEvents / unsubscribeCallEvents JSON-RPC commands + * introduced in commit d1e93dd. + */ +class SubscribeCallEventsTest { + + /** + * Feeds pre-configured JSON-RPC lines to the handler, then returns null to end. + */ + private static class LineFeeder { + private final Queue lines = new ConcurrentLinkedQueue<>(); + + void addLine(String line) { + lines.add(line); + } + + String getLine() { + return lines.poll(); + } + } + + /** + * Captures JSON-RPC responses written by the handler. + */ + private static class CapturingJsonWriter implements JsonWriter { + final List written = Collections.synchronizedList(new ArrayList<>()); + + @Override + public void write(final Object object) { + written.add(object); + } + } + + /** + * Minimal Manager stub that tracks call event listener add/remove calls. + */ + private static class StubManager implements Manager { + final List listeners = new ArrayList<>(); + final AtomicInteger addCount = new AtomicInteger(0); + final AtomicInteger removeCount = new AtomicInteger(0); + final String selfNumber; + + StubManager(String selfNumber) { + this.selfNumber = selfNumber; + } + + @Override public void addCallEventListener(CallEventListener listener) { + addCount.incrementAndGet(); + listeners.add(listener); + } + + @Override public void removeCallEventListener(CallEventListener listener) { + removeCount.incrementAndGet(); + listeners.remove(listener); + } + + @Override public String getSelfNumber() { return selfNumber; } + + // --- Stubs for remaining Manager interface methods --- + @Override public Map getUserStatus(Set n) { return Map.of(); } + @Override public Map getUsernameStatus(Set u) { return Map.of(); } + @Override public void updateAccountAttributes(String d, Boolean u, Boolean dn, Boolean ns) {} + @Override public Configuration getConfiguration() { return null; } + @Override public void updateConfiguration(Configuration c) {} + @Override public void updateProfile(UpdateProfile u) {} + @Override public String getUsername() { return null; } + @Override public UsernameLinkUrl getUsernameLink() { return null; } + @Override public void setUsername(String u) {} + @Override public void deleteUsername() {} + @Override public void startChangeNumber(String n, boolean v, String c) {} + @Override public void finishChangeNumber(String n, String v, String p) {} + @Override public void unregister() {} + @Override public void deleteAccount() {} + @Override public void submitRateLimitRecaptchaChallenge(String c, String cap) {} + @Override public List getLinkedDevices() { return List.of(); } + @Override public void updateLinkedDevice(int d, String n) {} + @Override public void removeLinkedDevices(int d) {} + @Override public void addDeviceLink(DeviceLinkUrl u) {} + @Override public void setRegistrationLockPin(Optional p) {} + @Override public List getGroups() { return List.of(); } + @Override public List getGroups(Collection g) { return List.of(); } + @Override public SendGroupMessageResults quitGroup(GroupId g, Set a) { return null; } + @Override public void deleteGroup(GroupId g) {} + @Override public Pair createGroup(String n, Set m, String a) { return null; } + @Override public SendGroupMessageResults updateGroup(GroupId g, UpdateGroup u) { return null; } + @Override public Pair joinGroup(GroupInviteLinkUrl u) { return null; } + @Override public SendMessageResults sendTypingMessage(TypingAction a, Set r) { return null; } + @Override public SendMessageResults sendReadReceipt(RecipientIdentifier.Single s, List m) { return null; } + @Override public SendMessageResults sendViewedReceipt(RecipientIdentifier.Single s, List m) { return null; } + @Override public SendMessageResults sendMessage(Message m, Set r, boolean n) { return null; } + @Override public SendMessageResults sendEditMessage(Message m, Set r, long t) { return null; } + @Override public SendMessageResults sendRemoteDeleteMessage(long t, Set r) { return null; } + @Override public SendMessageResults sendMessageReaction(String e, boolean rm, RecipientIdentifier.Single a, long t, Set r, boolean n, boolean s) { return null; } + @Override public SendMessageResults sendAdminDelete(RecipientIdentifier.Single a, long t, Set r, boolean n, boolean s) { return null; } + @Override public SendMessageResults sendPinMessage(int d, RecipientIdentifier.Single a, long t, Set r, boolean n, boolean s) { return null; } + @Override public SendMessageResults sendUnpinMessage(RecipientIdentifier.Single a, long t, Set r, boolean n, boolean s) { return null; } + @Override public SendMessageResults sendPaymentNotificationMessage(byte[] r, String n, RecipientIdentifier.Single re) { return null; } + @Override public SendMessageResults sendEndSessionMessage(Set r) { return null; } + @Override public SendMessageResults sendMessageRequestResponse(MessageEnvelope.Sync.MessageRequestResponse.Type t, Set r) { return null; } + @Override public SendMessageResults sendPollCreateMessage(String q, boolean a, List o, Set r, boolean n) { return null; } + @Override public SendMessageResults sendPollVoteMessage(RecipientIdentifier.Single a, long t, List o, int v, Set r, boolean n) { return null; } + @Override public SendMessageResults sendPollTerminateMessage(long t, Set r, boolean n) { return null; } + @Override public void hideRecipient(RecipientIdentifier.Single r) {} + @Override public void deleteRecipient(RecipientIdentifier.Single r) {} + @Override public void deleteContact(RecipientIdentifier.Single r) {} + @Override public void setContactName(RecipientIdentifier.Single r, String g, String f, String ng, String nf, String n) {} + @Override public void setContactsBlocked(Collection r, boolean b) {} + @Override public void setGroupsBlocked(Collection g, boolean b) {} + @Override public void setExpirationTimer(RecipientIdentifier.Single r, int t) {} + @Override public StickerPackUrl uploadStickerPack(File p) { return null; } + @Override public void installStickerPack(StickerPackUrl u) {} + @Override public List getStickerPacks() { return List.of(); } + @Override public void requestAllSyncData() {} + @Override public void addReceiveHandler(ReceiveMessageHandler h, boolean w) {} + @Override public void removeReceiveHandler(ReceiveMessageHandler h) {} + @Override public boolean isReceiving() { return false; } + @Override public void receiveMessages(Optional t, Optional m, ReceiveMessageHandler h) {} + @Override public void stopReceiveMessages() {} + @Override public void setReceiveConfig(ReceiveConfig r) {} + @Override public boolean isContactBlocked(RecipientIdentifier.Single r) { return false; } + @Override public void sendContacts() {} + @Override public List getRecipients(boolean o, Optional b, Collection a, Optional n) { return List.of(); } + @Override public String getContactOrProfileName(RecipientIdentifier.Single r) { return null; } + @Override public Group getGroup(GroupId g) { return null; } + @Override public List getIdentities() { return List.of(); } + @Override public List getIdentities(RecipientIdentifier.Single r) { return List.of(); } + @Override public boolean trustIdentityVerified(RecipientIdentifier.Single r, IdentityVerificationCode v) { return false; } + @Override public boolean trustIdentityAllKeys(RecipientIdentifier.Single r) { return false; } + @Override public void addAddressChangedListener(Runnable l) {} + @Override public void addClosedListener(Runnable l) {} + @Override public InputStream retrieveAttachment(String id) { return null; } + @Override public InputStream retrieveContactAvatar(RecipientIdentifier.Single r) { return null; } + @Override public InputStream retrieveProfileAvatar(RecipientIdentifier.Single r) { return null; } + @Override public InputStream retrieveGroupAvatar(GroupId g) { return null; } + @Override public InputStream retrieveSticker(StickerPackId s, int i) { return null; } + @Override public CallInfo startCall(RecipientIdentifier.Single r) { return null; } + @Override public CallInfo acceptCall(long c) { return null; } + @Override public void hangupCall(long c) {} + @Override public void rejectCall(long c) {} + @Override public List listActiveCalls() { return List.of(); } + @Override public void sendCallOffer(RecipientIdentifier.Single r, CallOffer o) {} + @Override public void sendCallAnswer(RecipientIdentifier.Single r, long c, byte[] a) {} + @Override public void sendIceUpdate(RecipientIdentifier.Single r, long c, List i) {} + @Override public void sendHangup(RecipientIdentifier.Single r, long c, MessageEnvelope.Call.Hangup.Type t) {} + @Override public void sendBusy(RecipientIdentifier.Single r, long c) {} + @Override public List getTurnServerInfo() { return List.of(); } + @Override public void close() {} + } + + /** + * Minimal MultiAccountManager stub for multi-account mode tests. + */ + private static class StubMultiAccountManager implements MultiAccountManager { + final List managers; + final List> addedHandlers = new ArrayList<>(); + + StubMultiAccountManager(List managers) { + this.managers = new ArrayList<>(managers); + } + + @Override public List getAccountNumbers() { + return managers.stream().map(Manager::getSelfNumber).toList(); + } + + @Override public List getManagers() { return managers; } + + @Override public void addOnManagerAddedHandler(Consumer handler) { + addedHandlers.add(handler); + } + + @Override public void addOnManagerRemovedHandler(Consumer handler) {} + + @Override public Manager getManager(String phoneNumber) { + return managers.stream().filter(m -> phoneNumber.equals(m.getSelfNumber())).findFirst().orElse(null); + } + + @Override public URI getNewProvisioningDeviceLinkUri() { return null; } + @Override public ProvisioningManager getProvisioningManagerFor(URI u) { return null; } + @Override public RegistrationManager getNewRegistrationManager(String a) { return null; } + @Override public void close() {} + } + + private static String jsonRpcCall(int id, String method) { + return "{\"jsonrpc\":\"2.0\",\"id\":" + id + ",\"method\":\"" + method + "\"}"; + } + + // --- Single-account mode tests --- + + @Test + void callEventsNotSubscribedByDefault() { + var manager = new StubManager("+15551234567"); + var feeder = new LineFeeder(); + var writer = new CapturingJsonWriter(); + + // Send no subscribeCallEvents, just end the connection + var handler = new SignalJsonRpcDispatcherHandler(writer, feeder::getLine, true); + handler.handleConnection(manager); + + // No listeners should have been added + assertEquals(0, manager.addCount.get(), "call events should not be auto-subscribed"); + } + + @Test + void subscribeCallEventsAddsListener() { + var manager = new StubManager("+15551234567"); + var feeder = new LineFeeder(); + var writer = new CapturingJsonWriter(); + + feeder.addLine(jsonRpcCall(1, "subscribeCallEvents")); + // null terminates the read loop + + var handler = new SignalJsonRpcDispatcherHandler(writer, feeder::getLine, true); + handler.handleConnection(manager); + + assertEquals(1, manager.addCount.get(), "subscribeCallEvents should add one listener"); + // Cleanup in finally block should remove it + assertEquals(1, manager.removeCount.get(), "cleanup should remove the listener"); + assertEquals(0, manager.listeners.size(), "no listeners should remain after cleanup"); + } + + @Test + void subscribeCallEventsIsIdempotent() { + var manager = new StubManager("+15551234567"); + var feeder = new LineFeeder(); + var writer = new CapturingJsonWriter(); + + feeder.addLine(jsonRpcCall(1, "subscribeCallEvents")); + feeder.addLine(jsonRpcCall(2, "subscribeCallEvents")); + + var handler = new SignalJsonRpcDispatcherHandler(writer, feeder::getLine, true); + handler.handleConnection(manager); + + // Idempotent guard: second call should not add another listener + assertEquals(1, manager.addCount.get(), "duplicate subscribeCallEvents should be ignored"); + } + + @Test + void unsubscribeCallEventsRemovesListener() { + var manager = new StubManager("+15551234567"); + var feeder = new LineFeeder(); + var writer = new CapturingJsonWriter(); + + feeder.addLine(jsonRpcCall(1, "subscribeCallEvents")); + feeder.addLine(jsonRpcCall(2, "unsubscribeCallEvents")); + + var handler = new SignalJsonRpcDispatcherHandler(writer, feeder::getLine, true); + handler.handleConnection(manager); + + assertEquals(1, manager.addCount.get(), "should have subscribed once"); + // removeCount: 1 from explicit unsubscribe. The finally block's unsubscribeAllCallEvents + // iterates an empty list so adds 0 more. + assertEquals(1, manager.removeCount.get(), "should have unsubscribed once"); + assertEquals(0, manager.listeners.size()); + } + + @Test + void unsubscribeWithoutSubscribeIsNoOp() { + var manager = new StubManager("+15551234567"); + var feeder = new LineFeeder(); + var writer = new CapturingJsonWriter(); + + feeder.addLine(jsonRpcCall(1, "unsubscribeCallEvents")); + + var handler = new SignalJsonRpcDispatcherHandler(writer, feeder::getLine, true); + handler.handleConnection(manager); + + assertEquals(0, manager.addCount.get()); + assertEquals(0, manager.removeCount.get()); + } + + // --- Multi-account mode tests --- + + @Test + void multiAccountSubscribeCallEventsSubscribesAllManagers() { + var manager1 = new StubManager("+15551111111"); + var manager2 = new StubManager("+15552222222"); + var multi = new StubMultiAccountManager(List.of(manager1, manager2)); + + var feeder = new LineFeeder(); + var writer = new CapturingJsonWriter(); + + feeder.addLine(jsonRpcCall(1, "subscribeCallEvents")); + + var handler = new SignalJsonRpcDispatcherHandler(writer, feeder::getLine, true); + handler.handleConnection(multi); + + assertEquals(1, manager1.addCount.get(), "manager1 should have one listener"); + assertEquals(1, manager2.addCount.get(), "manager2 should have one listener"); + // Also registers an onManagerAdded handler + assertEquals(1, multi.addedHandlers.size(), "should register onManagerAdded handler"); + } + + @Test + void multiAccountUnsubscribeCallEventsCleansUpAll() { + var manager1 = new StubManager("+15551111111"); + var manager2 = new StubManager("+15552222222"); + var multi = new StubMultiAccountManager(List.of(manager1, manager2)); + + var feeder = new LineFeeder(); + var writer = new CapturingJsonWriter(); + + feeder.addLine(jsonRpcCall(1, "subscribeCallEvents")); + feeder.addLine(jsonRpcCall(2, "unsubscribeCallEvents")); + + var handler = new SignalJsonRpcDispatcherHandler(writer, feeder::getLine, true); + handler.handleConnection(multi); + + assertEquals(1, manager1.addCount.get()); + assertEquals(1, manager2.addCount.get()); + assertEquals(1, manager1.removeCount.get(), "manager1 listener should be removed"); + assertEquals(1, manager2.removeCount.get(), "manager2 listener should be removed"); + } + + @Test + void multiAccountCallEventsNotSubscribedByDefault() { + var manager1 = new StubManager("+15551111111"); + var multi = new StubMultiAccountManager(List.of(manager1)); + + var feeder = new LineFeeder(); + var writer = new CapturingJsonWriter(); + + var handler = new SignalJsonRpcDispatcherHandler(writer, feeder::getLine, true); + handler.handleConnection(multi); + + assertEquals(0, manager1.addCount.get(), "call events should not be auto-subscribed in multi mode"); + } +}