Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement new hostname resolving infrastructure #3127

merged 15 commits into from Jun 11, 2017


Copy link

commented Jun 10, 2017

This PR implements the new hostname resolving infrastructure in Mumble. This includes support for SRV records in Mumble.

The new resolver lives in the ServerResolver class. It now allows Mumble to also resolve SRV records.

The class isn't meant to be used exclusively for SRV records. We could add an alternative later on. For example, something I've toyed with is using a ".well-known" HTTPS URL, like as a "server pointer". The file at /.well-known/mumble would then link to a Mumble server (maybe something as simple as mumble://my-host:64800/). However, users would be able to connect to the server via mumble:// because of the pointer file. ...Just something I've toyed around with. Not related to this PR.

The PR changes ConnectDialog and ServerHandler to use ServerResolver for resolving hostnames of Mumble servers. That's where the bulk of the code changes happen.

It also introduces some new helper types: UnresolvedServerAddress (a hostname and a port), as well as a ServerAddress (an IP address and a port). The implementation of ServerResolver in ConnectDialog and ServerHandler makes use of these new types.

This changes the Mumble client to prefer SRV record. If no SRV record is available, the old code path is taken. (A/AAAA/CNAME).

For Qt 4, Mumble will not attempt do SRV resolving, since we rely on QtNetwork's DNS resolver. SRV resolving isn't available in Qt 4.

mkrautz added 15 commits Jun 10, 2017
ServerAddress: new struct for containing a HostAddress along with a p…
…ort number.

This is meant to replace the QPair-based qpAddress type used in ConnectDialog.
ConnectDialog: use ServerAddress for address list in ServerItem, and …
…for qhDNSCache.

This is preparation for the implementation of the ServerResolver class.
(For SRV record resolving.)

We now store address/port pairs for the DNS cache and list of resolved
addresses in ConnectDialog. This is because SRV records resolve to
address/port pairs.
TestServerResolver: modify test to work on systems that do not suppor…
…t both IPv4 and IPv6.

The initial test implementation was written targetting systems
that have both IPv4 and IPv6 available.

Qt's resolver does not return IPv6 results on systems that cannot
access the IPv6 internet. Nor does it return IPv4 results for systems
that cannot access the IPv4 internet.

Because of this, we modfiy the test case to be a little more
TestServerResolver: make the test Qt 4 compatible.
Qt 4 doesn't have QSignalSpy::wait().

This commit replaces calls to wait() with a similar construct
that works on Qt 4.

The implementation of QSignalSpy::wait() is quite simple:
It has its own QTestEventLoop and calls enterLoopMSecs()
on it. The return value of wait() is true if the count()
of the QSignalSpy changed since the last invocation.

Our construct is equivalent that that. Tests now pass on Qt 4.
ConnectDialog: update the connect dialog to use ServerResolver for ho…
…stname lookups.

Also, convert qlDNSLookup, qsDNSActive, qhDNSWait and qhDNSCache to use

This is necessary since we now expect DNS lookups to be keyed by
(hostname, port), not just hostname.
ServerHandler: make qtsSock a member variable.
This is in preparation of the integration of the ServerResolver
class into ServerHandler.
ServerHandler: ensure pings are only sent when in the Connected state.
Otherwise, we'll try to send pings before we've established a
connection to the server.
ServerHandler: use qsHostName as the hostname for the TLS handshake.
We want to use the SRV record's name in the TLS handshake.
ServerHandler: manually perform the TLS handshake.
This is necessary to be able to connect to an address/port
instead of hostname with QSslSocket.

@mkrautz mkrautz changed the title New hostname resolving infrastructure Implement new hostname resolving infrastructure Jun 11, 2017

@mkrautz mkrautz merged commit e913a44 into mumble-voip:master Jun 11, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
continuous-integration/travis-ci/pr The Travis CI build passed

This comment has been minimized.

Copy link

commented Jul 10, 2017

This is awesome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
3 participants
You can’t perform that action at this time.