From 4bda78d4269cffdf174b2ee70c0d061b22beaa5e Mon Sep 17 00:00:00 2001 From: Tony Mountifield Date: Mon, 9 Nov 2026 15:19:15 +0000 Subject: [PATCH 1/5] Convert host and SRV lookups to async instead of blocking Also use direct parsing of literal IP addresses. --- src/client.cpp | 63 +++++---- src/client.h | 2 +- src/clientdlg.cpp | 33 +++-- src/clientdlg.h | 1 + src/clientrpc.cpp | 36 ++++- src/connectdlg.cpp | 56 +++++--- src/connectdlg.h | 1 + src/serverdlg.cpp | 33 +++-- src/serverdlg.h | 3 + src/serverlist.cpp | 86 ++++++++---- src/serverlist.h | 3 + src/util.cpp | 328 ++++++++++++++++++++++++++++----------------- src/util.h | 38 +++++- 13 files changed, 459 insertions(+), 224 deletions(-) diff --git a/src/client.cpp b/src/client.cpp index 9b5e502bee..645da46bd2 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -619,25 +619,6 @@ void CClient::SetRemoteChanPan ( const int iId, const float fPan ) StartTimerGainOrPan(); } -bool CClient::SetServerAddr ( QString strNAddr ) -{ - CHostAddress HostAddress; - if ( NetworkUtil::ParseNetworkAddress ( strNAddr, HostAddress, bIPv6Available ) ) - { - // apply address to the channel - Channel.SetAddress ( HostAddress ); - - // By default, set server name to HostAddress. If using the Connect() method, this may be overwritten - SetConnectedServerName ( HostAddress.toString() ); - - return true; - } - else - { - return false; // invalid address - } -} - bool CClient::GetAndResetbJitterBufferOKFlag() { // get the socket buffer put status flag and reset it @@ -1162,7 +1143,7 @@ void CClient::Disconnect() /// @method /// @brief Connects to strServerAddress. If a connection is currently requested /// or established, that connection is terminated first. -/// @emit Connecting (strServerName) if SetServerAddr was valid. emit happens through Start(). +/// @emit Connecting (strServerName) if address was valid. emit happens through Start(). /// Use to set CClientDlg to show being connected /// @emit ConnectingFailed (error) if an error occurred /// Use to display error message in CClientDlg @@ -1176,24 +1157,54 @@ void CClient::Connect ( const QString& strServerAddress, const QString& strServe // different server while connected behaves as a reconnect Disconnect(); - // Set server address and connect if valid address was supplied - if ( SetServerAddr ( strServerAddress ) ) + if ( NetworkUtil::ResolveAddress ( strServerAddress, bIPv6Available, this, &CClient::OnHostResolved, true, true ) ) { - SetConnectedServerName ( strServerName ); - Start(); + SetConnectedServerName ( strServerName ); // set if resolution initiated ok } else { - throw CGenErr ( tr ( "Received invalid server address. Please check for typos in the provided server address." ) ); + emit ConnectingFailed ( tr ( "Received invalid server address. Please check for typos in the provided server address." ) ); } } catch ( const CGenErr& generr ) { - Stop(); + // Stop(); // XXX: probably not needed? emit ConnectingFailed ( generr.GetErrorText() ); } } +void CClient::OnHostResolved ( const QHostInfo& info, quint16 port ) +{ + if ( info.error() != QHostInfo::NoError ) + { + emit ConnectingFailed ( "Cannot connect: " + info.errorString() ); + return; + } + + for ( const QHostAddress& HostAddr : info.addresses() ) + { + // qInfo() << "Resolved network address to" << HostAddr << "for proto" << HostAddr.protocol(); + if ( HostAddr.protocol() == QAbstractSocket::IPv4Protocol || ( HostAddr.protocol() == QAbstractSocket::IPv6Protocol && bIPv6Available ) ) + { + CHostAddress HostAddress ( HostAddr, port ); + Channel.SetAddress ( HostAddress ); + // sound device start can throw an exception, which we must catch here + try + { + Start(); + } + catch ( const CGenErr& generr ) + { + Stop(); + emit ConnectingFailed ( generr.GetErrorText() ); + } + return; + } + } + + emit ConnectingFailed ( "Hostname returned no usable addresses" ); +} + /// @method /// @brief Updates the connection state and, if it changed, notifies observers. /// @emit ConnectionStateChanged (state) when the state actually changes diff --git a/src/client.h b/src/client.h index 1c4c4e5428..d692df8fe4 100644 --- a/src/client.h +++ b/src/client.h @@ -180,7 +180,6 @@ class CClient : public QObject bool IsRunning() { return Sound.IsRunning(); } bool IsCallbackEntered() const { return Sound.IsCallbackEntered(); } - bool SetServerAddr ( QString strNAddr ); // IPv6 Available bool IsIPv6Available() { return bIPv6Available; } @@ -514,6 +513,7 @@ protected slots: void OnMuteStateHasChangedReceived ( int iServerChanID, bool bIsMuted ); void OnCLChannelLevelListReceived ( CHostAddress InetAddr, CVector vecLevelList ); void OnConClientListMesReceived ( CVector vecChanInfo ); + void OnHostResolved ( const QHostInfo& info, quint16 port ); signals: void ConClientListMesReceived ( CVector vecChanInfo ); diff --git a/src/clientdlg.cpp b/src/clientdlg.cpp index 2c7c35f02c..34e95ff2dd 100644 --- a/src/clientdlg.cpp +++ b/src/clientdlg.cpp @@ -605,21 +605,38 @@ CClientDlg::CClientDlg ( CClient* pNCliP, // that the connection less message respond may not make it back but that // is not critical since the next time Jamulus is started we have another // chance and the update check is not time-critical at all) - CHostAddress UpdateServerHostAddress; - - // Send the request to two servers for redundancy if either or both of them - // has a higher release version number, the reply will trigger the notification. // Don't use SRV resolution when resolving update servers. + if ( !NetworkUtil::ResolveAddress ( UPDATECHECK1_ADDRESS, pClient->IsIPv6Available(), this, &CClientDlg::OnHostResolved, true, false ) ) + { + qWarning() << "Cannot start update check for" << UPDATECHECK1_ADDRESS; + } + + if ( !NetworkUtil::ResolveAddress ( UPDATECHECK2_ADDRESS, pClient->IsIPv6Available(), this, &CClientDlg::OnHostResolved, true, false ) ) + { + qWarning() << "Cannot start update check for" << UPDATECHECK2_ADDRESS; + } +} - if ( NetworkUtil::ParseNetworkAddressBare ( UPDATECHECK1_ADDRESS, UpdateServerHostAddress, pClient->IsIPv6Available() ) ) +void CClientDlg::OnHostResolved ( const QHostInfo& info, quint16 port ) +{ + if ( info.error() != QHostInfo::NoError ) { - pClient->CreateCLServerListReqVerAndOSMes ( UpdateServerHostAddress ); + qWarning() << "Update lookup failed:" << info.errorString(); + return; } - if ( NetworkUtil::ParseNetworkAddressBare ( UPDATECHECK2_ADDRESS, UpdateServerHostAddress, pClient->IsIPv6Available() ) ) + for ( const QHostAddress& HostAddr : info.addresses() ) { - pClient->CreateCLServerListReqVerAndOSMes ( UpdateServerHostAddress ); + // qInfo() << "Resolved network address to" << HostAddr << "for proto" << HostAddr.protocol(); + if ( HostAddr.protocol() == QAbstractSocket::IPv4Protocol || + ( HostAddr.protocol() == QAbstractSocket::IPv6Protocol && pClient->IsIPv6Available() ) ) + { + CHostAddress HostAddress ( HostAddr, port ); + + pClient->CreateCLServerListReqVerAndOSMes ( HostAddress ); + return; + } } } diff --git a/src/clientdlg.h b/src/clientdlg.h index b127231b27..b7da510e05 100644 --- a/src/clientdlg.h +++ b/src/clientdlg.h @@ -260,6 +260,7 @@ public slots: void SetMixerBoardDeco ( const ERecorderState newRecorderState, const EGUIDesign eNewDesign ); void OnAudioChannelsChanged() { UpdateRevSelection(); } void OnNumClientsChanged ( int iNewNumClients ); + void OnHostResolved ( const QHostInfo& info, quint16 port ); void accept() { close(); } // introduced by pljones diff --git a/src/clientrpc.cpp b/src/clientrpc.cpp index 67f46a9843..ed8d728c7a 100644 --- a/src/clientrpc.cpp +++ b/src/clientrpc.cpp @@ -187,18 +187,42 @@ CClientRpc::CClientRpc ( CClient* pClient, CClientSettings* pSettings, CRpcServe return; } - CHostAddress haDirectoryAddress; - - // Allow IPv4 only for communicating with Directories - if ( !NetworkUtil::ParseNetworkAddress ( jsonDirectoryIp.toString(), haDirectoryAddress, false ) ) + // Start asynchronous address resolution with lambda callback slot + bool accepted = NetworkUtil::ResolveAddress ( + jsonDirectoryIp.toString(), + false, // Currently IPv4 only for directories + pClient, + [pClient] ( const QHostInfo& info, quint16 port ) { + // this callback runs later, after the RPC handler has returned + if ( info.error() != QHostInfo::NoError ) + { + qWarning() << "Directory lookup failed:" << info.errorString(); + return; + } + + // select a usable IPv4 address + for ( const QHostAddress& addr : info.addresses() ) + { + if ( addr.protocol() == QAbstractSocket::IPv4Protocol ) + { + CHostAddress haDirectoryAddress ( addr, port ); + + pClient->CreateCLReqServerListMes ( haDirectoryAddress ); + return; + } + } + qWarning() << "No usable address found for directory"; + }, + true, // allow hostname lookup + true ); // allow SRV lookup + + if ( !accepted ) { response["error"] = CRpcServer::CreateJsonRpcError ( CRpcServer::iErrInvalidParams, "Invalid params: directory is not a valid socket address" ); return; } - // send the request for the server list - pClient->CreateCLReqServerListMes ( haDirectoryAddress ); response["result"] = "ok"; } ); diff --git a/src/connectdlg.cpp b/src/connectdlg.cpp index 103d4d9193..9b5a9595e2 100644 --- a/src/connectdlg.cpp +++ b/src/connectdlg.cpp @@ -312,24 +312,48 @@ void CConnectDlg::RequestServerList() } cbxDirectory->blockSignals ( false ); - // Get the IP address of the directory server (using the ParseNetworkAddress + // Get the IP address of the directory server (using the async ResolveAddress // function) when the connect dialog is opened, this seems to be the correct // time to do it. Note that in case of custom directories we // use iCustomDirectoryIndex as an index into the vector. - // Allow IPv4 only for communicating with Directories - if ( NetworkUtil::ParseNetworkAddress ( - NetworkUtil::GetDirectoryAddress ( pSettings->eDirectoryType, pSettings->vstrDirectoryAddress[pSettings->iCustomDirectoryIndex] ), - haDirectoryAddress, - false ) ) + // Currently allow only IPv4 for communicating with Directories + + const QString strDirectoryAddress = + NetworkUtil::GetDirectoryAddress ( pSettings->eDirectoryType, pSettings->vstrDirectoryAddress[pSettings->iCustomDirectoryIndex] ); + + if ( !NetworkUtil::ResolveAddress ( strDirectoryAddress, pClient->IsIPv6Available(), this, &CConnectDlg::OnHostResolved, true, true ) ) { - // send the request for the server list - emit ReqServerListQuery ( haDirectoryAddress ); + qWarning() << "Cannot start directory fetch from" << strDirectoryAddress; + } +} - // start timer, if this message did not get any respond to retransmit - // the server list request message - TimerReRequestServList.start ( SERV_LIST_REQ_UPDATE_TIME_MS ); - TimerInitialSort.start ( SERV_LIST_REQ_UPDATE_TIME_MS ); // reuse the time value +void CConnectDlg::OnHostResolved ( const QHostInfo& info, quint16 port ) +{ + if ( info.error() != QHostInfo::NoError ) + { + qWarning() << "Directory lookup failed:" << info.errorString(); + return; + } + + for ( const QHostAddress& HostAddr : info.addresses() ) + { + // qInfo() << "Resolved network address to" << HostAddr << "for proto" << HostAddr.protocol(); + if ( HostAddr.protocol() == QAbstractSocket::IPv4Protocol /* || + ( HostAddr.protocol() == QAbstractSocket::IPv6Protocol && pClient->IsIPv6Available() ) */ ) + { + haDirectoryAddress = CHostAddress ( HostAddr, port ); + + // send the request for the server list + emit ReqServerListQuery ( haDirectoryAddress ); + + // start timer, if this message did not get any respond to retransmit + // the server list request message + TimerReRequestServList.start ( SERV_LIST_REQ_UPDATE_TIME_MS ); + TimerInitialSort.start ( SERV_LIST_REQ_UPDATE_TIME_MS ); // reuse the time value + + return; + } } } @@ -855,10 +879,10 @@ void CConnectDlg::OnTimerPing() // try to parse host address string which is stored as user data // in the server list item GUI control element - // the data to be parsed is just IP:port, so no SRV discovery is needed - if ( NetworkUtil::ParseNetworkAddressBare ( pCurListViewItem->data ( LVC_NAME, Qt::UserRole ).toString(), - haServerAddress, - pClient->IsIPv6Available() ) ) + // the data to be parsed is just IP:port, so no hostname lookup is needed + if ( NetworkUtil::ParseAddressLiteral ( pCurListViewItem->data ( LVC_NAME, Qt::UserRole ).toString(), + pClient->IsIPv6Available(), + haServerAddress ) ) { // if address is valid, send ping message using a new thread #if QT_VERSION >= QT_VERSION_CHECK( 6, 0, 0 ) diff --git a/src/connectdlg.h b/src/connectdlg.h index 0b2eb8d7ae..cf77a2b9f6 100644 --- a/src/connectdlg.h +++ b/src/connectdlg.h @@ -157,6 +157,7 @@ public slots: void OnTimerPing(); void OnTimerReRequestServList(); void OnCurrentServerItemChanged ( QTreeWidgetItem* current, QTreeWidgetItem* previous ); + void OnHostResolved ( const QHostInfo& info, quint16 port ); signals: void ReqServerListQuery ( CHostAddress InetAddr ); diff --git a/src/serverdlg.cpp b/src/serverdlg.cpp index 5e1dbe5274..cc41324a0d 100644 --- a/src/serverdlg.cpp +++ b/src/serverdlg.cpp @@ -499,21 +499,38 @@ CServerDlg::CServerDlg ( CServer* pNServP, CServerSettings* pNSetP, const bool b // that the connection less message respond may not make it back but that // is not critical since the next time Jamulus is started we have another // chance and the update check is not time-critical at all) - CHostAddress UpdateServerHostAddress; - - // Send the request to two servers for redundancy if either or both of them - // has a higher release version number, the reply will trigger the notification. // Don't use SRV resolution when resolving update servers. + if ( !NetworkUtil::ResolveAddress ( UPDATECHECK1_ADDRESS, pServer->IsIPv6Available(), this, &CServerDlg::OnHostResolved, true, false ) ) + { + qWarning() << "Cannot start update check for" << UPDATECHECK1_ADDRESS; + } - if ( NetworkUtil::ParseNetworkAddressBare ( UPDATECHECK1_ADDRESS, UpdateServerHostAddress, pServer->IsIPv6Available() ) ) + if ( !NetworkUtil::ResolveAddress ( UPDATECHECK2_ADDRESS, pServer->IsIPv6Available(), this, &CServerDlg::OnHostResolved, true, false ) ) { - pServer->CreateCLServerListReqVerAndOSMes ( UpdateServerHostAddress ); + qWarning() << "Cannot start update check for" << UPDATECHECK2_ADDRESS; } +} - if ( NetworkUtil::ParseNetworkAddressBare ( UPDATECHECK2_ADDRESS, UpdateServerHostAddress, pServer->IsIPv6Available() ) ) +void CServerDlg::OnHostResolved ( const QHostInfo& info, quint16 port ) +{ + if ( info.error() != QHostInfo::NoError ) { - pServer->CreateCLServerListReqVerAndOSMes ( UpdateServerHostAddress ); + qWarning() << "Update lookup failed:" << info.errorString(); + return; + } + + for ( const QHostAddress& HostAddr : info.addresses() ) + { + // qinfo() << "Resolved network address to" << HostAddr << "for proto" << HostAddr.protocol(); + if ( HostAddr.protocol() == QAbstractSocket::IPv4Protocol || + ( HostAddr.protocol() == QAbstractSocket::IPv6Protocol && pServer->IsIPv6Available() ) ) + { + CHostAddress HostAddress ( HostAddr, port ); + + pServer->CreateCLServerListReqVerAndOSMes ( HostAddress ); + return; + } } } diff --git a/src/serverdlg.h b/src/serverdlg.h index 80d427a4fe..38cc5ee8b7 100644 --- a/src/serverdlg.h +++ b/src/serverdlg.h @@ -149,4 +149,7 @@ public slots: // Our timer void OnTimer(); + + // From ResolveAddress() + void OnHostResolved ( const QHostInfo& info, quint16 port ); }; diff --git a/src/serverlist.cpp b/src/serverlist.cpp index 0dafa73863..023bd0de31 100644 --- a/src/serverlist.cpp +++ b/src/serverlist.cpp @@ -87,15 +87,15 @@ CServerListEntry CServerListEntry::parse ( QString strHAddr, bool bIPv6Available ) { CHostAddress haServerHostAddr; - NetworkUtil::ParseNetworkAddress ( strHAddr, haServerHostAddr, bIPv6Available ); - if ( CHostAddress() == haServerHostAddr ) + CHostAddress haServerLocalAddr; + + if ( !NetworkUtil::ParseAddressLiteral ( strHAddr, bIPv6Available, haServerHostAddr ) || + !NetworkUtil::ParseAddressLiteral ( strLHAddr, bIPv6Available, haServerLocalAddr ) || CHostAddress() == haServerHostAddr ) { // do not proceed without server host address! return CServerListEntry(); } - CHostAddress haServerLocalAddr; - NetworkUtil::ParseNetworkAddress ( strLHAddr, haServerLocalAddr, bIPv6Available ); if ( haServerLocalAddr.iPort == 0 ) { haServerLocalAddr.iPort = haServerHostAddr.iPort; @@ -827,9 +827,13 @@ bool CServerListManager::Load() continue; } - // This uses ParseNetworkAddressBare because it is just parsing ip:host that was saved to the file. - // Therefore no SRV lookup is appropriate. - NetworkUtil::ParseNetworkAddressBare ( slLine[0], haServerHostAddr, pServer->IsIPv6Available() ); + // This uses ParseAddressLiteral because it is just parsing ip:host that was saved to the file. + if ( !NetworkUtil::ParseAddressLiteral ( slLine[0], pServer->IsIPv6Available(), haServerHostAddr ) ) + { + qWarning() << "Skipping invalid host entry for" << slLine[0]; + continue; + } + int iIdx = IndexOf ( haServerHostAddr ); if ( iIdx != INVALID_INDEX ) { @@ -999,6 +1003,18 @@ void CServerListManager::SetRegistered ( const bool bIsRegister ) return; } + // If we are unregistering from a directory we previously registered to, + // just use the saved address from registering, without address resolution. + + if ( !bIsRegister ) + { + // unregister server if it is registered - regardless of success of parse + SetSvrRegStatus ( SRS_NOT_REGISTERED ); + + pConnLessProtocol->CreateCLUnregisterServerMes ( DirectoryAddress ); + return; + } + // It is very important to unlock the Mutex before doing address resolution, // so that the event loop can run and any other timers that need the mutex // can obtain it. Otherwise there is the possibility of deadlock when doing @@ -1011,39 +1027,53 @@ void CServerListManager::SetRegistered ( const bool bIsRegister ) // changed in the meanwhile. // Allow IPv4 only for communicating with Directories // Use SRV DNS discovery for directory connections, fallback to A/AAAA if none. - const QString strNetworkAddress = NetworkUtil::GetDirectoryAddress ( DirectoryType, strDirectoryAddress ); - const bool bDirectoryAddressValid = NetworkUtil::ParseNetworkAddress ( strNetworkAddress, DirectoryAddress, false ); + const QString strNetworkAddress = NetworkUtil::GetDirectoryAddress ( DirectoryType, strDirectoryAddress ); + + // allow hostname resolution and SRV resolution + const bool bDirectoryAddressValid = + NetworkUtil::ResolveAddress ( strNetworkAddress, false, this, &CServerListManager::OnHostResolved, true, true ); // lock the mutex again now that the address has been resolved. locker.relock(); - if ( bIsRegister ) + if ( !bDirectoryAddressValid ) { - if ( bDirectoryAddressValid ) - { - // register server - SetSvrRegStatus ( SRS_REQUESTED ); + SetSvrRegStatus ( SRS_BAD_ADDRESS ); + } +} - // For a registered server, the server properties are stored in the - // very first item in the server list (which is actually no server list - // but just one item long for the registered server). - pConnLessProtocol->CreateCLRegisterServerExMes ( DirectoryAddress, ServerList[0].LHostAddr, ServerList[0] ); - } - else +void CServerListManager::OnHostResolved ( const QHostInfo& info, quint16 port ) +{ + QMutexLocker locker ( &Mutex ); + + if ( info.error() == QHostInfo::NoError ) + { + for ( const QHostAddress& HostAddr : info.addresses() ) { - SetSvrRegStatus ( SRS_BAD_ADDRESS ); + // qInfo() << "Resolved network address to" << HostAddr << "for proto" << HostAddr.protocol(); + if ( HostAddr.protocol() == QAbstractSocket::IPv4Protocol /* || + ( HostAddr.protocol() == QAbstractSocket::IPv6Protocol && pServer->IsIPv6Available() ) */ ) + { + DirectoryAddress = CHostAddress ( HostAddr, port ); + + // register server + SetSvrRegStatus ( SRS_REQUESTED ); + + // For a registered server, the server properties are stored in the + // very first item in the server list (which is actually no server list + // but just one item long for the registered server). + pConnLessProtocol->CreateCLRegisterServerExMes ( DirectoryAddress, ServerList[0].LHostAddr, ServerList[0] ); + return; + } } + qWarning() << "Directory lookup failed: Hostname returned no usable addresses"; } else { - // unregister server if it is registered - regardless of success of parse - SetSvrRegStatus ( SRS_NOT_REGISTERED ); - - if ( bDirectoryAddressValid ) - { - pConnLessProtocol->CreateCLUnregisterServerMes ( DirectoryAddress ); - } + qWarning() << "Directory lookup failed:" << info.errorString(); } + + SetSvrRegStatus ( SRS_BAD_ADDRESS ); } void CServerListManager::SetSvrRegStatus ( ESvrRegStatus eNSvrRegStatus ) diff --git a/src/serverlist.h b/src/serverlist.h index f18c40678d..afb3ad2b82 100644 --- a/src/serverlist.h +++ b/src/serverlist.h @@ -256,6 +256,9 @@ public slots: void OnAboutToQuit(); + // From ResolveAddress() + void OnHostResolved ( const QHostInfo& info, quint16 port ); + signals: void SvrRegStatusChanged(); }; diff --git a/src/util.cpp b/src/util.cpp index 39efa10a16..637418dac3 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -855,112 +855,57 @@ void PlayAudioAlert ( QUrl soundUrl ) * Other Classes * \******************************************************************************/ // Network utility functions --------------------------------------------------- -bool NetworkUtil::ParseNetworkAddressString ( QString strAddress, QHostAddress& InetAddr, const bool bIPv6Available ) -{ - // try to get host by name, assuming - // that the string contains a valid host name string or IP address - const QHostInfo HostInfo = QHostInfo::fromName ( strAddress ); - - if ( HostInfo.error() != QHostInfo::NoError ) - { - // qInfo() << qUtf8Printable ( QString ( "Invalid hostname" ) ); - return false; // invalid address - } - - foreach ( const QHostAddress HostAddr, HostInfo.addresses() ) - { - // qInfo() << qUtf8Printable ( QString ( "Resolved network address to %1 for proto %2" ) .arg ( HostAddr.toString() ) .arg ( - // HostAddr.protocol() ) ); - if ( HostAddr.protocol() == QAbstractSocket::IPv4Protocol || ( bIPv6Available && HostAddr.protocol() == QAbstractSocket::IPv6Protocol ) ) - { - InetAddr = HostAddr; - return true; - } - } - return false; -} -#ifndef DISABLE_SRV_DNS -bool NetworkUtil::ParseNetworkAddressSrv ( QString strAddress, CHostAddress& HostAddress, const bool bIPv6Available ) +static void resolveHostname ( const QString& strHostname, + const quint16 port, + QObject* context, + std::function slot ) { - // init requested host address with invalid address first - HostAddress = CHostAddress(); - - QRegularExpression plainHostRegex ( "^([^\\[:0-9.][^:]*)$" ); - if ( plainHostRegex.match ( strAddress ).capturedStart() != 0 ) - { - // not a plain hostname? then don't attempt SRV lookup and fail - // immediately. - return false; - } - - QDnsLookup* dns = new QDnsLookup(); - dns->setType ( QDnsLookup::SRV ); - dns->setName ( QString ( "_jamulus._udp.%1" ).arg ( strAddress ) ); - dns->lookup(); - // QDnsLookup::lookup() works asynchronously. Therefore, wait for - // it to complete here by resuming the main loop here. - // This is not nice and blocks the UI, but is similar to what - // the regular resolve function does as well. - QTime dieTime = QTime::currentTime().addMSecs ( DNS_SRV_RESOLVE_TIMEOUT_MS ); - while ( QTime::currentTime() < dieTime && !dns->isFinished() ) - { - QCoreApplication::processEvents ( QEventLoop::ExcludeUserInputEvents, 100 ); - } - QList records = dns->serviceRecords(); - dns->deleteLater(); - if ( records.length() != 1 ) - { - return false; - } - QDnsServiceRecord record = records.first(); - if ( record.target() == "." || record.target() == "" ) - { - // RFC2782 says that "." indicates that the service is not available. - // Qt strips the trailing dot, which is why we check for empty string - // as well. Therefore, the "." part might be redundant, but this would - // need further testing to confirm. - // End processing here (= return true), but pass back an - // invalid HostAddress to let the connect logic fail properly. - HostAddress = CHostAddress ( QHostAddress ( "." ), 0 ); - return true; - } - qDebug() << qUtf8Printable ( - QString ( "resolved %1 to a single SRV record: %2:%3" ).arg ( strAddress ).arg ( record.target() ).arg ( record.port() ) ); - - QHostAddress InetAddr; - if ( ParseNetworkAddressString ( record.target(), InetAddr, bIPv6Available ) ) - { - HostAddress = CHostAddress ( InetAddr, record.port() ); - return true; - } - return false; + QHostInfo::lookupHost ( strHostname, context, [slot, port] ( const QHostInfo& info ) { slot ( info, port ); } ); } -#endif -bool NetworkUtil::ParseNetworkAddress ( QString strAddress, CHostAddress& HostAddress, const bool bIPv6Available ) +static bool validateHostnameFormat ( const QString& strHostname ) { -#ifndef DISABLE_SRV_DNS - // Try SRV-based discovery first: - if ( ParseNetworkAddressSrv ( strAddress, HostAddress, bIPv6Available ) ) - { - return true; - } -#endif - // Try regular connect via plain IP or host name lookup (A/AAAA): - return ParseNetworkAddressBare ( strAddress, HostAddress, bIPv6Available ); + // Validate hostname format before trying to resolve: + // (all matches are non-greedy) + static const QRegularExpression plainHostRegex ( + // # beginning of string + "^" + // # doesn't contain underscore (this is so that we can use \w later on for [A-Za-z0-9]) + "(?!.*?_)" + // # doesn't contain two dots in a row + "(?!.*?\\.\\.)" + // # doesn't have only digits in the last element (also eliminates raw IP addresses) + "(?!.*\\.\\d+\\.?$)" + // # doesn't contain dash at the beginning of a label + "(?!(?:\\w+?\\.)?-[\\w\\.\\-]*?)" + // # doesn't contain dash at the end of a label + "(?!\\w+?-\\.(?:[\\w\\.\\-]+?))" + // # starts with an alphanumeric char + "(?=\\w)" + // # not longer than 253 chars + "(?![\\w\\.\\-]{254})" + // # doesn't contain a label longer than 63 char + "(?!.*?[\\w\\-]{64})" + // # allowed chars alphanumeric, dot and dash + "[\\w\\.\\-]+?" + // # end of string + "$" ); + + auto match = plainHostRegex.match ( strHostname ); + + return match.hasMatch(); } -bool NetworkUtil::ParseNetworkAddressBare ( QString strAddress, CHostAddress& HostAddress, const bool bIPv6Available ) +static bool parseHostAndPort ( const QString& strAddress, bool& bLiteralAddrOut, QString& strHostOut, QString& strPortOut, quint16& iNetPortOut ) { - QHostAddress InetAddr; - unsigned int iNetPort = DEFAULT_PORT_NUMBER; + quint16 iNetPort = DEFAULT_PORT_NUMBER; + QString strHost; + QString strPort; + bool bLiteralAddr = false; // qInfo() << qUtf8Printable ( QString ( "Parsing network address %1" ).arg ( strAddress ) ); - // init requested host address with invalid address first - HostAddress = CHostAddress(); - // Allow the following address formats: // [addr4or6] // [addr4or6]:port @@ -970,79 +915,212 @@ bool NetworkUtil::ParseNetworkAddressBare ( QString strAddress, CHostAddress& Ho // hostname:port // (where addr4or6 is a literal IPv4 or IPv6 address, and addr4 is a literal IPv4 address - bool bLiteralAddr = false; - QRegularExpression rx1 ( "^\\[([^]]*)\\](?::(\\d+))?$" ); // [addr4or6] or [addr4or6]:port - QRegularExpression rx2 ( "^([^:]*)(?::(\\d+))?$" ); // addr4 or addr4:port or host or host:port - - QString strPort; + static const QRegularExpression rx1 ( "^\\[([^]]+)\\](?::(\\d+))?$" ); // [addr4or6] or [addr4or6]:port + static const QRegularExpression rx2 ( "^([^:]+)(?::(\\d+))?$" ); // addr4 or addr4:port or host or host:port QRegularExpressionMatch rx1match = rx1.match ( strAddress ); QRegularExpressionMatch rx2match = rx2.match ( strAddress ); // parse input address with rx1 and rx2 in turn, capturing address/host and port - if ( rx1match.capturedStart() == 0 ) + if ( rx1match.hasMatch() ) { // literal address within [] - strAddress = rx1match.captured ( 1 ); + strHost = rx1match.captured ( 1 ); strPort = rx1match.captured ( 2 ); bLiteralAddr = true; // don't allow hostname within [] } - else if ( rx2match.capturedStart() == 0 ) + else if ( rx2match.hasMatch() ) { // hostname or IPv4 address - strAddress = rx2match.captured ( 1 ); - strPort = rx2match.captured ( 2 ); + strHost = rx2match.captured ( 1 ); + strPort = rx2match.captured ( 2 ); } else { // invalid format - // qInfo() << qUtf8Printable ( QString ( "Invalid address format" ) ); + // qInfo() << "Invalid address format"; return false; } if ( !strPort.isEmpty() ) { // a port number was given: extract port number - iNetPort = strPort.toInt(); + bool ok; + unsigned int port = strPort.toUInt ( &ok ); - if ( iNetPort >= 65536 ) + if ( !ok || port == 0 || port >= 65536 ) { // invalid port number - // qInfo() << qUtf8Printable ( QString ( "Invalid port number specified" ) ); + // qInfo() << "Invalid port number specified"; return false; } + + iNetPort = static_cast ( port ); } - // first try if this is an IP number an can directly applied to QHostAddress - if ( InetAddr.setAddress ( strAddress ) ) + // valid split and port number + bLiteralAddrOut = bLiteralAddr; + strHostOut = strHost; + strPortOut = strPort; + iNetPortOut = iNetPort; + return true; +} + +// parse literal address and optional port with no hostname lookup and address returned directly +bool NetworkUtil::ParseAddressLiteral ( const QString& strAddress, const bool bIPv6Available, CHostAddress& haHostAddress ) +{ + QHostAddress InetAddr; + quint16 iNetPort; + QString strHost; + QString strPort; + bool bLiteralAddr; + + if ( !parseHostAndPort ( strAddress, bLiteralAddr, strHost, strPort, iNetPort ) ) + { + return false; + } + + // try if this is an IP number and can directly applied to QHostAddress + if ( InetAddr.setAddress ( strHost ) ) { - if ( !bIPv6Available && InetAddr.protocol() == QAbstractSocket::IPv6Protocol ) + if ( InetAddr.protocol() == QAbstractSocket::IPv6Protocol && !bIPv6Available ) { // do not allow IPv6 addresses if not enabled - // qInfo() << qUtf8Printable ( QString ( "IPv6 addresses disabled" ) ); + // qInfo() << "IPv6 addresses disabled"; return false; } + + haHostAddress = CHostAddress ( InetAddr, iNetPort ); + return true; } - else + + // qInfo() << "Invalid literal IP address"; + return false; // invalid address +} + +// ResolveAddress() with resolved address(es) returned asynchronously in a slot function +bool NetworkUtil::ResolveAddress ( const QString& strAddress, + const bool bIPv6Available, + QObject* context, + std::function slot, + const bool allowHostname, + const bool allowSrv ) +{ + QHostAddress InetAddr; + quint16 iNetPort; + QString strHost; + QString strPort; + bool bLiteralAddr; + + if ( !parseHostAndPort ( strAddress, bLiteralAddr, strHost, strPort, iNetPort ) ) { - // it was no valid IP address. If literal required, return as invalid - if ( bLiteralAddr ) - { - // qInfo() << qUtf8Printable ( QString ( "Invalid literal IP address" ) ); - return false; // invalid address - } + return false; + } - if ( !ParseNetworkAddressString ( strAddress, InetAddr, bIPv6Available ) ) + // first try if this is an IP number and can directly applied to QHostAddress + if ( InetAddr.setAddress ( strHost ) ) + { + if ( InetAddr.protocol() == QAbstractSocket::IPv6Protocol && !bIPv6Available ) { - // no valid address found - // qInfo() << qUtf8Printable ( QString ( "No IP address found for hostname" ) ); + // do not allow IPv6 addresses if not enabled + // qInfo() << "IPv6 addresses disabled"; return false; } + + QHostInfo info; + info.setHostName ( strHost ); + info.setAddresses ( { InetAddr } ); + info.setError ( QHostInfo::NoError ); + + // call the slot directly with IP and port + QMetaObject::invokeMethod ( + context, + [slot, info, iNetPort]() { slot ( info, iNetPort ); }, + Qt::QueuedConnection ); + + return true; + } + else if ( bLiteralAddr ) + { + // it was not a valid IP address. If literal required due to [], return as invalid + // qInfo() << "Invalid literal IP address"; + return false; // invalid address + } + else if ( !allowHostname ) + { + // not allowing hostname lookup + // qInfo() << "Hostname lookup disabled"; + return false; + } + else if ( !validateHostnameFormat ( strHost ) ) + { + // invalid format for hostname + // qInfo() << "Hostname format invalid"; + return false; } +#ifndef DISABLE_SRV_DNS + else if ( allowSrv && strPort.isEmpty() ) + { + // qInfo() << "Looking up SRV record for" << strHost; + + // check for SRV record to obtain hostname and port + QDnsLookup* dns = new QDnsLookup ( context ); + dns->setType ( QDnsLookup::SRV ); + dns->setName ( QString ( "_jamulus._udp.%1" ).arg ( strHost ) ); + + QObject::connect ( dns, &QDnsLookup::finished, context, [strHost, iNetPort, context, dns, slot]() { + if ( dns->error() == QDnsLookup::NoError ) + { + const QList records = dns->serviceRecords(); + if ( records.length() == 1 ) + { + QDnsServiceRecord record = records.first(); + qInfo() << "SRV record found: target =" << record.target() << "port =" << record.port(); + + dns->deleteLater(); + + if ( record.target().isEmpty() || record.target() == "." ) + { + // RFC2782 says that "." indicates that the service is not available. + // Qt strips the trailing dot, which is why we check for empty string + // as well. Therefore, the "." part might be redundant, but this would + // need further testing to confirm. + + // SRV says unavailable - create a QHostInfo to pass error to the slot + QHostInfo info; + info.setHostName ( strHost ); + info.setError ( QHostInfo::HostNotFound ); + info.setErrorString ( "SRV record indicates service unavailable" ); + + QMetaObject::invokeMethod ( + context, + [slot, info, iNetPort]() { slot ( info, iNetPort ); }, + Qt::QueuedConnection ); + return; + } + + resolveHostname ( record.target(), record.port(), context, slot ); + return; + } + else + { + qWarning() << "SRV returned" << records.length() << "records"; + } + } + + dns->deleteLater(); - // qInfo() << qUtf8Printable ( QString ( "Parsed network address %1" ).arg ( InetAddr.toString() ) ); + // SRV failed - lookup host directly + resolveHostname ( strHost, iNetPort, context, slot ); + } ); + + dns->lookup(); + + return true; + } +#endif - HostAddress = CHostAddress ( InetAddr, iNetPort ); + resolveHostname ( strHost, iNetPort, context, slot ); return true; } diff --git a/src/util.h b/src/util.h index 08e45166cb..01e271c71b 100644 --- a/src/util.h +++ b/src/util.h @@ -48,6 +48,7 @@ #include #include #include +#include #ifdef _WIN32 # include # include @@ -1102,13 +1103,38 @@ class CNetworkTransportProps class NetworkUtil { public: - static bool ParseNetworkAddressString ( QString strAddress, QHostAddress& InetAddr, const bool bIPv6Available ); + // parse literal address and optional port with no hostname lookup and address returned directly + static bool ParseAddressLiteral ( const QString& strAddress, const bool bIPv6Available, CHostAddress& haHostAddress ); -#ifndef DISABLE_SRV_DNS - static bool ParseNetworkAddressSrv ( QString strAddress, CHostAddress& HostAddress, const bool bIPv6Available ); -#endif - static bool ParseNetworkAddress ( QString strAddress, CHostAddress& HostAddress, const bool bIPv6Available ); - static bool ParseNetworkAddressBare ( QString strAddress, CHostAddress& HostAddress, const bool bIPv6Available ); + // resolve address and optional port with possible hostname and/or SRC lookup, + // with resolved address(es) returned asynchronously in a slot function + + // ResolveAddress() with object method as slot + template + static bool ResolveAddress ( const QString& strAddress, + const bool bIPv6Available, + T* context, + void ( T::*slot ) ( const QHostInfo& info, quint16 port ), + const bool allowHostname = false, + const bool allowSrv = false ) + { + // this is not a recursive call, as the parameter types are different + return ResolveAddress ( + strAddress, + bIPv6Available, + context, + [context, slot] ( const QHostInfo& info, quint16 port ) { ( context->*slot ) ( info, port ); }, + allowHostname, + allowSrv ); + } + + // ResolveAddress() with static method or function as slot + static bool ResolveAddress ( const QString& strAddress, + const bool bIPv6Available, + QObject* context, + std::function slot, + const bool allowHostname = false, + const bool allowSrv = false ); static QString FixAddress ( const QString& strAddress ); static CHostAddress GetLocalAddress(); From c3e897aacd40772abe294bde93e132bda975b0e6 Mon Sep 17 00:00:00 2001 From: Tony Mountifield Date: Mon, 9 Nov 2026 16:35:08 +0000 Subject: [PATCH 2/5] Fix typo --- src/util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util.h b/src/util.h index 01e271c71b..cff2a544da 100644 --- a/src/util.h +++ b/src/util.h @@ -1106,7 +1106,7 @@ class NetworkUtil // parse literal address and optional port with no hostname lookup and address returned directly static bool ParseAddressLiteral ( const QString& strAddress, const bool bIPv6Available, CHostAddress& haHostAddress ); - // resolve address and optional port with possible hostname and/or SRC lookup, + // resolve address and optional port with possible hostname and/or SRV lookup, // with resolved address(es) returned asynchronously in a slot function // ResolveAddress() with object method as slot From 4c42f3432192844e3a89eb27c3a9ed16c87e20ce Mon Sep 17 00:00:00 2001 From: Tony Mountifield Date: Mon, 9 Nov 2026 16:37:04 +0000 Subject: [PATCH 3/5] Remove unused #define --- src/util.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/util.h b/src/util.h index cff2a544da..f338d4ce0e 100644 --- a/src/util.h +++ b/src/util.h @@ -108,9 +108,8 @@ class CClient; // forward declaration of CClient #endif /* Definitions ****************************************************************/ -#define METER_FLY_BACK 2 -#define INVALID_MIDI_CH -1 // invalid MIDI channel definition -#define DNS_SRV_RESOLVE_TIMEOUT_MS 500 +#define METER_FLY_BACK 2 +#define INVALID_MIDI_CH -1 // invalid MIDI channel definition /* Global functions ***********************************************************/ // converting float to short From 18fbfa1c18bd9829e2a8852af5705a0cca0e24ac Mon Sep 17 00:00:00 2001 From: Tony Mountifield Date: Tue, 10 Nov 2026 22:46:54 +0000 Subject: [PATCH 4/5] Add sequence number for CClient Connect and Disconnect This prevents stale address resolver slots from firing, if Disconnect is called while a Connect is still pending resolution. --- src/client.cpp | 21 +++++++++++++++++++-- src/client.h | 3 +++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/client.cpp b/src/client.cpp index 645da46bd2..126760b402 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -93,7 +93,8 @@ CClient::CClient ( const quint16 iPortNumber, bJitterBufferOK ( true ), bMuteMeInPersonalMix ( bNMuteMeInPersonalMix ), iServerSockBufNumFrames ( DEF_NET_BUF_SIZE_NUM_BL ), - bRawAudioIsSupported ( false ) + bRawAudioIsSupported ( false ), + iResolverSeq ( 0 ) { int iOpusError; @@ -1131,6 +1132,8 @@ void CClient::Stop() /// @emit Disconnected void CClient::Disconnect() { + ++iResolverSeq; // invalidate any uncompleted resolver requests + // Key off the connection state, not IsRunning() (which tracks the audio // device): the two diverge while connecting and in headless mode, and on // SIGTERM we must still send the disconnect message to the server. @@ -1157,7 +1160,21 @@ void CClient::Connect ( const QString& strServerAddress, const QString& strServe // different server while connected behaves as a reconnect Disconnect(); - if ( NetworkUtil::ResolveAddress ( strServerAddress, bIPv6Available, this, &CClient::OnHostResolved, true, true ) ) + const int seq = iResolverSeq; // for capturing the current request + + if ( NetworkUtil::ResolveAddress ( + strServerAddress, + bIPv6Available, + this, + [this, seq] ( const QHostInfo& info, quint16 port ) { + // check that the request has not been invalidated + if ( seq == iResolverSeq ) + { + OnHostResolved ( info, port ); // handle result + } + }, + true, + true ) ) { SetConnectedServerName ( strServerName ); // set if resolution initiated ok } diff --git a/src/client.h b/src/client.h index d692df8fe4..08289d047f 100644 --- a/src/client.h +++ b/src/client.h @@ -477,6 +477,9 @@ class CClient : public QObject int maxGainOrPanId; int iCurPingTime; + // sequence number for asynchronous DNS resolver + int iResolverSeq; + protected slots: void OnHandledSignal ( int sigNum ); void OnSendProtMessage ( CVector vecMessage ); From 156cfed1d77e5e278925f9b8752791b761e13950 Mon Sep 17 00:00:00 2001 From: Tony Mountifield Date: Thu, 12 Nov 2026 16:04:04 +0000 Subject: [PATCH 5/5] Add sequence number for CConnectDlg directory resolution This prevents stale address resolver slots from firing, if a new directory is selected before the current one has been resolved. --- src/connectdlg.cpp | 20 ++++++++++++++++++-- src/connectdlg.h | 3 +++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/connectdlg.cpp b/src/connectdlg.cpp index 9b5a9595e2..d8c85c9783 100644 --- a/src/connectdlg.cpp +++ b/src/connectdlg.cpp @@ -90,7 +90,8 @@ CConnectDlg::CConnectDlg ( CClient* pNCliP, CClientSettings* pNSetP, const bool bReducedServerListReceived ( false ), bServerListItemWasChosen ( false ), bListFilterWasActive ( false ), - bShowAllMusicians ( true ) + bShowAllMusicians ( true ), + iResolverSeq ( 0 ) { setupUi ( this ); @@ -322,7 +323,22 @@ void CConnectDlg::RequestServerList() const QString strDirectoryAddress = NetworkUtil::GetDirectoryAddress ( pSettings->eDirectoryType, pSettings->vstrDirectoryAddress[pSettings->iCustomDirectoryIndex] ); - if ( !NetworkUtil::ResolveAddress ( strDirectoryAddress, pClient->IsIPv6Available(), this, &CConnectDlg::OnHostResolved, true, true ) ) + // get a new sequence number for matching the resolved callback + const int seq = ++iResolverSeq; + + if ( !NetworkUtil::ResolveAddress ( + strDirectoryAddress, + pClient->IsIPv6Available(), + this, + [this, seq] ( const QHostInfo& info, quint16 port ) { + // check that the request has not been superseded + if ( seq == iResolverSeq ) + { + OnHostResolved ( info, port ); // handle result + } + }, + true, + true ) ) { qWarning() << "Cannot start directory fetch from" << strDirectoryAddress; } diff --git a/src/connectdlg.h b/src/connectdlg.h index cf77a2b9f6..6f7e29eb79 100644 --- a/src/connectdlg.h +++ b/src/connectdlg.h @@ -146,6 +146,9 @@ class CConnectDlg : public CBaseDlg, private Ui_CConnectDlgBase bool bListFilterWasActive; bool bShowAllMusicians; + // sequence number for asynchronous DNS resolver + int iResolverSeq; + public slots: void OnServerListItemDoubleClicked ( QTreeWidgetItem* Item, int ); void OnDirectoryChanged ( int iTypeIdx );