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

RESOLVED FIXED in mozilla27

Status

()

Core
WebRTC
RESOLVED FIXED
5 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)

(Assignee)

Description

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:

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

5 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?

Comment 2

5 years ago
Confirmed, stun.services.mozilla.com is the correct hostname to use.
Flags: needinfo?(mmayo)
(Assignee)

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:"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+
(Assignee)

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