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

Fix SRV port bugs in ServerResolver and ConnectDialog #3268

Merged

Conversation

@mkrautz
Copy link
Member

commented Nov 26, 2017

This PR includes two fixes for SRV port issues.

The first fix is for ServerResolver. The Qt 5 (SRV) version had a bug where resolved ServerResolverRecords would be created with m_origPort. In practice, this meant that Mumble would always use the port specified in the ConnectDialog record for the server. Typically, this would be 64738, the default port.

The second fix is somewhat related to the first. The ConnectDialog had a bug where it would use ServerResolver::port() instead of ServerResolverRecord::port() when constructing the list of resolved addresses for a ConnectDialog entry. Unfortunately, ServerResolver::port() is the "original port". That is, the port specified in the ConnectDialog entry. Once again, this meant that Mumble would display the wrong port to user: the one from the ConnectDialog entry instead of the resolved port. Typically, this would be the default port (64738).

Fixes #3267

ServerResolver: fix bug where ServerResolver_qt5 would always pass on…
… the original port given to the resolver.

Instead of using the port from the QDnsServiceRecord, the
srvResolved() slot used m_origPort. Oops.

Fixes #3267

@mkrautz mkrautz requested review from Kissaki, hacst and davidebeatrici Nov 26, 2017

ConnectDialog: use port from ServerResolverRecord instead of original…
… port from ServerResolver::port().

This caused the ConnectDialog to show the wrong resolved port
for servers resolved using SRV records.

@mkrautz mkrautz force-pushed the mkrautz:serverresolver-fix-custom-port-bug branch from bc8c158 to 7acf687 Nov 26, 2017

@mkrautz mkrautz merged commit 48f3eb9 into mumble-voip:master Nov 26, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.