From 8ea52766c1095823d0c6911e0f42bdb7f9b315c6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20=C5=9Aled=C5=BA?= <michalsledz34@gmail.com>
Date: Wed, 15 Jan 2025 13:03:38 +0100
Subject: [PATCH] Don't forward user's RTP header extensions in `send_rtp/4`

---
 lib/ex_webrtc/peer_connection.ex              | 26 ++++++++++++++++---
 .../peer_connection/configuration.ex          |  7 ++++-
 test/ex_webrtc/peer_connection_test.exs       |  8 +++++-
 3 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/lib/ex_webrtc/peer_connection.ex b/lib/ex_webrtc/peer_connection.ex
index b2c1c20d..29abb4a7 100644
--- a/lib/ex_webrtc/peer_connection.ex
+++ b/lib/ex_webrtc/peer_connection.ex
@@ -142,6 +142,16 @@ defmodule ExWebRTC.PeerConnection do
     GenServer.call(peer_connection, {:controlling_process, controlling_process})
   end
 
+  @doc """
+  Returns current `peer_connection` configuration.
+
+  Note: the configuration may change after applying remote description.
+  """
+  @spec get_configuration(peer_connection()) :: Configuration.t()
+  def get_configuration(peer_connection) do
+    GenServer.call(peer_connection, :get_configuration)
+  end
+
   @doc """
   Sends an RTP packet to the remote peer using the track specified by the `track_id`.
 
@@ -561,6 +571,11 @@ defmodule ExWebRTC.PeerConnection do
     {:reply, :ok, state}
   end
 
+  @impl true
+  def handle_call(:get_configuration, _from, state) do
+    {:reply, state.config, state}
+  end
+
   @impl true
   def handle_call(:get_connection_state, _from, state) do
     {:reply, state.conn_state, state}
@@ -1097,6 +1112,12 @@ defmodule ExWebRTC.PeerConnection do
     |> Enum.find(fn {tr, _idx} -> tr.sender.track && tr.sender.track.id == track_id end)
     |> case do
       {tr, idx} ->
+        # When RTP packet comes from another peer connection,
+        # it can already have some extensions, in particular,
+        # extensions that we didn't negotiate (consider simulcast and rid).
+        # Remove these extensions and add ours.
+        packet = ExRTP.Packet.remove_extensions(packet)
+
         {packet, state} =
           case state.twcc_extension_id do
             nil ->
@@ -1107,10 +1128,7 @@ defmodule ExWebRTC.PeerConnection do
                 ExRTP.Packet.Extension.TWCC.new(state.sent_packets)
                 |> ExRTP.Packet.Extension.TWCC.to_raw(id)
 
-              packet =
-                packet
-                |> ExRTP.Packet.remove_extension(id)
-                |> ExRTP.Packet.add_extension(twcc)
+              packet = ExRTP.Packet.add_extension(packet, twcc)
 
               state = %{state | sent_packets: state.sent_packets + 1 &&& 0xFFFF}
               {packet, state}
diff --git a/lib/ex_webrtc/peer_connection/configuration.ex b/lib/ex_webrtc/peer_connection/configuration.ex
index 51f3adc0..49cf012c 100644
--- a/lib/ex_webrtc/peer_connection/configuration.ex
+++ b/lib/ex_webrtc/peer_connection/configuration.ex
@@ -158,7 +158,12 @@ defmodule ExWebRTC.PeerConnection.Configuration do
           rtcp_feedbacks: [rtcp_feedback()]
         ]
 
-  @typedoc false
+  @typedoc """
+  `ExWebRTC.PeerConnection` configuration.
+
+  It is created from options passed to `ExWebRTC.PeerConnection.start_link/1`.
+  See `t:options/0` for more.
+  """
   @type t() :: %__MODULE__{
           controlling_process: Process.dest(),
           ice_servers: [ice_server()],
diff --git a/test/ex_webrtc/peer_connection_test.exs b/test/ex_webrtc/peer_connection_test.exs
index 6300d0e5..adb16f3b 100644
--- a/test/ex_webrtc/peer_connection_test.exs
+++ b/test/ex_webrtc/peer_connection_test.exs
@@ -227,8 +227,13 @@ defmodule ExWebRTC.PeerConnectionTest do
     :ok = negotiate(pc1, pc2)
     :ok = connect(pc1, pc2)
 
+    config = PeerConnection.get_configuration(pc1)
+    negotiated_ext_ids = Enum.map(config.audio_extensions ++ config.video_extensions, & &1.id)
+    non_negotiated_ext_id = Enum.random(Enum.to_list(1..255) -- negotiated_ext_ids)
+
     payload = <<3, 2, 5>>
-    packet = ExRTP.Packet.new(payload)
+    non_negotiated_ext = %ExRTP.Packet.Extension{id: non_negotiated_ext_id, data: "0"}
+    packet = payload |> ExRTP.Packet.new() |> ExRTP.Packet.add_extension(non_negotiated_ext)
 
     # Try to send data using correct track id.
     # Assert that pc2 received this data and send_rtp didn't modify some of the fields.
@@ -237,6 +242,7 @@ defmodule ExWebRTC.PeerConnectionTest do
     assert recv_packet.payload == packet.payload
     assert recv_packet.sequence_number == packet.sequence_number
     assert recv_packet.timestamp == packet.timestamp
+    assert non_negotiated_ext not in recv_packet.extensions
 
     # Try to send data using invalid track id.
     # Assert that pc1 is still alive and pc2 didn't receive this data.