Closed Bug 920991 Opened 9 years ago Closed 9 years ago

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


(Core :: WebRTC, defect)

Not set





(Reporter: standard8, Assigned: standard8)



(1 file, 1 obsolete file)

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)
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?
Confirmed, is the correct hostname to use.
Flags: needinfo?(mmayo)
Attached patch Update code examples (obsolete) — Splinter Review
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 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+
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+
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.