Default stun server ip address should be changed to a domain name

RESOLVED FIXED in mozilla27



5 years ago
5 years ago


(Reporter: standard8, Assigned: standard8)



Firefox Tracking Flags

(Not tracked)



(1 attachment, 1 obsolete attachment)



5 years ago
The services team have set up a stun network and we should start using that in Firefox, rather than the hard-coded ip address.

I believe the new network address is:

Mark Mayo: please can you confirm this is correct and we should be using this as the default in Firefox from now on?
Flags: needinfo?(mmayo)

Comment 1

5 years ago
On a quick survey, I believe the following files need to be updated:

> dom/media/PeerConnection.js:285:   * { "iceServers": [ { url:"stun:" },
> media/mtransport/test/ice_unittest.cpp:48:const std::string kDefaultStunServerAddress((char *)"");
> media/mtransport/test/ice_unittest.cpp:50:    (char *)"");
> media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:493: * { "iceServers": [ { url:"stun:" },
> media/webrtc/signaling/test/signaling_unittests.cpp:54:std::string g_stun_server_address((char *)"");
> modules/libpref/src/init/all.js:232:pref("media.peerconnection.default_iceservers", "[{\"url\": \"stun:\"}]");

Some of these hardcode IP addresses out of necessity (e.g., the ice unittest runs some tests which necessarily operate below the layer that performs name resolution). I presume these should be updated to one of the IP addresses that the DNS name resolves to? Should we use any one in particular, or just choose one?

Comment 2

5 years ago
Confirmed, is the correct hostname to use.
Flags: needinfo?(mmayo)

Comment 3

5 years ago
Created attachment 819603 [details] [diff] [review]
Update code examples

This patch updates just the Firefox default stun servers pref and leaves the tests unaffected.

I would suggest that we have a separate bug to address the stun servers for tests - I can see possibilities for releng wanting to handling things differently, or possibly retaining the status-quo.
Attachment #819603 - Flags: review?(adam)

Comment 4

5 years ago
Comment on attachment 819603 [details] [diff] [review]
Update code examples

Review of attachment 819603 [details] [diff] [review]:

Looks good to me.

::: dom/media/PeerConnection.js
@@ +331,4 @@
>    /**
>     * An RTCConfiguration looks like this:
>     *
> +   * { "iceServers": [ { url:"" },

It's purely editorial, but I think this would make more sense as Your call.

::: media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ +556,4 @@
>  /**
>   * In JS, an RTCConfiguration looks like this:
>   *
> + * { "iceServers": [ { url:"" },

Same comment as above.
Attachment #819603 - Flags: review?(adam) → review+

Comment 5

5 years ago
Created attachment 820882 [details] [diff] [review]
Update code examples v2

You're right, that's a much better example for the comments.
Assignee: nobody → mbanner
Attachment #819603 - Attachment is obsolete: true
Attachment #820882 - Flags: review+
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.