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

RESOLVED FIXED in mozilla27

Status

()

RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

unspecified
mozilla27
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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?
Flags: needinfo?(mmayo)

Comment 1

6 years ago
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?
Confirmed, stun.services.mozilla.com is the correct hostname to use.
Flags: needinfo?(mmayo)
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:"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.
Attachment #819603 - Flags: review?(adam) → review+
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
Status: NEW → ASSIGNED
Attachment #820882 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/5503a6548d3d
Status: ASSIGNED → RESOLVED
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.