shinyoshiaki / werift-webrtc

WebRTC Implementation for TypeScript (Node.js), includes ICE/DTLS/SCTP/RTP/SRTP/WEBM/MP4
MIT License
443 stars 28 forks source link

`parseIceServers` returns undefined turnServer and stunServer #385

Open nandito opened 1 month ago

nandito commented 1 month ago

The RTCIceGatherer's ice server config has undefined value for each key:

https://github.com/shinyoshiaki/werift-webrtc/blob/develop/packages/webrtc/src/peerConnection.ts#L485

{
  stunServer: undefined,
  turnServer: undefined,
  turnUsername: undefined,
  turnPassword: undefined
}

I think this happens as the parseIceServers util (https://github.com/shinyoshiaki/werift-webrtc/blob/develop/packages/webrtc/src/peerConnection.ts#L485) tries to use the .includes("turn:") and .includes("stun:") on arrays.

So we have this function call in the utils.ts:

const turnServer = url2Address(
  iceServers.find(({ urls }) => urls.includes("turn:"))?.urls.slice(5),
);

where the iceServers looks like this:

 [
  {
    url: 'turn:turn-myturnserver.com:443',
    urls: ['turn:turn-myturnserver.com:443'],
    username: 'username',
    credential: 'creds='
  }
]

If we inspect the url2Address parameter:

iceServers.find(({urls}) => urls.includes("turn:"))

We can see that it runs the includes() fn on an array, not on a string.

['turn:turn-myturnserver.com:443'].includes("turn:")

This is always false, therefore the find() won't find any turn: in this case.

I assume a nested find() function would fix this issue.

nandito commented 1 month ago

As I see urls can be a string or an array of strings:

urls This required property is either a single string or an array of strings, each specifying a URL which can be used to connect to the server.

So the current implementation is also correct, but not 100% complete as it does not handle the array of strings.

shinyoshiaki commented 1 month ago

I will eventually fix it, but PullRequests are also welcome