fix(datachannel): send DataChannelOpen for pre-negotiated channels (closes #61)

rtc-datachannel/src/data_channel/data_channel_test.rs

@@ -208,6 +208,100 @@ fn test_data_channel_channel_type_reliable_ordered() -> Result<()> {
     Ok(())
 }
 
+/// Pre-negotiated (out-of-band) channels rely on the `SctpHandler` to suppress
+/// the DCEP DataChannelOpen payload on the wire.  This test verifies the
+/// `dial()` side of that contract: the outbound `DataChannelMessage` must carry
+/// `negotiated = true` so the SCTP handler knows to open the stream without
+/// sending the DCEP payload to the remote peer.
+///
+/// Note: the handler-side suppression is exercised by integration tests in the
+/// `rtc` crate; this unit test only covers the flagging produced by `dial()`.
+///
+/// Regression test for <https://github.com/webrtc-rs/rtc/issues/61>.
+#[test]
+fn test_data_channel_negotiated_dial_flags_message() -> Result<()> {
+    let (a0, a1) = create_new_association_pair()?;
+    let stream_id = 42;
+
+    let cfg = DataChannelConfig {
+        channel_type: ChannelType::Reliable,
+        negotiated: true,
+        label: "negotiated-ch".to_string(),
+        protocol: "test-proto".to_string(),
+        ..Default::default()
+    };
+
+    let mut dc = DataChannel::dial(cfg.clone(), a0, stream_id)?;
+
+    // dial() must still produce exactly one outbound DCEP message so the SCTP
+    // handler can register the stream.
+    let msg = dc.poll_write().ok_or(Error::ErrAssociationNotExisted)?;
+    assert_eq!(msg.ppi, PayloadProtocolIdentifier::Dcep);
+    assert_eq!(msg.stream_id, stream_id);
+
+    // The message must be flagged as negotiated so the SCTP handler opens the
+    // stream but does NOT send the DCEP payload to the peer.
+    assert!(
+        msg.negotiated,
+        "negotiated DataChannelOpen must be marked as internal-only"
+    );
+
+    // No further outbound messages should be queued.
+    assert!(
+        dc.poll_write().is_none(),
+        "negotiated dial should produce exactly one outbound message"
+    );
+
+    // Verify the config round-trips correctly.
+    assert_eq!(dc.config(), &cfg);
+
+    dc.close()?;
+    close_association_pair(a0, a1);
+
+    Ok(())
+}
+
+/// Non-negotiated (in-band) channels must send a regular DCEP DataChannelOpen
+/// that can be accepted by the peer.
+#[test]
+fn test_data_channel_non_negotiated_sends_dcep() -> Result<()> {
+    let (a0, a1) = create_new_association_pair()?;
+
+    let cfg = DataChannelConfig {
+        channel_type: ChannelType::Reliable,
+        negotiated: false,
+        label: "in-band".to_string(),
+        ..Default::default()
+    };
+
+    let mut dc0 = DataChannel::dial(cfg.clone(), a0, 100)?;
+
+    let msg = dc0.poll_write().ok_or(Error::ErrAssociationNotExisted)?;
+    assert_eq!(msg.ppi, PayloadProtocolIdentifier::Dcep);
+    // In-band channel: negotiated flag must be false so the SCTP handler sends
+    // the DCEP payload over the wire.
+    assert!(
+        !msg.negotiated,
+        "in-band DataChannelOpen must NOT be marked as internal-only"
+    );
+
+    // The peer should be able to accept this DCEP message.
+    let mut dc1 = DataChannel::accept(
+        DataChannelConfig::default(),
+        a1,
+        msg.stream_id,
+        PayloadProtocolIdentifier::Dcep,
+        &msg.payload,
+    )?;
+    assert_eq!(dc1.config(), &cfg, "remote config should match");
+
+    dc0.close()?;
+    dc1.close()?;
+    close_association_pair(a0, a1);
+
+    Ok(())
+}
+
 /*
 #[tokio::test]
 async fn test_data_channel_channel_type_reliable_unordered() -> Result<()> {

rtc-datachannel/src/data_channel/mod.rs

@@ -25,13 +25,41 @@ pub struct DataChannelConfig {
     pub protocol: String,
 }
 
-/// DataChannelMessage is used to data sent over SCTP
+/// DataChannelMessage is used for data sent over SCTP.
+// Note: #[non_exhaustive] is intentional — this crate is pre-1.0 (0.20.0-alpha)
+// and adding the `negotiated` field would otherwise be a semver-breaking change
+// for downstream code that constructs `DataChannelMessage` via struct literals.
+#[non_exhaustive]
 #[derive(Debug, Default, Clone)]
 pub struct DataChannelMessage {
     pub association_handle: usize,
     pub stream_id: u16,
     pub ppi: PayloadProtocolIdentifier,
     pub payload: BytesMut,
+    /// When `true`, the channel was created via out-of-band (pre-negotiated)
+    /// negotiation.  The SCTP handler opens the stream but does **not** send
+    /// the DCEP open message over the wire, because both peers already agree
+    /// on the channel parameters.
+    pub negotiated: bool,
+}
+
+impl DataChannelMessage {
+    /// Creates a new `DataChannelMessage` with all fields specified.
+    pub fn new(
+        association_handle: usize,
+        stream_id: u16,
+        ppi: PayloadProtocolIdentifier,
+        payload: BytesMut,
+        negotiated: bool,
+    ) -> Self {
+        Self {
+            association_handle,
+            stream_id,
+            ppi,
+            payload,
+            negotiated,
+        }
+    }
 }
 
 /// DataChannel represents a data channel
@@ -52,6 +80,8 @@ pub struct DataChannel {
 }
 
 impl DataChannel {
+    /// Creates a new `DataChannel` with the given configuration, association
+    /// handle, and SCTP stream identifier.  Counters start at zero.
     fn new(config: DataChannelConfig, association_handle: usize, stream_id: u16) -> Self {
         Self {
             config,
@@ -63,31 +93,47 @@ impl DataChannel {
         }
     }
 
-    /// Dial opens a data channels over SCTP
+    /// Dial opens a data channel over SCTP.
+    ///
+    /// A DCEP `DataChannelOpen` message is always constructed so that the SCTP
+    /// handler registers the underlying stream.  For **in-band** channels the
+    /// message is also sent over the wire (RFC 8832 sec. 3); for
+    /// **pre-negotiated** channels it stays internal-only (see issue #61).
     pub fn dial(
         config: DataChannelConfig,
         association_handle: usize,
         stream_id: u16,
     ) -> Result<Self> {
         let mut data_channel = DataChannel::new(config.clone(), association_handle, stream_id);
 
-        if !config.negotiated {
-            let msg = Message::DataChannelOpen(DataChannelOpen {
-                channel_type: config.channel_type,
-                priority: config.priority,
-                reliability_parameter: config.reliability_parameter,
-                label: config.label.bytes().collect(),
-                protocol: config.protocol.bytes().collect(),
-            })
-            .marshal()?;
-
-            data_channel.write_outs.push_back(DataChannelMessage {
-                association_handle,
-                stream_id,
-                ppi: PayloadProtocolIdentifier::Dcep,
-                payload: msg,
-            });
-        }
+        // Build a DCEP DataChannelOpen that the SCTP handler intercepts to
+        // register the underlying SCTP stream.
+        //
+        // For in-band channels (negotiated=false) the message is also sent over
+        // the wire to initiate the DCEP handshake per RFC 8832 sec. 3.
+        //
+        // For pre-negotiated channels (negotiated=true) the message is
+        // internal-only: the SCTP handler opens the stream and sets its
+        // reliability parameters but does NOT send DCEP to the peer.  Without
+        // this the SCTP association never registers the stream, causing every
+        // subsequent write to fail with "Stream not existed" (issue #61).
+        let msg = Message::DataChannelOpen(DataChannelOpen {
+            channel_type: config.channel_type,
+            priority: config.priority,
+            reliability_parameter: config.reliability_parameter,
+            label: config.label.bytes().collect(),
+            protocol: config.protocol.bytes().collect(),
+        })
+        .marshal()?;
+
+        data_channel.write_outs.push_back(DataChannelMessage {
+            association_handle,
+            stream_id,
+            ppi: PayloadProtocolIdentifier::Dcep,
+            payload: msg,
+            // Forward the negotiated flag so the SCTP handler can skip the DCEP write.
+            negotiated: config.negotiated,
+        });
 
         Ok(data_channel)
     }
@@ -189,6 +235,7 @@ impl DataChannel {
             stream_id: self.stream_id,
             ppi: PayloadProtocolIdentifier::Dcep,
             payload: ack,
+            ..Default::default()
         });
         Ok(())
     }
@@ -200,6 +247,7 @@ impl DataChannel {
             stream_id: self.stream_id,
             ppi: PayloadProtocolIdentifier::Dcep,
             payload: close,
+            ..Default::default()
         });
         Ok(())
     }
@@ -212,6 +260,7 @@ impl DataChannel {
             stream_id: self.stream_id,
             ppi: PayloadProtocolIdentifier::Dcep,
             payload: low_threshold,
+            ..Default::default()
         });
         Ok(())
     }
@@ -224,6 +273,7 @@ impl DataChannel {
             stream_id: self.stream_id,
             ppi: PayloadProtocolIdentifier::Dcep,
             payload: low_threshold,
+            ..Default::default()
         });
         Ok(())
     }