Skip to content

Commit

Permalink
fix: Empty iceServers.urls handling fix
Browse files Browse the repository at this point in the history 
According to https://www.w3.org/TR/webrtc/#set-the-configuration browser
should throw SyntaxError when starting call with `iceServers` defined
with empty `urls`.

Talk may call `RTCPeerConnection` with empty `iceServers[0].urls` when
no STUN nor TURN servers are configured (tested with HPB) what makes
Firefox to throw errors like

`RTCPeerConnection constructor passed invalid RTCConfiguration - urls is empty SyntaxError`.

With this fix Talk will no longer generate STUN and TURN server settings
with empty (or invalid) `urls`.

Author-Change-Id: IB#1125985
Signed-off-by: Pawel Boguslawski <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
  • Loading branch information
@pboguslawski @backportbot
pboguslawski authored and backportbot[bot] committed Feb 5, 2025
1 parent 1495f50 commit 292150f
Show file tree
Hide file tree
Showing 2 changed files with 238 additions and 3 deletions.
16 changes: 13 additions & 3 deletions lib/Controller/SignalingController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,15 +164,25 @@ public function getSettings(string $token = ''): DataResponse {
$stunUrls = [];
$stunServers = $this->talkConfig->getStunServers();
foreach ($stunServers as $stunServer) {
if (empty($stunServer)) {
continue;
}

$stunUrls[] = 'stun:' . $stunServer;
}
$stun[] = [
'urls' => $stunUrls
];
if (!empty($stunUrls)) {
$stun[] = [
'urls' => $stunUrls
];
}

$turn = [];
$turnSettings = $this->talkConfig->getTurnSettings();
foreach ($turnSettings as $turnServer) {
if (empty($turnServer['schemes']) || empty($turnServer['server']) || empty($turnServer['protocols'])) {
continue;
}

$turnUrls = [];
$schemes = explode(',', $turnServer['schemes']);
$protocols = explode(',', $turnServer['protocols']);
Expand Down
225 changes: 225 additions & 0 deletions tests/php/Controller/SignalingControllerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,231 @@ private function recreateSignalingController() {
);
}

public static function dataGetSettingsStunServer(): array {
return [
[
[
'stun.nextcloud.com:443',
],
[
[
'urls' => [
'stun:stun.nextcloud.com:443',
]
],
],
],
[
[
'stun.nextcloud.com:443',
'stun.other.com:3478',
],
[
[
'urls' => [
'stun:stun.nextcloud.com:443',
'stun:stun.other.com:3478',
],
],
],
],
[
[
'',
],
[
],
],
[
[
],
[
],
],
];
}

/**
* @dataProvider dataGetSettingsStunServer
*/
public function testGetSettingsStunServer(array $stunServersConfig, array $expectedStunServers): void {
$this->config = $this->createMock(Config::class);
$this->recreateSignalingController();

$this->config->expects($this->once())
->method('getStunServers')
->willReturn($stunServersConfig);

$settings = $this->controller->getSettings()->getData();

// "stunservers" is always returned, even if empty
$this->assertArrayHasKey('stunservers', $settings);
$this->assertSame($expectedStunServers, $settings['stunservers']);
}

public static function dataGetSettingsTurnServer(): array {
return [
[
[
[
'schemes' => 'turn',
'server' => 'turn.example.org:3478',
'username' => 'theUserName',
'password' => 'thePassword',
'protocols' => 'udp',
],
],
[
[
'urls' => [
'turn:turn.example.org:3478?transport=udp',
],
'username' => 'theUserName',
'credential' => 'thePassword',
],
],
],
[
[
[
'schemes' => 'turn,turns',
'server' => 'turn.example.org:3478',
'username' => 'theUserName',
'password' => 'thePassword',
'protocols' => 'udp,tcp',
],
],
[
[
'urls' => [
'turn:turn.example.org:3478?transport=udp',
'turn:turn.example.org:3478?transport=tcp',
'turns:turn.example.org:3478?transport=udp',
'turns:turn.example.org:3478?transport=tcp',
],
'username' => 'theUserName',
'credential' => 'thePassword',
],
],
],
[
[
[
'schemes' => 'turn',
'server' => 'turn.example.org:3478',
'username' => 'theUserName1',
'password' => 'thePassword1',
'protocols' => 'udp,tcp',
],
[
'schemes' => 'turns',
'server' => 'turn.other.org:443',
'username' => 'theUserName2',
'password' => 'thePassword2',
'protocols' => 'tcp',
],
[
'schemes' => 'turn,turns',
'server' => 'turn.another.org:443',
'username' => 'theUserName3',
'password' => 'thePassword3',
'protocols' => 'udp',
],
],
[
[
'urls' => [
'turn:turn.example.org:3478?transport=udp',
'turn:turn.example.org:3478?transport=tcp',
],
'username' => 'theUserName1',
'credential' => 'thePassword1',
],
[
'urls' => [
'turns:turn.other.org:443?transport=tcp',
],
'username' => 'theUserName2',
'credential' => 'thePassword2',
],
[
'urls' => [
'turn:turn.another.org:443?transport=udp',
'turns:turn.another.org:443?transport=udp',
],
'username' => 'theUserName3',
'credential' => 'thePassword3',
],
],
],
[
// This would never happen, as the scheme is forced by
// "getTurnSettings" if empty, but it is added for completeness.
[
[
'schemes' => '',
'server' => 'turn.example.org:3478',
'username' => 'theUserName',
'password' => 'thePassword',
'protocols' => 'udp',
],
],
[
],
],
[
[
[
'schemes' => 'turn',
'server' => '',
'username' => 'theUserName',
'password' => 'thePassword',
'protocols' => 'udp',
],
],
[
],
],
[
[
[
'schemes' => 'turn',
'server' => 'turn.example.org:3478',
'username' => 'theUserName',
'password' => 'thePassword',
'protocols' => '',
],
],
[
],
],
[
[
],
[
],
],
];
}

/**
* @dataProvider dataGetSettingsTurnServer
*/
public function testGetSettingsTurnServer(array $turnServersConfig, array $expectedTurnServers): void {
$this->config = $this->createMock(Config::class);
$this->recreateSignalingController();

$this->config->expects($this->once())
->method('getTurnSettings')
->willReturn($turnServersConfig);

$settings = $this->controller->getSettings()->getData();

// "turnservers" is always returned, even if empty
$this->assertArrayHasKey('turnservers', $settings);
$this->assertSame($expectedTurnServers, $settings['turnservers']);
}

public static function dataIsTryingToPublishMedia(): array {
// For simplicity the SDP contains only the relevant fields and it is
// not a valid SDP
Expand Down

0 comments on commit 292150f

Please sign in to comment.