assume credentials are the same for all contents when restarting ICE

Daniel Gultsch created

Change summary

src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java                   |  1 
src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java         | 23 
src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java               | 17 
src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java |  4 
4 files changed, 28 insertions(+), 17 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java πŸ”—

@@ -1003,6 +1003,7 @@ public class RtpSessionActivity extends XmppActivity implements XmppConnectionSe
                     RendererCommon.ScalingType.SCALE_ASPECT_FILL,
                     RendererCommon.ScalingType.SCALE_ASPECT_FIT
             );
+            //TODO this should probably only be 'connected'
             if (STATES_CONSIDERED_CONNECTED.contains(state)) {
                 binding.appBarLayout.setVisibility(View.GONE);
                 getWindow().addFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN);

src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java πŸ”—

@@ -141,7 +141,6 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web
     }
 
     private final WebRTCWrapper webRTCWrapper = new WebRTCWrapper(this);
-    //TODO convert to Queue<Map.Entry<String, Description>>?
     private final Queue<Map.Entry<String, RtpContentMap.DescriptionTransport>> pendingIceCandidates = new LinkedList<>();
     private final OmemoVerification omemoVerification = new OmemoVerification();
     private final Message message;
@@ -295,9 +294,13 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web
 
     private boolean checkForIceRestart(final JinglePacket jinglePacket, final RtpContentMap rtpContentMap) {
         final RtpContentMap existing = getRemoteContentMap();
-        final Map<String, IceUdpTransportInfo.Credentials> existingCredentials = existing.getCredentials();
-        final Map<String, IceUdpTransportInfo.Credentials> newCredentials = rtpContentMap.getCredentials();
-        if (!existingCredentials.keySet().equals(newCredentials.keySet())) {
+        final IceUdpTransportInfo.Credentials existingCredentials;
+        final IceUdpTransportInfo.Credentials newCredentials;
+        try {
+            existingCredentials = existing.getCredentials();
+            newCredentials = rtpContentMap.getCredentials();
+        } catch (final IllegalStateException e) {
+            Log.d(Config.LOGTAG, "unable to gather credentials for comparison", e);
             return false;
         }
         if (existingCredentials.equals(newCredentials)) {
@@ -307,11 +310,11 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web
         final RtpContentMap restartContentMap;
         try {
             if (isOffer) {
-                Log.d(Config.LOGTAG, "received offer to restart ICE " + newCredentials.values());
+                Log.d(Config.LOGTAG, "received offer to restart ICE " + newCredentials);
                 restartContentMap = existing.modifiedCredentials(newCredentials, IceUdpTransportInfo.Setup.ACTPASS);
             } else {
                 final IceUdpTransportInfo.Setup setup = getPeerDtlsSetup();
-                Log.d(Config.LOGTAG, "received confirmation of ICE restart" + newCredentials.values() + " peer_setup=" + setup);
+                Log.d(Config.LOGTAG, "received confirmation of ICE restart" + newCredentials + " peer_setup=" + setup);
                 // DTLS setup attribute needs to be rewritten to reflect current peer state
                 // https://groups.google.com/g/discuss-webrtc/c/DfpIMwvUfeM
                 restartContentMap = existing.modifiedCredentials(newCredentials, setup);
@@ -319,7 +322,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web
             if (applyIceRestart(jinglePacket, restartContentMap, isOffer)) {
                 return isOffer;
             } else {
-                Log.d(Config.LOGTAG,"ignored ice restart. offer="+isOffer);
+                Log.d(Config.LOGTAG, "ignoring ICE restart. sending tie-break");
                 respondWithTieBreak(jinglePacket);
                 return true;
             }
@@ -327,7 +330,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web
             respondOk(jinglePacket);
             final Throwable rootCause = Throwables.getRootCause(exception);
             if (rootCause instanceof WebRTCWrapper.PeerConnectionNotInitialized) {
-                Log.d(Config.LOGTAG,"ignoring PeerConnectionNotInitialized");
+                Log.d(Config.LOGTAG, "ignoring PeerConnectionNotInitialized");
                 //TODO don’t respond OK but respond with out-of-order
                 return true;
             }
@@ -1451,8 +1454,8 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web
     @Override
     public void onIceCandidate(final IceCandidate iceCandidate) {
         final RtpContentMap rtpContentMap = isInitiator() ? this.initiatorRtpContentMap : this.responderRtpContentMap;
-        final Collection<String> currentUfrags = Collections2.transform(rtpContentMap.getCredentials().values(), c -> c.ufrag);
-        final IceUdpTransportInfo.Candidate candidate = IceUdpTransportInfo.Candidate.fromSdpAttribute(iceCandidate.sdp, currentUfrags);
+        final String ufrag = rtpContentMap.getCredentials().ufrag;
+        final IceUdpTransportInfo.Candidate candidate = IceUdpTransportInfo.Candidate.fromSdpAttribute(iceCandidate.sdp, ufrag);
         if (candidate == null) {
             Log.d(Config.LOGTAG, "ignoring (not sending) candidate: " + iceCandidate.toString());
             return;

src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java πŸ”—

@@ -146,14 +146,22 @@ public class RtpContentMap {
         );
     }
 
-    public Map<String, IceUdpTransportInfo.Credentials> getCredentials() {
-        return Maps.transformValues(contents, dt -> dt.transport.getCredentials());
+    public IceUdpTransportInfo.Credentials getCredentials() {
+        final Set<IceUdpTransportInfo.Credentials> allCredentials = ImmutableSet.copyOf(Collections2.transform(
+                contents.values(),
+                dt -> dt.transport.getCredentials()
+        ));
+        final IceUdpTransportInfo.Credentials credentials = Iterables.getFirst(allCredentials, null);
+        if (allCredentials.size() == 1 && credentials != null) {
+            return credentials;
+        }
+        throw new IllegalStateException("Content map does not have distinct credentials");
     }
 
     public IceUdpTransportInfo.Setup getDtlsSetup() {
         final Set<IceUdpTransportInfo.Setup> setups = ImmutableSet.copyOf(Collections2.transform(
                 contents.values(),
-                dt->dt.transport.getFingerprint().getSetup()
+                dt -> dt.transport.getFingerprint().getSetup()
         ));
         final IceUdpTransportInfo.Setup setup = Iterables.getFirst(setups, null);
         if (setups.size() == 1 && setup != null) {
@@ -170,12 +178,11 @@ public class RtpContentMap {
         return count == 0;
     }
 
-    public RtpContentMap modifiedCredentials(Map<String, IceUdpTransportInfo.Credentials> credentialsMap, final IceUdpTransportInfo.Setup setup) {
+    public RtpContentMap modifiedCredentials(IceUdpTransportInfo.Credentials credentials, final IceUdpTransportInfo.Setup setup) {
         final ImmutableMap.Builder<String, DescriptionTransport> contentMapBuilder = new ImmutableMap.Builder<>();
         for (final Map.Entry<String, DescriptionTransport> content : contents.entrySet()) {
             final RtpDescription rtpDescription = content.getValue().description;
             IceUdpTransportInfo transportInfo = content.getValue().transport;
-            final IceUdpTransportInfo.Credentials credentials = Objects.requireNonNull(credentialsMap.get(content.getKey()));
             final IceUdpTransportInfo modifiedTransportInfo = transportInfo.modifyCredentials(credentials, setup);
             contentMapBuilder.put(content.getKey(), new DescriptionTransport(rtpDescription, modifiedTransportInfo));
         }

src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java πŸ”—

@@ -146,7 +146,7 @@ public class IceUdpTransportInfo extends GenericTransportInfo {
         }
 
         // https://tools.ietf.org/html/draft-ietf-mmusic-ice-sip-sdp-39#section-5.1
-        public static Candidate fromSdpAttribute(final String attribute, Collection<String> currentUfrags) {
+        public static Candidate fromSdpAttribute(final String attribute, String currentUfrag) {
             final String[] pair = attribute.split(":", 2);
             if (pair.length == 2 && "candidate".equals(pair[0])) {
                 final String[] segments = pair[1].split(" ");
@@ -163,7 +163,7 @@ public class IceUdpTransportInfo extends GenericTransportInfo {
                         additional.put(segments[i], segments[i + 1]);
                     }
                     final String ufrag = additional.get("ufrag");
-                    if (ufrag != null && !currentUfrags.contains(ufrag)) {
+                    if (ufrag != null && !ufrag.equals(currentUfrag)) {
                         return null;
                     }
                     final Candidate candidate = new Candidate();