From 917aa5d96b0de8f9aeb0f04f68a66157ba235dd5 Mon Sep 17 00:00:00 2001 From: Volker Fischer Date: Mon, 28 Sep 2020 17:39:43 +0200 Subject: [PATCH 1/3] added reduced server list message (to reduce probability of UDP fragmentation issues) --- src/client.cpp | 3 ++ src/client.h | 3 ++ src/clientdlg.cpp | 3 ++ src/clientdlg.h | 4 ++ src/connectdlg.cpp | 23 +++++--- src/connectdlg.h | 3 +- src/protocol.cpp | 131 ++++++++++++++++++++++++++++++++++++++++++--- src/protocol.h | 13 ++++- src/serverlist.cpp | 5 +- 9 files changed, 168 insertions(+), 20 deletions(-) diff --git a/src/client.cpp b/src/client.cpp index 4190645861..c0e83e99b6 100755 --- a/src/client.cpp +++ b/src/client.cpp @@ -157,6 +157,9 @@ CClient::CClient ( const quint16 iPortNumber, QObject::connect ( &ConnLessProtocol, &CProtocol::CLServerListReceived, this, &CClient::CLServerListReceived ); + QObject::connect ( &ConnLessProtocol, &CProtocol::CLRedServerListReceived, + this, &CClient::CLRedServerListReceived ); + QObject::connect ( &ConnLessProtocol, &CProtocol::CLConnClientsListMesReceived, this, &CClient::CLConnClientsListMesReceived ); diff --git a/src/client.h b/src/client.h index d7d470dfb5..b2dd419d71 100755 --- a/src/client.h +++ b/src/client.h @@ -412,6 +412,9 @@ protected slots: void CLServerListReceived ( CHostAddress InetAddr, CVector vecServerInfo ); + void CLRedServerListReceived ( CHostAddress InetAddr, + CVector vecServerInfo ); + void CLConnClientsListMesReceived ( CHostAddress InetAddr, CVector vecChanInfo ); diff --git a/src/clientdlg.cpp b/src/clientdlg.cpp index 8c4a6d4d99..f542ddbe87 100755 --- a/src/clientdlg.cpp +++ b/src/clientdlg.cpp @@ -432,6 +432,9 @@ CClientDlg::CClientDlg ( CClient* pNCliP, QObject::connect ( pClient, &CClient::CLServerListReceived, this, &CClientDlg::OnCLServerListReceived ); + QObject::connect ( pClient, &CClient::CLRedServerListReceived, + this, &CClientDlg::OnCLRedServerListReceived ); + QObject::connect ( pClient, &CClient::CLConnClientsListMesReceived, this, &CClientDlg::OnCLConnClientsListMesReceived ); diff --git a/src/clientdlg.h b/src/clientdlg.h index 4069c12570..ac09ba0795 100755 --- a/src/clientdlg.h +++ b/src/clientdlg.h @@ -201,6 +201,10 @@ public slots: CVector vecServerInfo ) { ConnectDlg.SetServerList ( InetAddr, vecServerInfo ); } + void OnCLRedServerListReceived ( CHostAddress InetAddr, + CVector vecServerInfo ) + { ConnectDlg.SetServerList ( InetAddr, vecServerInfo, true ); } + void OnCLConnClientsListMesReceived ( CHostAddress InetAddr, CVector vecChanInfo ) { ConnectDlg.SetConnClientsList ( InetAddr, vecChanInfo ); } diff --git a/src/connectdlg.cpp b/src/connectdlg.cpp index f7d7ce9a66..cc0e7e353a 100755 --- a/src/connectdlg.cpp +++ b/src/connectdlg.cpp @@ -267,11 +267,16 @@ void CConnectDlg::OnTimerReRequestServList() } void CConnectDlg::SetServerList ( const CHostAddress& InetAddr, - const CVector& vecServerInfo ) + const CVector& vecServerInfo, + const bool bIsReducedServerList ) { - // set flag and disable timer for resend server list request - bServerListReceived = true; - TimerReRequestServList.stop(); + // set flag and disable timer for resend server list request if full list + // was received (i.e. not the reduced list) + if ( !bIsReducedServerList ) + { + bServerListReceived = true; + TimerReRequestServList.stop(); + } // first clear list lvwServers->clear(); @@ -747,13 +752,15 @@ void CConnectDlg::SetPingTimeAndNumClientsResult ( const CHostAddress& InetAddr, // update number of clients text if ( iNumClients >= pCurListViewItem->text ( 5 ).toInt() ) { - pCurListViewItem-> - setText ( 2, QString().setNum ( iNumClients ) + " (full)" ); + pCurListViewItem->setText ( 2, QString().setNum ( iNumClients ) + " (full)" ); + } + else if ( pCurListViewItem->text ( 5 ).toInt() == 0 ) + { + pCurListViewItem->setText ( 2, QString().setNum ( iNumClients ) ); } else { - pCurListViewItem-> - setText ( 2, QString().setNum ( iNumClients ) + "/" + pCurListViewItem->text ( 5 ) ); + pCurListViewItem->setText ( 2, QString().setNum ( iNumClients ) + "/" + pCurListViewItem->text ( 5 ) ); } // check if the number of child list items matches the number of diff --git a/src/connectdlg.h b/src/connectdlg.h index eaa7128210..8afa966fce 100755 --- a/src/connectdlg.h +++ b/src/connectdlg.h @@ -61,7 +61,8 @@ class CConnectDlg : public QDialog, private Ui_CConnectDlgBase bool GetShowAllMusicians() { return bShowAllMusicians; } void SetServerList ( const CHostAddress& InetAddr, - const CVector& vecServerInfo ); + const CVector& vecServerInfo, + const bool bIsReducedServerList = false ); void SetConnClientsList ( const CHostAddress& InetAddr, const CVector& vecChanInfo ); diff --git a/src/protocol.cpp b/src/protocol.cpp index 2cffe5309b..a66ccb2f03 100755 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -338,6 +338,18 @@ CONNECTION LESS MESSAGES of the PROTMESSID_CLM_REGISTER_SERVER message is used +- PROTMESSID_CLM_RED_SERVER_LIST: Reduced server list message (to have less UDP fragmentation) + + for each registered server append following data: + + +--------------------+------------------------------+ ... + | 4 bytes IP address | 2 bytes server internal port | ... + +--------------------+------------------------------+ ... + ... -----------------+----------------------------------+ + ... 1 byte number n | n bytes UTF-8 string server name | + ... -----------------+----------------------------------+ + + - PROTMESSID_CLM_REQ_SERVER_LIST: Request server list note: does not have any data -> n = 0 @@ -883,6 +895,10 @@ if ( rand() < ( RAND_MAX / 2 ) ) return false; EvaluateCLServerListMes ( InetAddr, vecbyMesBodyData ); break; + case PROTMESSID_CLM_RED_SERVER_LIST: + EvaluateCLRedServerListMes ( InetAddr, vecbyMesBodyData ); + break; + case PROTMESSID_CLM_REQ_SERVER_LIST: EvaluateCLReqServerListMes ( InetAddr ); break; @@ -2323,6 +2339,103 @@ bool CProtocol::EvaluateCLServerListMes ( const CHostAddress& InetAddr, return false; // no error } +void CProtocol::CreateCLRedServerListMes ( const CHostAddress& InetAddr, + const CVector vecServerInfo ) +{ + const int iNumServers = vecServerInfo.Size(); + + // build data vector + CVector vecData ( 0 ); + int iPos = 0; // init position pointer + + for ( int i = 0; i < iNumServers; i++ ) + { + // convert server list strings to utf-8 + const QByteArray strUTF8Name = vecServerInfo[i].strName.toUtf8(); + + // size of current list entry + const int iCurListEntrLen = + 4 /* IP address */ + + 2 /* port number */ + + 1 /* name utf-8 string size */ + strUTF8Name.size(); + + // make space for new data + vecData.Enlarge ( iCurListEntrLen ); + + // IP address (4 bytes) + // note the Server List manager has put the internal details in HostAddr where required + PutValOnStream ( vecData, iPos, static_cast ( + vecServerInfo[i].HostAddr.InetAddr.toIPv4Address() ), 4 ); + + // port number (2 bytes) + // note the Server List manager has put the internal details in HostAddr where required + PutValOnStream ( vecData, iPos, + static_cast ( vecServerInfo[i].HostAddr.iPort ), 2 ); + + // name (note that the string length indicator is 1 in this special case) + PutStringUTF8OnStream ( vecData, iPos, strUTF8Name, 1 ); + } + + CreateAndImmSendConLessMessage ( PROTMESSID_CLM_RED_SERVER_LIST, + vecData, + InetAddr ); +} + +bool CProtocol::EvaluateCLRedServerListMes ( const CHostAddress& InetAddr, + const CVector& vecData ) +{ + int iPos = 0; // init position pointer + const int iDataLen = vecData.Size(); + CVector vecServerInfo ( 0 ); + + while ( iPos < iDataLen ) + { + // check size (the next 6 bytes) + if ( ( iDataLen - iPos ) < 6 ) + { + return true; // return error code + } + + // IP address (4 bytes) + const quint32 iIpAddr = static_cast ( GetValFromStream ( vecData, iPos, 4 ) ); + + // port number (2 bytes) + const quint16 iPort = static_cast ( GetValFromStream ( vecData, iPos, 2 ) ); + + // server name (note that the string length indicator is 1 in this special case) + QString strName; + if ( GetStringFromStream ( vecData, + iPos, + MAX_LEN_SERVER_NAME, + strName, + 1 ) ) + { + return true; // return error code + } + + // add server information to vector + vecServerInfo.Add ( + CServerInfo ( CHostAddress ( QHostAddress ( iIpAddr ), iPort ), + CHostAddress ( QHostAddress ( iIpAddr ), iPort ), + strName, + QLocale::AnyCountry, // set to any country since the information is not transmitted + "", // empty city name since the information is not transmitted + 0, // per definition: if max. num. client is zero, we ignore the value in the server list + false ) ); // assume not permanent since the information is not transmitted + } + + // check size: all data is read, the position must now be at the end + if ( iPos != iDataLen ) + { + return true; // return error code + } + + // invoke message action + emit CLRedServerListReceived ( InetAddr, vecServerInfo ); + + return false; // no error +} + void CProtocol::CreateCLReqServerListMes ( const CHostAddress& InetAddr ) { CreateAndImmSendConLessMessage ( PROTMESSID_CLM_REQ_SERVER_LIST, @@ -2883,21 +2996,22 @@ uint32_t CProtocol::GetValFromStream ( const CVector& vecIn, bool CProtocol::GetStringFromStream ( const CVector& vecIn, int& iPos, const int iMaxStringLen, - QString& strOut ) + QString& strOut, + const int iNumberOfBytsLen ) { /* note: iPos is automatically incremented in this function */ const int iInLen = vecIn.Size(); - // check if at least two bytes are available - if ( ( iInLen - iPos ) < 2 ) + // check if at least iNumberOfBytsLen bytes are available + if ( ( iInLen - iPos ) < iNumberOfBytsLen ) { return true; // return error code } - // number of bytes for utf-8 string (2 bytes) - const int iStrUTF8Len = static_cast ( GetValFromStream ( vecIn, iPos, 2 ) ); + // number of bytes for utf-8 string (1 or 2 bytes) + const int iStrUTF8Len = static_cast ( GetValFromStream ( vecIn, iPos, iNumberOfBytsLen ) ); // (note that iPos was incremented by 2 in the above code!) if ( ( iInLen - iPos ) < iStrUTF8Len ) @@ -3034,13 +3148,14 @@ void CProtocol::PutValOnStream ( CVector& vecIn, void CProtocol::PutStringUTF8OnStream ( CVector& vecIn, int& iPos, - const QByteArray& sStringUTF8 ) + const QByteArray& sStringUTF8, + const int iNumberOfBytsLen ) { // get the utf-8 string size const int iStrUTF8Len = sStringUTF8.size(); - // number of bytes for utf-8 string (2 bytes) - PutValOnStream ( vecIn, iPos, static_cast ( iStrUTF8Len ), 2 ); + // number of bytes for utf-8 string (iNumberOfBytsLen bytes) + PutValOnStream ( vecIn, iPos, static_cast ( iStrUTF8Len ), iNumberOfBytsLen ); // actual utf-8 string (n bytes) for ( int j = 0; j < iStrUTF8Len; j++ ) diff --git a/src/protocol.h b/src/protocol.h index 8afbf10f3e..fd0c6c32e0 100755 --- a/src/protocol.h +++ b/src/protocol.h @@ -83,6 +83,7 @@ #define PROTMESSID_CLM_CHANNEL_LEVEL_LIST 1015 // channel level list #define PROTMESSID_CLM_REGISTER_SERVER_RESP 1016 // status of server registration request #define PROTMESSID_CLM_REGISTER_SERVER_EX 1017 // register server with extended information +#define PROTMESSID_CLM_RED_SERVER_LIST 1018 // reduced server list // special IDs #define PROTMESSID_SPECIAL_SPLIT_MESSAGE 2001 // a container for split messages @@ -145,6 +146,8 @@ class CProtocol : public QObject void CreateCLUnregisterServerMes ( const CHostAddress& InetAddr ); void CreateCLServerListMes ( const CHostAddress& InetAddr, const CVector vecServerInfo ); + void CreateCLRedServerListMes ( const CHostAddress& InetAddr, + const CVector vecServerInfo ); void CreateCLReqServerListMes ( const CHostAddress& InetAddr ); void CreateCLSendEmptyMesMes ( const CHostAddress& InetAddr, const CHostAddress& TargetInetAddr ); @@ -238,7 +241,8 @@ class CProtocol : public QObject void PutStringUTF8OnStream ( CVector& vecIn, int& iPos, - const QByteArray& sStringUTF8 ); + const QByteArray& sStringUTF8, + const int iNumberOfBytsLen = 2 ); // default is 2 bytes lenght indicator static uint32_t GetValFromStream ( const CVector& vecIn, int& iPos, @@ -247,7 +251,8 @@ class CProtocol : public QObject bool GetStringFromStream ( const CVector& vecIn, int& iPos, const int iMaxStringLen, - QString& strOut ); + QString& strOut, + const int iNumberOfBytsLen = 2 ); // default is 2 bytes lenght indicator void SendMessage(); @@ -290,6 +295,8 @@ class CProtocol : public QObject bool EvaluateCLUnregisterServerMes ( const CHostAddress& InetAddr ); bool EvaluateCLServerListMes ( const CHostAddress& InetAddr, const CVector& vecData ); + bool EvaluateCLRedServerListMes ( const CHostAddress& InetAddr, + const CVector& vecData ); bool EvaluateCLReqServerListMes ( const CHostAddress& InetAddr ); bool EvaluateCLSendEmptyMesMes ( const CVector& vecData ); bool EvaluateCLDisconnectionMes ( const CHostAddress& InetAddr ); @@ -367,6 +374,8 @@ public slots: void CLUnregisterServerReceived ( CHostAddress InetAddr ); void CLServerListReceived ( CHostAddress InetAddr, CVector vecServerInfo ); + void CLRedServerListReceived ( CHostAddress InetAddr, + CVector vecServerInfo ); void CLReqServerList ( CHostAddress InetAddr ); void CLSendEmptyMes ( CHostAddress TargetInetAddr ); void CLDisconnection ( CHostAddress InetAddr ); diff --git a/src/serverlist.cpp b/src/serverlist.cpp index 74c1b37fc7..f5a67d2db1 100755 --- a/src/serverlist.cpp +++ b/src/serverlist.cpp @@ -526,7 +526,10 @@ void CServerListManager::CentralServerQueryServerList ( const CHostAddress& Inet } } - // send the server list to the client + // send the server list to the client, since we do not know that the client + // has a UDP fragmentation issue, we send both lists, the reduced and the + // normal list after each other + pConnLessProtocol->CreateCLRedServerListMes ( InetAddr, vecServerInfo ); pConnLessProtocol->CreateCLServerListMes ( InetAddr, vecServerInfo ); } } From f27cf88f727e048b81ec37c2c7a14274a321eb3e Mon Sep 17 00:00:00 2001 From: Volker Fischer Date: Mon, 28 Sep 2020 17:53:27 +0200 Subject: [PATCH 2/3] bug fix: all servers were shown to be "(full)" --- src/connectdlg.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/connectdlg.cpp b/src/connectdlg.cpp index cc0e7e353a..ee52164ae6 100755 --- a/src/connectdlg.cpp +++ b/src/connectdlg.cpp @@ -750,13 +750,14 @@ void CConnectDlg::SetPingTimeAndNumClientsResult ( const CHostAddress& InetAddr, } // update number of clients text - if ( iNumClients >= pCurListViewItem->text ( 5 ).toInt() ) + if ( pCurListViewItem->text ( 5 ).toInt() == 0 ) { - pCurListViewItem->setText ( 2, QString().setNum ( iNumClients ) + " (full)" ); + // special case: reduced server list + pCurListViewItem->setText ( 2, QString().setNum ( iNumClients ) ); } - else if ( pCurListViewItem->text ( 5 ).toInt() == 0 ) + else if ( iNumClients >= pCurListViewItem->text ( 5 ).toInt() ) { - pCurListViewItem->setText ( 2, QString().setNum ( iNumClients ) ); + pCurListViewItem->setText ( 2, QString().setNum ( iNumClients ) + " (full)" ); } else { From b4a0757bd84eb88bd42f94f1f4e1a236c71a8a65 Mon Sep 17 00:00:00 2001 From: Volker Fischer Date: Mon, 28 Sep 2020 18:01:27 +0200 Subject: [PATCH 3/3] bug fix: do not update reduced server list if it was once received --- src/connectdlg.cpp | 48 ++++++++++++++++++++++++++++++---------------- src/connectdlg.h | 1 + 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/connectdlg.cpp b/src/connectdlg.cpp index ee52164ae6..cdd99522e3 100755 --- a/src/connectdlg.cpp +++ b/src/connectdlg.cpp @@ -30,16 +30,17 @@ CConnectDlg::CConnectDlg ( CClient* pNCliP, const bool bNewShowCompleteRegList, QWidget* parent, Qt::WindowFlags f ) - : QDialog ( parent, f ), - pClient ( pNCliP ), - strCentralServerAddress ( "" ), - strSelectedAddress ( "" ), - strSelectedServerName ( "" ), - bShowCompleteRegList ( bNewShowCompleteRegList ), - bServerListReceived ( false ), - bServerListItemWasChosen ( false ), - bListFilterWasActive ( false ), - bShowAllMusicians ( true ) + : QDialog ( parent, f ), + pClient ( pNCliP ), + strCentralServerAddress ( "" ), + strSelectedAddress ( "" ), + strSelectedServerName ( "" ), + bShowCompleteRegList ( bNewShowCompleteRegList ), + bServerListReceived ( false ), + bReducedServerListReceived ( false ), + bServerListItemWasChosen ( false ), + bListFilterWasActive ( false ), + bShowAllMusicians ( true ) { setupUi ( this ); @@ -216,9 +217,10 @@ void CConnectDlg::showEvent ( QShowEvent* ) void CConnectDlg::RequestServerList() { // reset flags - bServerListReceived = false; - bServerListItemWasChosen = false; - bListFilterWasActive = false; + bServerListReceived = false; + bReducedServerListReceived = false; + bServerListItemWasChosen = false; + bListFilterWasActive = false; // clear current address and name strSelectedAddress = ""; @@ -270,10 +272,24 @@ void CConnectDlg::SetServerList ( const CHostAddress& InetAddr, const CVector& vecServerInfo, const bool bIsReducedServerList ) { - // set flag and disable timer for resend server list request if full list - // was received (i.e. not the reduced list) - if ( !bIsReducedServerList ) + // special treatment if a reduced server list was received + if ( bIsReducedServerList ) { + // make sure we only apply the reduced version list once + if ( bReducedServerListReceived ) + { + // do nothing + return; + } + else + { + bReducedServerListReceived = true; + } + } + else + { + // set flag and disable timer for resend server list request if full list + // was received (i.e. not the reduced list) bServerListReceived = true; TimerReRequestServList.stop(); } diff --git a/src/connectdlg.h b/src/connectdlg.h index 8afa966fce..99700dde89 100755 --- a/src/connectdlg.h +++ b/src/connectdlg.h @@ -102,6 +102,7 @@ class CConnectDlg : public QDialog, private Ui_CConnectDlgBase QString strSelectedServerName; bool bShowCompleteRegList; bool bServerListReceived; + bool bReducedServerListReceived; bool bServerListItemWasChosen; bool bListFilterWasActive; bool bShowAllMusicians;