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

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Mark Banner (:standard8)]

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:

stun.services.mozilla.com

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

Comment 1

[Adam Roach [:abr]]

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

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

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

[Mark Mayo [:mmayo]]

Confirmed, stun.services.mozilla.com is the correct hostname to use.

Comment 3

[Mark Banner (:standard8)]

Attachment: 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.

Comment 4

[Adam Roach [:abr]]

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:"stun:my.stun.server.example.org" },

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

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

Same comment as above.

Comment 5

[Mark Banner (:standard8)]

Attachment: Update code examples v2

You're right, that's a much better example for the comments.

Comment 6

[Mark Banner (:standard8)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5503a6548d3d

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5503a6548d3d