Add JSON-RPC methods to connect, disconnect and query connection state

docs/JSON-RPC.md

@@ -129,6 +129,23 @@ Results:
 | result.version | string | The Jamulus version. |
 
 
+### jamulusclient/disconnect
+
+Disconnects the client from the current server, or cancels a pending connection attempt. Does nothing if the client is disconnected.
+
+Parameters:
+
+| Name | Type | Description |
+| --- | --- | --- |
+| params | object | No parameters (empty object). |
+
+Results:
+
+| Name | Type | Description |
+| --- | --- | --- |
+| result | string | Always "ok". |
+
+
 ### jamulusclient/getChannelInfo
 
 Returns the client's profile information.
@@ -188,6 +205,24 @@ Results:
 | result.clients | array | The client list. See jamulusclient/clientListReceived for the format. |
 
 
+### jamulusclient/getConnectionState
+
+Returns the current connection state.
+
+Parameters:
+
+| Name | Type | Description |
+| --- | --- | --- |
+| params | object | No parameters (empty object). |
+
+Results:
+
+| Name | Type | Description |
+| --- | --- | --- |
+| result.state | string | The connection state (disconnected, connecting, or connected). |
+| result.serverName | string | The human readable name of the current server (empty if disconnected). |
+
+
 ### jamulusclient/getCurrentDirectory
 
 Returns the currently selected directory socket address.
@@ -256,6 +291,24 @@ Results:
 | result | string | "ok" or "error" if bad arguments. |
 
 
+### jamulusclient/requestConnection
+
+Connects the client to a server. Any current connection is terminated first. The connection is established asynchronously: subscribe to the jamulusclient/connected and jamulusclient/connectionStateChanged notifications to follow its progress (a failed attempt arrives as connectionStateChanged with an error field). An address that cannot be resolved is rejected with an error and leaves the current connection untouched.
+
+Parameters:
+
+| Name | Type | Description |
+| --- | --- | --- |
+| params.address | string | Socket address of the server (host:port). |
+| params.serverName | string | Optional human readable server name used for display purposes; if given it must be a string (null counts as omitted). Defaults to the address. |
+
+Results:
+
+| Name | Type | Description |
+| --- | --- | --- |
+| result | string | "ok" once the connection attempt has been initiated. |
+
+
 ### jamulusclient/sendChatText
 
 Sends a chat text message.
@@ -656,6 +709,19 @@ Parameters:
 | params.id | number | The channel ID assigned to the client. |
 
 
+### jamulusclient/connectionStateChanged
+
+Emitted whenever the connection state changes, and on a failed connection attempt, which adds an error field and reports the state the client is left in.
+
+Parameters:
+
+| Name | Type | Description |
+| --- | --- | --- |
+| params.state | string | The connection state (disconnected, connecting, or connected). |
+| params.serverName | string | The human readable server name (empty when disconnected). |
+| params.error | string | Only present on a failed connection attempt. |
+
+
 ### jamulusclient/disconnected
 
 Emitted when the client is disconnected from the server.

src/client.cpp

@@ -45,6 +45,7 @@
 \******************************************************************************/
 
 #include "client.h"
+#include <QThread>
 #include "settings.h"
 #include "util.h"
 
@@ -619,25 +620,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
@@ -1108,19 +1090,12 @@ void CClient::Stop()
         qWarning() << "Could not reinitialise the sound device while disconnecting:" << generr.GetErrorText();
     }
 
-    // wait for approx. 100 ms to make sure no audio packet is still in the
-    // network queue causing the channel to be reconnected right after having
-    // received the disconnect message (seems not to gain much, disconnect is
-    // still not working reliably)
-    QTime DieTime = QTime::currentTime().addMSecs ( 100 );
-    while ( QTime::currentTime() < DieTime )
-    {
-        // exclude user input events because if we use AllEvents, it happens
-        // that if the user initiates a connection and disconnection quickly
-        // (e.g. quickly pressing enter five times), the software can get into
-        // an unknown state
-        QCoreApplication::processEvents ( QEventLoop::ExcludeUserInputEvents, 100 );
-    }
+    // Wait ~100 ms so no audio packet is still in the network queue causing the
+    // channel to be reconnected right after the disconnect message. We must NOT
+    // run the event loop to do this: pumping events here re-entered the JSON-RPC
+    // read handler and freed a socket still being written to (use-after-free). A
+    // plain sleep keeps the settle without re-entrancy.
+    QThread::msleep ( 100 );
 
     // Send disconnect message to server (Since we disable our protocol
     // receive mechanism with the next command, we do not evaluate any
