From 601a8cb3bcc15d15fdcc7dde387394848fa6e180 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Thu, 5 Oct 2023 16:23:43 +0200 Subject: [PATCH] process content-modify for pending content-adds --- .../xmpp/jingle/JingleRtpConnection.java | 29 +++++- .../xmpp/jingle/RtpContentMap.java | 99 ++++++++++++++++--- .../jingle/stanzas/IceUdpTransportInfo.java | 4 + 3 files changed, 116 insertions(+), 16 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java index 766482899..2995c41ad 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -503,6 +503,7 @@ public class JingleRtpConnection extends AbstractJingleConnection sendSessionTerminate(Reason.FAILED_APPLICATION, cause.getMessage()); return; } + processCandidates(receivedContentAccept.contents.entrySet()); updateEndUserState(); Log.d( Config.LOGTAG, @@ -516,7 +517,31 @@ public class JingleRtpConnection extends AbstractJingleConnection Maps.transformEntries( jinglePacket.getJingleContents(), (key, value) -> value.getSenders()); respondOk(jinglePacket); + final RtpContentMap currentOutgoing = this.outgoingContentAdd; + final Set currentOutgoingMediaIds = currentOutgoing == null ? Collections.emptySet() : currentOutgoing.contents.keySet(); Log.d(Config.LOGTAG, "receiveContentModification(" + modification + ")"); + if (currentOutgoing != null && currentOutgoingMediaIds.containsAll(modification.keySet())) { + final boolean isInitiator = isInitiator(); + final RtpContentMap modifiedContentMap; + try { + modifiedContentMap = currentOutgoing.modifiedSendersChecked(isInitiator, modification); + } catch (final IllegalArgumentException e) { + webRTCWrapper.close(); + sendSessionTerminate(Reason.FAILED_APPLICATION, e.getMessage()); + return; + } + this.outgoingContentAdd = modifiedContentMap; + Log.d(Config.LOGTAG, id.account.getJid().asBareJid()+": processed content-modification for pending content-add"); + } else { + webRTCWrapper.close(); + sendSessionTerminate( + Reason.FAILED_APPLICATION, + String.format( + "%s only supports %s as a means to modify a not yet accepted %s", + BuildConfig.APP_NAME, + JinglePacket.Action.CONTENT_MODIFY, + JinglePacket.Action.CONTENT_ADD)); + } } private void receiveContentReject(final JinglePacket jinglePacket) { @@ -613,7 +638,7 @@ public class JingleRtpConnection extends AbstractJingleConnection "%s only supports %s as a means to retract a not yet accepted %s", BuildConfig.APP_NAME, JinglePacket.Action.CONTENT_REMOVE, - JinglePacket.Action.CONTENT_ACCEPT)); + JinglePacket.Action.CONTENT_ADD)); } } @@ -796,6 +821,8 @@ public class JingleRtpConnection extends AbstractJingleConnection // ICE-restart // and if that's the case we are seeing an answer. // This might be more spec compliant but also more error prone potentially + final boolean isSignalStateStable = this.webRTCWrapper.getSignalingState() == PeerConnection.SignalingState.STABLE; + // TODO a stable signal state can be another indicator that we have an offer to restart ICE final boolean isOffer = rtpContentMap.emptyCandidates(); final RtpContentMap restartContentMap; try { diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java index 08671ae03..45effba4d 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java @@ -13,14 +13,6 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Maps; import com.google.common.collect.Sets; -import java.util.Collection; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Set; - -import javax.annotation.Nonnull; - import eu.siacs.conversations.xmpp.jingle.stanzas.Content; import eu.siacs.conversations.xmpp.jingle.stanzas.GenericDescription; import eu.siacs.conversations.xmpp.jingle.stanzas.GenericTransportInfo; @@ -30,6 +22,14 @@ import eu.siacs.conversations.xmpp.jingle.stanzas.JinglePacket; import eu.siacs.conversations.xmpp.jingle.stanzas.OmemoVerifiedIceUdpTransportInfo; import eu.siacs.conversations.xmpp.jingle.stanzas.RtpDescription; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import javax.annotation.Nonnull; + public class RtpContentMap { public final Group group; @@ -94,7 +94,7 @@ public class RtpContentMap { } public Set getSenders() { - return ImmutableSet.copyOf(Collections2.transform(contents.values(),dt -> dt.senders)); + return ImmutableSet.copyOf(Collections2.transform(contents.values(), dt -> dt.senders)); } public List getNames() { @@ -300,6 +300,57 @@ public class RtpContentMap { dt -> new DescriptionTransport(senders, dt.description, dt.transport))); } + public RtpContentMap modifiedSendersChecked( + final boolean isInitiator, final Map modification) { + final ImmutableMap.Builder contentMapBuilder = + new ImmutableMap.Builder<>(); + for (final Map.Entry content : contents.entrySet()) { + final String id = content.getKey(); + final DescriptionTransport descriptionTransport = content.getValue(); + final Content.Senders currentSenders = descriptionTransport.senders; + final Content.Senders targetSenders = modification.get(id); + if (targetSenders == null || currentSenders == targetSenders) { + contentMapBuilder.put(id, descriptionTransport); + } else { + checkSenderModification(isInitiator, currentSenders, targetSenders); + contentMapBuilder.put( + id, + new DescriptionTransport( + targetSenders, + descriptionTransport.description, + descriptionTransport.transport)); + } + } + return new RtpContentMap(this.group, contentMapBuilder.build()); + } + + private static void checkSenderModification( + final boolean isInitiator, + final Content.Senders current, + final Content.Senders target) { + if (isInitiator) { + // we were both sending and now other party only wants to receive + if (current == Content.Senders.BOTH && target == Content.Senders.INITIATOR) { + return; + } + // only we were sending but now other party wants to send too + if (current == Content.Senders.INITIATOR && target == Content.Senders.BOTH) { + return; + } + } else { + // we were both sending and now other party only wants to receive + if (current == Content.Senders.BOTH && target == Content.Senders.RESPONDER) { + return; + } + // only we were sending but now other party wants to send too + if (current == Content.Senders.RESPONDER && target == Content.Senders.BOTH) { + return; + } + } + throw new IllegalArgumentException( + String.format("Unsupported senders modification %s -> %s", current, target)); + } + public RtpContentMap toContentModification(final Collection modifications) { return new RtpContentMap( this.group, @@ -323,7 +374,8 @@ public class RtpContentMap { } public RtpContentMap activeContents() { - return new RtpContentMap(group, Maps.filterValues(this.contents, dt -> dt.senders != Content.Senders.NONE)); + return new RtpContentMap( + group, Maps.filterValues(this.contents, dt -> dt.senders != Content.Senders.NONE)); } public Diff diff(final RtpContentMap rtpContentMap) { @@ -347,15 +399,32 @@ public class RtpContentMap { final IceUdpTransportInfo.Credentials credentials = getDistinctCredentials(); final Collection iceOptions = getCombinedIceOptions(); final DTLS dtls = getDistinctDtls(); - final IceUdpTransportInfo iceUdpTransportInfo = - IceUdpTransportInfo.of(credentials, iceOptions, setup, dtls.hash, dtls.fingerprint); final Map combined = merge(contents, modification.contents); final Map combinedFixedTransport = Maps.transformValues( combined, - dt -> - new DescriptionTransport( - dt.senders, dt.description, iceUdpTransportInfo)); + dt -> { + final IceUdpTransportInfo iceUdpTransportInfo; + if (dt.transport.emptyCredentials()) { + iceUdpTransportInfo = + IceUdpTransportInfo.of( + credentials, + iceOptions, + setup, + dtls.hash, + dtls.fingerprint); + } else { + iceUdpTransportInfo = + IceUdpTransportInfo.of( + dt.transport.getCredentials(), + iceOptions, + setup, + dtls.hash, + dtls.fingerprint); + } + return new DescriptionTransport( + dt.senders, dt.description, iceUdpTransportInfo); + }); return new RtpContentMap(modification.group, combinedFixedTransport); } diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java index 35251f929..a30e2ff91 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java @@ -110,6 +110,10 @@ public class IceUdpTransportInfo extends GenericTransportInfo { return new Credentials(ufrag, password); } + public boolean emptyCredentials() { + return Strings.isNullOrEmpty(this.getAttribute("ufrag")) || Strings.isNullOrEmpty(this.getAttribute("pwd")); + } + public List getCandidates() { final ImmutableList.Builder builder = new ImmutableList.Builder<>(); for (final Element child : getChildren()) {