add timeout when device call gets no ringing response

Daniel Gultsch created

Change summary

src/main/java/eu/siacs/conversations/Config.java                              |  4 
src/main/java/eu/siacs/conversations/xmpp/jingle/JingleConnectionManager.java | 73 
src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java     | 48 
3 files changed, 84 insertions(+), 41 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/Config.java 🔗

@@ -121,7 +121,6 @@ public final class Config {
     public static final boolean USE_DIRECT_JINGLE_CANDIDATES = true;
     public static final boolean USE_JINGLE_MESSAGE_INIT = true;
 
-    public static final boolean JINGLE_MESSAGE_INIT_STRICT_OFFLINE_CHECK = false;
     public static final boolean DISABLE_HTTP_UPLOAD = false;
     public static final boolean EXTENDED_SM_LOGGING = false; // log stanza counts
     public static final boolean BACKGROUND_STANZA_LOGGING =
@@ -135,6 +134,9 @@ public final class Config {
             false; // use x509 certificates to verify OMEMO keys
     public static final boolean REQUIRE_RTP_VERIFICATION =
             false; // require a/v calls to be verified with OMEMO
+    public static final boolean JINGLE_MESSAGE_INIT_STRICT_OFFLINE_CHECK = false;
+    public static final boolean JINGLE_MESSAGE_INIT_STRICT_DEVICE_TIMEOUT = false;
+    public static final long DEVICE_DISCOVERY_TIMEOUT = 6000; // in milliseconds
 
     public static final boolean ONLY_INTERNAL_STORAGE =
             false; // use internal storage instead of sdcard to save attachments

src/main/java/eu/siacs/conversations/xmpp/jingle/JingleConnectionManager.java 🔗

@@ -39,11 +39,13 @@ import eu.siacs.conversations.xmpp.jingle.stanzas.Reason;
 import eu.siacs.conversations.xmpp.jingle.stanzas.RtpDescription;
 import eu.siacs.conversations.xmpp.jingle.transports.InbandBytestreamsTransport;
 import eu.siacs.conversations.xmpp.jingle.transports.Transport;
+
 import im.conversations.android.xmpp.model.jingle.Jingle;
 import im.conversations.android.xmpp.model.stanza.Iq;
 
 import java.lang.ref.WeakReference;
 import java.security.SecureRandom;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
@@ -78,7 +80,8 @@ public class JingleConnectionManager extends AbstractConnectionManager {
 
     public void deliverPacket(final Account account, final Iq packet) {
         final var jingle = packet.getExtension(Jingle.class);
-        Preconditions.checkNotNull(jingle,"Passed iq packet w/o jingle extension to Connection Manager");
+        Preconditions.checkNotNull(
+                jingle, "Passed iq packet w/o jingle extension to Connection Manager");
         final String sessionId = jingle.getSessionId();
         final Jingle.Action action = jingle.getAction();
         if (sessionId == null) {
@@ -89,7 +92,8 @@ public class JingleConnectionManager extends AbstractConnectionManager {
             respondWithJingleError(account, packet, null, "bad-request", "cancel");
             return;
         }
-        final AbstractJingleConnection.Id id = AbstractJingleConnection.Id.of(account, packet, jingle);
+        final AbstractJingleConnection.Id id =
+                AbstractJingleConnection.Id.of(account, packet, jingle);
         final AbstractJingleConnection existingJingleConnection = connections.get(id);
         if (existingJingleConnection != null) {
             existingJingleConnection.deliverPacket(packet);
@@ -168,7 +172,8 @@ public class JingleConnectionManager extends AbstractConnectionManager {
                 account, request.generateResponse(Iq.Type.RESULT), null);
         final var iq = new Iq(Iq.Type.SET);
         iq.setTo(id.with);
-        final var sessionTermination = iq.addExtension(new Jingle(Jingle.Action.SESSION_TERMINATE, id.sessionId));
+        final var sessionTermination =
+                iq.addExtension(new Jingle(Jingle.Action.SESSION_TERMINATE, id.sessionId));
         sessionTermination.setReason(Reason.BUSY, null);
         mXmppConnectionService.sendIqPacket(account, iq, null);
     }
@@ -497,7 +502,8 @@ public class JingleConnectionManager extends AbstractConnectionManager {
                             new im.conversations.android.xmpp.model.stanza.Message();
                     errorMessage.setTo(from);
                     errorMessage.setId(remoteMsgId);
-                    errorMessage.setType(im.conversations.android.xmpp.model.stanza.Message.Type.ERROR);
+                    errorMessage.setType(
+                            im.conversations.android.xmpp.model.stanza.Message.Type.ERROR);
                     final Element error = errorMessage.addChild("error");
                     error.setAttribute("code", "404");
                     error.setAttribute("type", "cancel");
@@ -794,11 +800,58 @@ public class JingleConnectionManager extends AbstractConnectionManager {
                     account, proposal.with, proposal.sessionId, RtpEndUserState.FINDING_DEVICE);
             final var messagePacket =
                     mXmppConnectionService.getMessageGenerator().sessionProposal(proposal);
+            // in privacy preserving environments 'propose' is only ACKed when we have presence
+            // subscription (to not leak presence). Therefor a timeout is only appropriate for
+            // contacts where we can expect the 'ringing' response
+            final boolean triggerTimeout =
+                    Config.JINGLE_MESSAGE_INIT_STRICT_DEVICE_TIMEOUT
+                            || contact.mutualPresenceSubscription();
+            SCHEDULED_EXECUTOR_SERVICE.schedule(
+                    () -> {
+                        final var currentProposalState = rtpSessionProposals.get(proposal);
+                        Log.d(
+                                Config.LOGTAG,
+                                "proposal state after timeout " + currentProposalState);
+                        if (triggerTimeout
+                                && Arrays.asList(
+                                                DeviceDiscoveryState.SEARCHING,
+                                                DeviceDiscoveryState.SEARCHING_ACKNOWLEDGED)
+                                        .contains(currentProposalState)) {
+                            deviceDiscoveryTimeout(account, proposal);
+                        }
+                    },
+                    Config.DEVICE_DISCOVERY_TIMEOUT,
+                    TimeUnit.MILLISECONDS);
             mXmppConnectionService.sendMessagePacket(account, messagePacket);
             return proposal;
         }
     }
 
+    private void deviceDiscoveryTimeout(final Account account, final RtpSessionProposal proposal) {
+        // 'endUserState' is what we display in the UI. There is an argument to use 'BUSY' here
+        // instead
+        // we may or may not want to match this with the tone we are playing (see
+        // callIntegration.error() or callIntegration.busy())
+        final var endUserState = RtpEndUserState.CONNECTIVITY_ERROR;
+        Log.d(Config.LOGTAG, "call proposal still in device discovery state after timeout");
+        setTerminalSessionState(proposal, endUserState);
+
+        rtpSessionProposals.remove(proposal);
+        // error and busy would probably be both appropriate tones to play
+        // playing the error tone is probably more in line with what happens on a technical level
+        // and would be a similar UX to what happens when you call a user that doesn't exist
+        // playing the busy tone might be more in line with what some telephony networks play
+        proposal.callIntegration.error();
+        writeLogMissedOutgoing(
+                account, proposal.with, proposal.sessionId, null, System.currentTimeMillis());
+        mXmppConnectionService.notifyJingleRtpConnectionUpdate(
+                account, proposal.with, proposal.sessionId, endUserState);
+
+        final var retraction =
+                mXmppConnectionService.getMessageGenerator().sessionRetract(proposal);
+        mXmppConnectionService.sendMessagePacket(account, retraction);
+    }
+
     public void sendJingleMessageFinish(
             final Contact contact, final String sessionId, final Reason reason) {
         final var account = contact.getAccount();
@@ -869,8 +922,7 @@ public class JingleConnectionManager extends AbstractConnectionManager {
             Log.d(
                     Config.LOGTAG,
                     account.getJid().asBareJid() + ": unable to deliver ibb packet. missing sid");
-            account.getXmppConnection()
-                    .sendIqPacket(packet.generateResponse(Iq.Type.ERROR), null);
+            account.getXmppConnection().sendIqPacket(packet.generateResponse(Iq.Type.ERROR), null);
             return;
         }
         for (final AbstractJingleConnection connection : this.connections.values()) {
@@ -880,12 +932,10 @@ public class JingleConnectionManager extends AbstractConnectionManager {
                     if (sid.equals(inBandTransport.getStreamId())) {
                         if (inBandTransport.deliverPacket(packetType, packet.getFrom(), payload)) {
                             account.getXmppConnection()
-                                    .sendIqPacket(
-                                            packet.generateResponse(Iq.Type.RESULT), null);
+                                    .sendIqPacket(packet.generateResponse(Iq.Type.RESULT), null);
                         } else {
                             account.getXmppConnection()
-                                    .sendIqPacket(
-                                            packet.generateResponse(Iq.Type.ERROR), null);
+                                    .sendIqPacket(packet.generateResponse(Iq.Type.ERROR), null);
                         }
                         return;
                     }
@@ -895,8 +945,7 @@ public class JingleConnectionManager extends AbstractConnectionManager {
         Log.d(
                 Config.LOGTAG,
                 account.getJid().asBareJid() + ": unable to deliver ibb packet with sid=" + sid);
-        account.getXmppConnection()
-                .sendIqPacket(packet.generateResponse(Iq.Type.ERROR), null);
+        account.getXmppConnection().sendIqPacket(packet.generateResponse(Iq.Type.ERROR), null);
     }
 
     public void notifyRebound(final Account account) {

src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java 🔗

@@ -256,8 +256,7 @@ public class JingleRtpConnection extends AbstractJingleConnection
         }
     }
 
-    private void receiveTransportInfo(
-            final Iq jinglePacket, final RtpContentMap contentMap) {
+    private void receiveTransportInfo(final Iq jinglePacket, final RtpContentMap contentMap) {
         final Set<Map.Entry<String, DescriptionTransport<RtpDescription, IceUdpTransportInfo>>>
                 candidates = contentMap.contents.entrySet();
         final RtpContentMap remote = getRemoteContentMap();
@@ -345,8 +344,7 @@ public class JingleRtpConnection extends AbstractJingleConnection
         }
     }
 
-    private void receiveContentAdd(
-            final Iq jinglePacket, final RtpContentMap modification) {
+    private void receiveContentAdd(final Iq jinglePacket, final RtpContentMap modification) {
         final RtpContentMap remote = getRemoteContentMap();
         if (!Collections.disjoint(modification.getNames(), remote.getNames())) {
             respondOk(jinglePacket);
@@ -890,8 +888,7 @@ public class JingleRtpConnection extends AbstractJingleConnection
     }
 
     private void sendContentAccept(final RtpContentMap contentAcceptMap) {
-        final Iq iq =
-                contentAcceptMap.toJinglePacket(Jingle.Action.CONTENT_ACCEPT, id.sessionId);
+        final Iq iq = contentAcceptMap.toJinglePacket(Jingle.Action.CONTENT_ACCEPT, id.sessionId);
         send(iq);
     }
 
@@ -907,9 +904,7 @@ public class JingleRtpConnection extends AbstractJingleConnection
 
     private void rejectContentAdd(final RtpContentMap contentMap) {
         final Iq iq =
-                contentMap
-                        .toStub()
-                        .toJinglePacket(Jingle.Action.CONTENT_REJECT, id.sessionId);
+                contentMap.toStub().toJinglePacket(Jingle.Action.CONTENT_REJECT, id.sessionId);
         Log.d(
                 Config.LOGTAG,
                 id.getAccount().getJid().asBareJid()
@@ -918,8 +913,7 @@ public class JingleRtpConnection extends AbstractJingleConnection
         send(iq);
     }
 
-    private boolean checkForIceRestart(
-            final Iq jinglePacket, final RtpContentMap rtpContentMap) {
+    private boolean checkForIceRestart(final Iq jinglePacket, final RtpContentMap rtpContentMap) {
         final RtpContentMap existing = getRemoteContentMap();
         final Set<IceUdpTransportInfo.Credentials> existingCredentials;
         final IceUdpTransportInfo.Credentials newCredentials;
@@ -998,9 +992,7 @@ public class JingleRtpConnection extends AbstractJingleConnection
     }
 
     private boolean applyIceRestart(
-            final Iq jinglePacket,
-            final RtpContentMap restartContentMap,
-            final boolean isOffer)
+            final Iq jinglePacket, final RtpContentMap restartContentMap, final boolean isOffer)
             throws ExecutionException, InterruptedException {
         final SessionDescription sessionDescription =
                 SessionDescription.of(restartContentMap, isResponder());
@@ -1165,8 +1157,7 @@ public class JingleRtpConnection extends AbstractJingleConnection
                 MoreExecutors.directExecutor());
     }
 
-    private void receiveSessionInitiate(
-            final Iq jinglePacket, final RtpContentMap contentMap) {
+    private void receiveSessionInitiate(final Iq jinglePacket, final RtpContentMap contentMap) {
         try {
             contentMap.requireContentDescriptions();
             contentMap.requireDTLSFingerprint(true);
@@ -1256,8 +1247,7 @@ public class JingleRtpConnection extends AbstractJingleConnection
                 MoreExecutors.directExecutor());
     }
 
-    private void receiveSessionAccept(
-            final Iq jinglePacket, final RtpContentMap contentMap) {
+    private void receiveSessionAccept(final Iq jinglePacket, final RtpContentMap contentMap) {
         try {
             contentMap.requireContentDescriptions();
             contentMap.requireDTLSFingerprint();
@@ -1401,8 +1391,7 @@ public class JingleRtpConnection extends AbstractJingleConnection
         sendSessionTerminate(Reason.ofThrowable(rootCause), rootCause.getMessage());
     }
 
-    private void failureToPerformAction(
-            final Jingle.Action action, final Throwable throwable) {
+    private void failureToPerformAction(final Jingle.Action action, final Throwable throwable) {
         if (isTerminated()) {
             return;
         }
@@ -1676,7 +1665,11 @@ public class JingleRtpConnection extends AbstractJingleConnection
             }
             this.message.setTime(timestamp);
             startRinging();
-            if (xmppConnectionService.confirmMessages() && id.getContact().showInContactList()) {
+            // in environments where we always use discovery timeouts we always want to respond with
+            // 'ringing'
+            if (Config.JINGLE_MESSAGE_INIT_STRICT_DEVICE_TIMEOUT
+                    || (xmppConnectionService.confirmMessages()
+                            && id.getContact().showInContactList())) {
                 sendJingleMessage("ringing");
             }
         } else {
@@ -2011,8 +2004,7 @@ public class JingleRtpConnection extends AbstractJingleConnection
                             + contentName);
             return;
         }
-        final Iq iq =
-                transportInfo.toJinglePacket(Jingle.Action.TRANSPORT_INFO, id.sessionId);
+        final Iq iq = transportInfo.toJinglePacket(Jingle.Action.TRANSPORT_INFO, id.sessionId);
         send(iq);
     }
 
@@ -2368,7 +2360,9 @@ public class JingleRtpConnection extends AbstractJingleConnection
 
     private void sendJingleMessage(final String action, final Jid to) {
         final var messagePacket = new im.conversations.android.xmpp.model.stanza.Message();
-        messagePacket.setType(im.conversations.android.xmpp.model.stanza.Message.Type.CHAT); // we want to carbon copy those
+        messagePacket.setType(
+                im.conversations.android.xmpp.model.stanza.Message.Type
+                        .CHAT); // we want to carbon copy those
         messagePacket.setTo(to);
         final Element intent =
                 messagePacket
@@ -2548,8 +2542,7 @@ public class JingleRtpConnection extends AbstractJingleConnection
 
     private void initiateIceRestart(final RtpContentMap rtpContentMap) {
         final RtpContentMap transportInfo = rtpContentMap.transportInfo();
-        final Iq iq =
-                transportInfo.toJinglePacket(Jingle.Action.TRANSPORT_INFO, id.sessionId);
+        final Iq iq = transportInfo.toJinglePacket(Jingle.Action.TRANSPORT_INFO, id.sessionId);
         Log.d(Config.LOGTAG, "initiating ice restart: " + iq);
         iq.setTo(id.with);
         xmppConnectionService.sendIqPacket(
@@ -2604,8 +2597,7 @@ public class JingleRtpConnection extends AbstractJingleConnection
 
     private void sendContentAdd(final RtpContentMap contentAdd) {
 
-        final Iq iq =
-                contentAdd.toJinglePacket(Jingle.Action.CONTENT_ADD, id.sessionId);
+        final Iq iq = contentAdd.toJinglePacket(Jingle.Action.CONTENT_ADD, id.sessionId);
         iq.setTo(id.with);
         xmppConnectionService.sendIqPacket(
                 id.account,