Google Git
Sign in
webrtc/src/336f5fba8b^!/.
commit
336f5fba8bc1e2d99988552d225e767b7cac5d5d
[log]
author
Philipp Hancke <philipp.hancke@googlemail.com>
Mon Mar 23 07:04:12 2026
committer
WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com>
Tue Mar 24 07:00:51 2026
tree
4b7429e940b254073f1008cfdfd5262933e721d5
parent
c834307dfd27d63425738753f67a1e45237319c8 [diff]
snap: assert behavior when sctp-init changes due to pranswer

Also changes the SctpTransport::Start to a blocking call which allows
SetRemoteDescription to react to errors such as changing the sctp-init or sctp-port.

Bug: webrtc:426480601
Change-Id: Ifa8b97a4bbc055130c15293cda59a33281ad4f80
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/457901
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Philipp Hancke <philipp.hancke@googlemail.com>
Cr-Commit-Position: refs/heads/main@{#47237}

diff --git a/pc/data_channel_integrationtest.cc b/pc/data_channel_integrationtest.cc
index 3a4b280..cf7a5b7 100644
--- a/pc/data_channel_integrationtest.cc
+++ b/pc/data_channel_integrationtest.cc

@@ -1479,8 +1479,7 @@
   caller()->CreateAndSetAndSignalOffer();
   ASSERT_THAT(answer, NotNull());
 
-  // Currently SRD succeeds.
-  EXPECT_TRUE(caller()->SetRemoteDescription(std::move(answer)));
+  EXPECT_FALSE(caller()->SetRemoteDescription(std::move(answer)));
   // Check the state of the SCTP transport.
   VerifySctpState(caller(), SctpTransportState::kClosed);
 }

diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index e2a4ade..6270413 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc

@@ -3116,14 +3116,16 @@
   RTC_DCHECK_RUN_ON(signaling_thread());
   RTC_DCHECK(sctp_mid_s_);
 
-  network_thread()->PostTask(
-      SafeTask(network_thread_safety_, [this, mid = *sctp_mid_s_, options] {
-        scoped_refptr<SctpTransport> sctp_transport =
-            transport_controller_n()->GetSctpTransport(mid);
-        if (sctp_transport)
-          sctp_transport->Start(options);
-      }));
-  return RTCError::OK();
+  return network_thread()->BlockingCall([this, mid = *sctp_mid_s_, options] {
+    scoped_refptr<SctpTransport> sctp_transport =
+        transport_controller_n()->GetSctpTransport(mid);
+    RTC_DCHECK(sctp_transport);
+    if (!sctp_transport || !sctp_transport->Start(options)) {
+      LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_MODIFICATION,
+                           "Failed to start SCTP transport.");
+    }
+    return RTCError::OK();
+  });
 }
 
 CryptoOptions PeerConnection::GetCryptoOptions() {

diff --git a/pc/peer_connection.h b/pc/peer_connection.h
index 42387f8..15743e2 100644
--- a/pc/peer_connection.h
+++ b/pc/peer_connection.h

@@ -417,8 +417,9 @@
   bool CreateDataChannelTransport(absl::string_view mid) override;
   void DestroyDataChannelTransport(RTCError error) override;
 
-  // Asynchronously calls SctpTransport::Start() on the network thread for
-  // `sctp_mid()` if set. Called as part of setting the local description.
+  // Synchronously calls SctpTransport::Start() on the network thread for
+  // `sctp_mid()` if set. Called as part of pushing down the media descriptions
+  // after a complete offer/answer.
   RTCError StartSctpTransport(const SctpOptions& options) override;
 
   // Returns the CryptoOptions set as RTCConfiguration.crypto_options for this

diff --git a/pc/peer_connection_internal.h b/pc/peer_connection_internal.h
index 8314f7b..f31f999 100644
--- a/pc/peer_connection_internal.h
+++ b/pc/peer_connection_internal.h

@@ -127,8 +127,9 @@
       scoped_refptr<MediaStreamTrackInterface> track,
       const RtpTransceiverInit& init,
       bool fire_callback = true) = 0;
-  // Asynchronously calls SctpTransport::Start() on the network thread for
-  // `sctp_mid()` if set. Called as part of setting the local description.
+  // Synchronously calls SctpTransport::Start() on the network thread for
+  // `sctp_mid()` if set. Called as part of pushing down the media descriptions
+  // after a complete offer/answer.
   virtual RTCError StartSctpTransport(const SctpOptions& options) = 0;
 
   // Asynchronously adds a remote candidate on the network thread.

diff --git a/pc/sctp_transport.cc b/pc/sctp_transport.cc
index 063c855..abca8a1 100644
--- a/pc/sctp_transport.cc
+++ b/pc/sctp_transport.cc

@@ -176,7 +176,7 @@
   UpdateInformation(SctpTransportState::kClosed);
 }
 