@@ -1162,30 +1137,38 @@ 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 the address resolved. 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
 /// @param strServerAddress - the server address to connect to
 /// @param strServerName - the human readable server name passed to Connecting()
 void CClient::Connect ( const QString& strServerAddress, const QString& strServerName )
+{
+    // resolve before touching the current connection, so that an invalid
+    // address leaves it in place
+    CHostAddress HostAddress;
+
+    if ( !NetworkUtil::ParseNetworkAddress ( strServerAddress, HostAddress, bIPv6Available ) )
+    {
+        emit ConnectingFailed ( tr ( "Received invalid server address. Please check for typos in the provided server address." ) );
+        return;
+    }
+
+    Connect ( HostAddress, strServerName );
+}
+
+void CClient::Connect ( const CHostAddress& HostAddress, const QString& strServerName )
 {
     try
     {
         // disconnect from any current server first so that connecting to a
         // different server while connected behaves as a reconnect
         Disconnect();
 
-        // Set server address and connect if valid address was supplied
-        if ( SetServerAddr ( strServerAddress ) )
-        {
-            SetConnectedServerName ( strServerName );
-            Start();
-        }
-        else
-        {
-            throw CGenErr ( tr ( "Received invalid server address. Please check for typos in the provided server address." ) );
-        }
+        Channel.SetAddress ( HostAddress );
+        SetConnectedServerName ( strServerName );
+        Start();
     }
     catch ( const CGenErr& generr )
     {

src/client.h

@@ -171,6 +171,7 @@ class CClient : public QObject
 
     void Disconnect();
     void Connect ( const QString& strServerAddress, const QString& strServerName );
+    void Connect ( const CHostAddress& HostAddress, const QString& strServerName );
 
     // The ConnectedServerName is emitted by Connecting() to update the UI with a human readable server name
     void    SetConnectedServerName ( const QString& strServerName ) { strConnectedServerName = strServerName; };
@@ -180,7 +181,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; }

src/clientrpc.cpp

@@ -47,6 +47,21 @@
 
 #include "clientrpc.h"
 
+static QString ConnectionStateToString ( const EConnectionState eState )
+{
+    switch ( eState )
+    {
+    case CS_CONNECTING:
+        return "connecting";
+
+    case CS_CONNECTED:
+        return "connected";
+
+    default:
+        return "disconnected";
+    }
+}
+
 CClientRpc::CClientRpc ( CClient* pClient, CClientSettings* pSettings, CRpcServer* pRpcServer, QObject* parent ) :
     QObject ( parent ),
     m_pSettings ( pSettings )
@@ -168,6 +183,33 @@ CClientRpc::CClientRpc ( CClient* pClient, CClientSettings* pSettings, CRpcServe
     /// @param {object} params - No parameters (empty object).
     connect ( pClient, &CClient::Disconnected, [=]() { pRpcServer->BroadcastNotification ( "jamulusclient/disconnected", QJsonObject{} ); } );
 
+    // A failed attempt surfaces through connectionStateChanged with the error attached. The
+    // state and name are the current ones: an address that fails to resolve leaves any
+    // existing connection in place.
+    connect ( pClient, &CClient::ConnectingFailed, [=] ( QString strError ) {
+        const EConnectionState eState = pClient->GetConnectionState();
+        pRpcServer->BroadcastNotification ( "jamulusclient/connectionStateChanged",
+                                            QJsonObject{
+                                                { "state", ConnectionStateToString ( eState ) },
+                                                { "serverName", eState == CS_DISCONNECTED ? QString() : pClient->GetConnectedServerName() },
+                                                { "error", strError },
+                                            } );
+    } );
+
+    /// @rpc_notification jamulusclient/connectionStateChanged
+    /// @brief Emitted whenever the connection state changes, and on a failed connection attempt,
+    ///        which adds an error field and reports the state the client is left in.
+    /// @param {string} params.state - The connection state (disconnected, connecting, or connected).
+    /// @param {string} params.serverName - The human readable server name (empty when disconnected).
+    /// @param {string} params.error - Only present on a failed connection attempt.
+    connect ( pClient, &CClient::ConnectionStateChanged, [=] ( EConnectionState eState ) {
+        pRpcServer->BroadcastNotification ( "jamulusclient/connectionStateChanged",
+                                            QJsonObject{
+                                                { "state", ConnectionStateToString ( eState ) },
+                                                { "serverName", eState == CS_DISCONNECTED ? QString() : pClient->GetConnectedServerName() },
+                                            } );
+    } );
+
     /// @rpc_notification jamulusclient/recorderState
     /// @brief Emitted when the client is connected to a server whose recorder state changes.
     /// @param {number} params.state - The recorder state.
@@ -212,6 +254,76 @@ CClientRpc::CClientRpc ( CClient* pClient, CClientSettings* pSettings, CRpcServe
         Q_UNUSED ( params );
     } );
 
+    /// @rpc_method jamulusclient/requestConnection
+    /// @brief Connects the client to a server. Any current connection is terminated first.
+    ///        The connection is established asynchronously: subscribe to the jamulusclient/connected
+    ///        and jamulusclient/connectionStateChanged notifications to follow its progress (a failed
+    ///        attempt arrives as connectionStateChanged with an error field). An address that cannot
+    ///        be resolved is rejected with an error and leaves the current connection untouched.
+    /// @param {string} params.address - Socket address of the server (host:port).
+    /// @param {string} params.serverName - Optional human readable server name used for display purposes; if given it must be a string
+    /// (null counts as omitted). Defaults to the address.
+    /// @result {string} result - "ok" once the connection attempt has been initiated.
+    pRpcServer->HandleMethod ( "jamulusclient/requestConnection", [=] ( const QJsonObject& params, QJsonObject& response ) {
+        auto jsonAddress = params["address"];
+        if ( !jsonAddress.isString() )
+        {
+            response["error"] = CRpcServer::CreateJsonRpcError ( CRpcServer::iErrInvalidParams, "Invalid params: address is not a string" );
+            return;
+        }
+
+        auto jsonServerName = params["serverName"];
+        if ( !jsonServerName.isUndefined() && !jsonServerName.isNull() && !jsonServerName.isString() )
+        {
+            response["error"] = CRpcServer::CreateJsonRpcError ( CRpcServer::iErrInvalidParams, "Invalid params: serverName is not a string" );
+            return;
+        }
+
+        const QString strAddress    = NetworkUtil::FixAddress ( jsonAddress.toString() );
+        const QString strServerName = jsonServerName.isString() ? jsonServerName.toString() : strAddress;
+
+        // resolve here so that the caller gets an error result for an invalid address
+        CHostAddress haServer;
+        if ( !NetworkUtil::ParseNetworkAddress ( strAddress, haServer, pClient->IsIPv6Available() ) )
+        {
+            response["error"] =
+                CRpcServer::CreateJsonRpcError ( CRpcServer::iErrInvalidParams, "Invalid params: address is not a valid socket address" );
+            return;
+        }
+
+        pClient->Connect ( haServer, strServerName );
+
+        response["result"] = "ok";
+    } );
+
+    /// @rpc_method jamulusclient/disconnect
+    /// @brief Disconnects the client from the current server, or cancels a pending connection attempt. Does nothing if the client is
+    /// disconnected.
+    /// @param {object} params - No parameters (empty object).
+    /// @result {string} result - Always "ok".
+    pRpcServer->HandleMethod ( "jamulusclient/disconnect", [=] ( const QJsonObject& params, QJsonObject& response ) {
+        pClient->Disconnect();
+
+        response["result"] = "ok";
+        Q_UNUSED ( params );
+    } );
+
+    /// @rpc_method jamulusclient/getConnectionState
+    /// @brief Returns the current connection state.
+    /// @param {object} params - No parameters (empty object).
+    /// @result {string} result.state - The connection state (disconnected, connecting, or connected).
+    /// @result {string} result.serverName - The human readable name of the current server (empty if disconnected).
+    pRpcServer->HandleMethod ( "jamulusclient/getConnectionState", [=] ( const QJsonObject& params, QJsonObject& response ) {
+        const EConnectionState eState = pClient->GetConnectionState();
+
+        QJsonObject result{
+            { "state", ConnectionStateToString ( eState ) },
+            { "serverName", eState == CS_DISCONNECTED ? QString() : pClient->GetConnectedServerName() },
+        };
+        response["result"] = result;
+        Q_UNUSED ( params );
+    } );
+
     /// @rpc_method jamulus/getMode
     /// @brief Returns the current mode, i.e. whether Jamulus is running as a server or client.
     /// @param {object} params - No parameters (empty object).

src/util.cpp

@@ -944,7 +944,9 @@ bool NetworkUtil::ParseNetworkAddress ( QString strAddress, CHostAddress& HostAd
     // Try SRV-based discovery first:
     if ( ParseNetworkAddressSrv ( strAddress, HostAddress, bIPv6Available ) )
     {
-        return true;
+        // an SRV target of "." means the service is not offered: fail here
+        // rather than falling back to a host lookup
+        return !HostAddress.InetAddr.isNull();
     }
 #endif
     // Try regular connect via plain IP or host name lookup (A/AAAA):