diff --git a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt index c6af2d4462..7a45a2e1b3 100644 --- a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt @@ -2485,7 +2485,11 @@ class CallActivity : CallBaseActivity() { iceServers, tempSdpConstraints, sessionId, - callSession, + // #6169: the remote sessionId is a signaling-layer session ID when external signaling + // is used, so the offerer election must compare same-namespace IDs. Pass the local + // signaling session (matching the currentSessionId resolution used elsewhere); with + // internal signaling webSocketClient == null, so callSession is used as before. + if (webSocketClient != null) webSocketClient!!.sessionId else callSession, tempLocalStream, tempIsMCUPublisher, tempHasMCU, diff --git a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java index ba726753d1..484ad34460 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java +++ b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java @@ -116,7 +116,7 @@ public PeerConnectionWrapper(PeerConnectionFactory peerConnectionFactory, this.mediaConstraints = mediaConstraints; sdpObserver = new SdpObserver(); - boolean hasInitiated = sessionId.compareTo(localSession) < 0; + boolean hasInitiated = isOfferer(localSession, sessionId); this.isMCUPublisher = isMCUPublisher; PeerConnection.RTCConfiguration configuration = new PeerConnection.RTCConfiguration(iceServerList); @@ -165,6 +165,30 @@ public PeerConnectionWrapper(PeerConnectionFactory peerConnectionFactory, } } + /** + * Decides whether this client takes the offerer role for a peer connection. + * + *

The role is resolved purely by lexicographic comparison of the two session IDs: + * the client whose remote session ID sorts before its own local session ID becomes the + * offerer. This yields exactly one offerer per pair only if both peers compare the + * same ordered pair of strings. + * + *

Bug #6169: with the High-Performance Backend the remote {@code sessionId} is a + * signaling-layer session ID. Callers must therefore pass a {@code localSession} from the + * same namespace (the local signaling session, not the Nextcloud session); otherwise + * the two peers compare unrelated strings and can both compute {@code false} (neither offers — + * deadlock) or both {@code true} (offer collision). The {@code requestoffer} fallback only + * applies in MCU mode, so a direct (no-MCU) 2-party call deadlocks outright. The deadlock is + * exercised by {@code PeerConnectionWrapperOffererRoleTest}. + * + * @param localSession this client's own session ID (same namespace as {@code remoteSession}) + * @param remoteSession the remote peer's session ID + * @return true if this client should create the initial offer + */ + static boolean isOfferer(String localSession, String remoteSession) { + return remoteSession.compareTo(localSession) < 0; + } + public void raiseHand(Boolean raise) { NCMessagePayload ncMessagePayload = new NCMessagePayload(); ncMessagePayload.setState(raise); diff --git a/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperOffererRoleTest.kt b/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperOffererRoleTest.kt new file mode 100644 index 0000000000..897433920e --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperOffererRoleTest.kt @@ -0,0 +1,64 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.webrtc + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotEquals +import org.junit.Test + +/** + * Tests for the offerer-role election ([PeerConnectionWrapper.isOfferer]) — bug #6169. + * + * Contract: for a pair of peers, exactly one must create the initial offer. The role is resolved + * by a lexicographic comparison of the two session IDs, which yields exactly one offerer **only + * if both peers compare the same ordered pair of strings** — i.e. the local and remote session + * IDs are in the same namespace. + * + * #6169: in High-Performance Backend mode the remote `sessionId` is a signaling-layer session ID. + * The fix passes a local signaling session ID (not the Nextcloud session ID) at the construction + * site so both peers compare the same pair. These tests pin both the desired contract and the + * failure mode that occurs if cross-namespace IDs are ever passed again. + */ +class PeerConnectionWrapperOffererRoleTest { + + /** Roles computed by BOTH peers for one consistent (same-namespace) pair of session IDs. */ + private fun rolesForConsistentPair(sessionA: String, sessionB: String): Pair { + val aIsOfferer = PeerConnectionWrapper.isOfferer(sessionA, sessionB) + val bIsOfferer = PeerConnectionWrapper.isOfferer(sessionB, sessionA) + return aIsOfferer to bIsOfferer + } + + @Test + fun exactlyOneOfferer_whenBothPeersCompareTheSameSessionIdPair() { + val (a, b) = rolesForConsistentPair("session-aaa", "session-bbb") + assertNotEquals("exactly one of the two peers must be the offerer", a, b) + } + + @Test + fun offererRoleIsComplementary_forAnyConsistentPair_regardlessOfInsertionOrder() { + assertEquals(rolesForConsistentPair("x", "y"), rolesForConsistentPair("x", "y")) + val (a, b) = rolesForConsistentPair("alpha", "omega") + assertNotEquals(a, b) + } + + /** + * Regression guard for the failure mode of #6169: if cross-namespace IDs are passed (each peer + * comparing its own Nextcloud session against the other's HPB session — four unrelated + * strings), both peers can compute the same role. Here both compute "not offerer" → nobody + * offers → deadlock. The fix prevents this by passing same-namespace IDs at the call site. + */ + @Test + fun bothPeersComputeNotOfferer_whenCrossNamespaceSessionIdsArePassed() { + val aIsOfferer = PeerConnectionWrapper.isOfferer("nc-A", "sig-B") + val bIsOfferer = PeerConnectionWrapper.isOfferer("nc-B", "sig-A") + + assertFalse("peer A does not take the offerer role", aIsOfferer) + assertFalse("peer B does not take the offerer role", bIsOfferer) + assertEquals("both peers compute the same role = deadlock (#6169)", aIsOfferer, bIsOfferer) + } +}