-void SctpTransport::Start(const SctpOptions& options) {
+bool SctpTransport::Start(const SctpOptions& options) {
   RTC_DCHECK_RUN_ON(owner_thread_);
   info_ =
       SctpTransportInformation(info_.state(), info_.dtls_transport(),
@@ -185,7 +185,9 @@
   if (!internal()->Start(options)) {
     RTC_LOG(LS_ERROR) << "Failed to push down SCTP parameters, closing.";
     UpdateInformation(SctpTransportState::kClosed);
+    return false;
   }
+  return true;
 }
 
 void SctpTransport::UpdateInformation(SctpTransportState state) {

diff --git a/pc/sctp_transport.h b/pc/sctp_transport.h
index 32014aa..c6908a9 100644
--- a/pc/sctp_transport.h
+++ b/pc/sctp_transport.h

@@ -65,9 +65,9 @@
 
   // Internal functions
   void Clear();
-  // Initialize the webrtc::SctpTransport. This can be called from
+  // Initialize the webrtc::SctpTransport. This must be called from
   // the signaling thread.
-  void Start(const SctpOptions& options);
+  bool Start(const SctpOptions& options);
 
   // TODO(https://bugs.webrtc.org/10629): Move functions that need
   // internal() to be functions on the SctpTransport interface,

diff --git a/pc/sdp_offer_answer_unittest.cc b/pc/sdp_offer_answer_unittest.cc
index c216c89..a4a9c62 100644
--- a/pc/sdp_offer_answer_unittest.cc
+++ b/pc/sdp_offer_answer_unittest.cc

@@ -2011,30 +2011,30 @@
   auto pc2 = CreatePeerConnection("WebRTC-Sctp-Snap/Disabled/");
   EXPECT_TRUE(pc1->pc()->CreateDataChannelOrError("dc", nullptr).ok());
   auto offer = pc1->CreateOfferAndSetAsLocal();
-  ASSERT_NE(offer, nullptr);
+  ASSERT_THAT(offer, NotNull());
 
   {
     auto& contents = offer->description()->contents();
-    ASSERT_EQ(contents.size(), 1u);
+    ASSERT_THAT(contents, SizeIs(1));
     auto* media_description = contents[0].media_description();
-    ASSERT_TRUE(media_description);
+    ASSERT_THAT(media_description, NotNull());
     auto* sctp_description = media_description->as_sctp();
-    ASSERT_TRUE(sctp_description);
+    ASSERT_THAT(sctp_description, NotNull());
     EXPECT_FALSE(sctp_description->sctp_init());
   }
 
   RTCError error;
   EXPECT_TRUE(pc2->SetRemoteDescription(std::move(offer)));
   auto answer = pc2->CreateAnswerAndSetAsLocal();
-  ASSERT_NE(answer, nullptr);
+  ASSERT_THAT(answer, NotNull());
 
   {
     auto& contents = answer->description()->contents();
-    ASSERT_EQ(contents.size(), 1u);
+    ASSERT_THAT(contents, SizeIs(1));
     auto* media_description = contents[0].media_description();
-    ASSERT_TRUE(media_description);
+    ASSERT_THAT(media_description, NotNull());
     auto* sctp_description = media_description->as_sctp();
-    ASSERT_TRUE(sctp_description);
+    ASSERT_THAT(sctp_description, NotNull());
     EXPECT_FALSE(sctp_description->sctp_init());
   }
 
@@ -2046,7 +2046,7 @@
   auto pc2 = CreatePeerConnection("WebRTC-Sctp-Snap/Enabled/");
   EXPECT_TRUE(pc1->pc()->CreateDataChannelOrError("dc", nullptr).ok());
   auto offer = pc1->CreateOfferAndSetAsLocal();
-  ASSERT_NE(offer, nullptr);
+  ASSERT_THAT(offer, NotNull());
 
   {
     auto& contents = offer->description()->contents();
@@ -2061,7 +2061,7 @@
   RTCError error;
   EXPECT_TRUE(pc2->SetRemoteDescription(std::move(offer)));
   auto answer = pc2->CreateAnswerAndSetAsLocal();
-  ASSERT_NE(answer, nullptr);
+  ASSERT_THAT(answer, NotNull());
 
   {
     auto& contents = answer->description()->contents();
@@ -2101,7 +2101,7 @@
 
   EXPECT_TRUE(pc->SetRemoteDescription(std::move(desc)));
   auto answer = pc->CreateAnswerAndSetAsLocal();
-  ASSERT_NE(answer, nullptr);
+  ASSERT_THAT(answer, NotNull());
   EXPECT_TRUE(answer->ToString(&sdp));
 
   auto& contents = answer->description()->contents();
@@ -2134,6 +2134,31 @@
   auto desc = CreateSessionDescription(SdpType::kOffer, sdp);
   EXPECT_EQ(desc, nullptr);
 }
+
+TEST_F(SdpOfferAnswerTest,
+       AnswerFromNewPeerAfterProvisionalAnswerFailsSnapSctpInit) {
+  auto pc1 = CreatePeerConnection("WebRTC-Sctp-Snap/Enabled/");
+  auto pc2 = CreatePeerConnection("WebRTC-Sctp-Snap/Enabled/");
+  auto pc3 = CreatePeerConnection("WebRTC-Sctp-Snap/Enabled/");
+  EXPECT_TRUE(pc1->pc()->CreateDataChannelOrError("dc", nullptr).ok());
+  auto offer = pc1->CreateOfferAndSetAsLocal();
+  ASSERT_THAT(offer, NotNull());
+
+  EXPECT_TRUE(pc2->SetRemoteDescription(offer->Clone()));
+  EXPECT_TRUE(pc3->SetRemoteDescription(std::move(offer)));
+
+  auto answer2 = pc2->CreateAnswerAndSetAsLocal();
+  ASSERT_THAT(answer2, NotNull());
+  std::string sdp;
+  answer2->ToString(&sdp);
+  auto pranswer2 = CreateSessionDescription(SdpType::kPrAnswer, sdp);
+  EXPECT_TRUE(pc1->SetRemoteDescription(std::move(pranswer2)));
+
+  // Changing the sctp-init is not supported currently.
+  auto answer3 = pc3->CreateAnswerAndSetAsLocal();
+  ASSERT_THAT(answer3, NotNull());
+  EXPECT_FALSE(pc1->SetRemoteDescription(std::move(answer3)));
+}
 #endif  // WEBRTC_HAVE_SCTP
 
 class SdpOfferAnswerDirectionTest