search

songmonit / sipml5

Automatically exported from code.google.com/p/sipml5
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Settings ice_servers to [] does not disable ICE #187

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago 
What steps will reproduce the problem?
1. Create a SIPml.Stack, passing "ice_servers: []".
2. Set up a call.

What is the expected output? What do you see instead?

I expect it to disable ICE altogether, according to the documentation: 
http://sipml5.org/docgen/symbols/SIPml.Stack.Configuration.html#ice_servers

Instead, it falls back to the default: in the console I see 'ICE 
servers:[{"url":"stun:stun.l.google.com:19302"},{"url":"stun:stun.counterpath.ne
t:3478"},{"url":"stun:numb.viagenie.ca:3478"}]', and call setup is very slow.

What version of the product are you using? On what operating system?

The latest version (r220), in Google Chrome under Windows 8.1.

Please provide any additional information below.

The problem appears to be at tsip_stack.js line 720 
(https://code.google.com/p/sipml5/source/browse/trunk/src/tinySIP/src/tsip_stack
.js#720). Since tsk_string_is_null_or_empty([]) is true, it doesn't save it. So 
when tmedia_session_jsep.js looks for it, it doesn't find it, and falls back to 
the default.

It works correctly if I pass ice_servers as an empty array with its toString 
function overridden to return a non-empty string. Then 
tsk_string_is_null_or_empty returns false and it gets set correctly.

Original issue reported on code.google.com by micha...@connection-telecom.com on 23 Jul 2014 at 1:01

GoogleCodeExporter commented 9 years ago 
The same issue, but I can't fix it with empty array with redefined toString 
method. I have to wait about 10 seconds each time before call.

Original comment by dmitry...@gmail.com on 20 Jan 2015 at 4:07

GoogleCodeExporter commented 9 years ago 
I'm a little surprised this hasn't been fixed yet, considering it shouldn't be 
that difficult. In the source the ICE servers are indeed hardcoded into the 
array when it's empty. It is accompanied by a "HACK nightly" comment, so it 
shouldn't even be there in de stable version, I guess? Attached is a patch with 
my proposed fix.

Original comment by adriaa...@gmail.com on 24 Jan 2015 at 2:04

Attachments:

GoogleCodeExporter commented 9 years ago 
Just to be clear, I tested this patch in production, where the clients/server 
didn't reside behind a NAT, but on the same LAN, so I technically didn't need 
STUN. It may not work in other situations.

Original comment by adriaa...@gmail.com on 24 Jan 2015 at 2:07