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.

Sign up for GitHub

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

Jump to bottom

JSON-RPC: Add option to specify bind address for server #2917

Merged
merged 1 commit into from
Nov 10, 2022
Merged

JSON-RPC: Add option to specify bind address for server #2917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/JSON-RPC.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ It can be generated like this:
$ openssl rand -base64 10 > /file/with/a/secret.txt
```

The JSON-RPC server defaults to listening on the local loopback network interface (127.0.0.1). This can be optionally changed by using the `--jsonrpcbindip <ip address>` command line option. **IPv4 only. IPv6 support has not been tested.**


## Wire protocol

The JSON-RPC server is based on the [JSON-RPC 2.0](https://www.jsonrpc.org/specification) protocol, using [streaming newline-delimited JSON over TCP](https://clue.engineering/2018/introducing-reactphp-ndjson) as the transport. There are three main types of messages being exchanged:
Expand Down
4 changes: 2 additions & 2 deletions src/global.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,8 @@ LED bar: lbr
// minimum length of JSON-RPC secret string (main.cpp)
#define JSON_RPC_MINIMUM_SECRET_LENGTH 16

// JSON-RPC listen address
#define JSON_RPC_LISTEN_ADDRESS "127.0.0.1"
// JSON-RPC listen address (Default)
#define DEFAULT_JSON_RPC_LISTEN_ADDRESS "127.0.0.1"

#define _MAXSHORT 32767
#define _MINSHORT ( -32768 )
Expand Down
38 changes: 37 additions & 1 deletion src/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ int main ( int argc, char** argv )
int iNumServerChannels = DEFAULT_USED_NUM_CHANNELS;
quint16 iPortNumber = DEFAULT_PORT_NUMBER;
int iJsonRpcPortNumber = INVALID_PORT;
QString strJsonRpcBindIP = DEFAULT_JSON_RPC_LISTEN_ADDRESS;
quint16 iQosNumber = DEFAULT_QOS_NUMBER;
ELicenceType eLicenceType = LT_NO_LICENCE;
QString strMIDISetup = "";
Expand Down Expand Up @@ -209,6 +210,15 @@ int main ( int argc, char** argv )
continue;
}

// JSON-RPC bind address ------------------------------------------------
if ( GetStringArgument ( argc, argv, i, "--jsonrpcbindip", "--jsonrpcbindip", strArgument ) )
{
strJsonRpcBindIP = QString ( strArgument );
qInfo() << qUtf8Printable ( QString ( "- JSON-RPC will bind to: %1 if enabled" ).arg ( strJsonRpcBindIP ) );
CommandLineOptions << "--jsonrpcbindip";
continue;
}

// Quality of Service --------------------------------------------------
if ( GetNumericArgument ( argc, argv, i, "-Q", "--qos", 0, 255, rDbleArgument ) )
{
Expand Down Expand Up @@ -783,6 +793,31 @@ int main ( int argc, char** argv )
strServerBindIP = "";
}
}

#ifndef NO_JSON_RPC
//
// strJsonRpcBind address defaults to loopback and should not be empty, but
// in the odd chance that an empty IP is passed, we'll check for it here.

if ( strJsonRpcBindIP.trimmed().isEmpty() )
{
qCritical() << qUtf8Printable ( QString ( "JSON-RPC is enabled but no bind address was provided, exiting." ) );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the wording of the "objection" is slightly misleading here. --serverbindip was passed with a parameter, otherwise GetStringArgument would have failed earlier. So

Suggested change
qCritical() << qUtf8Printable ( QString ( "JSON-RPC is enabled but no bind address was provided, exiting." ) );
qCritical() << qUtf8Printable ( QString ( "JSON-RPC is enabled but bind address provided is invalid, exiting." ) );

It might even be useful to do if ( !InetAddr.setAddress ( strJsonRpcBindIP ) ), like for Server Bind IP.

I'd also question whether "exiting" is in keeping with the other checks we do... it's a bit hit and miss, I guess. I may even have said do it like this :).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the wording of the "objection" is slightly misleading here.

I'll squeeze that improvement into #2958 and rename it to be more generic.

I'd also question whether "exiting" is in keeping with the other checks we do... it's a bit hit and miss, I guess. I may even have said do it like this :).

I would have certainly supported that (maybe I did ;)). I think we should fail early for new features, even if we didn't do so previously and don't want to make those existing checks more strict for compatibility reasons for now.

exit ( 1 );
}

// This means of testing the validity of IP addresses is far from perfect but
// we do it here as an upfront check. The downstream network calls will error
// out on malformed addresses not caught here.
{
QHostAddress InetAddr;
if ( !InetAddr.setAddress ( strJsonRpcBindIP ) )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should introduce a method which checks IP validity e.g via Regex

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're probably right -- an IP address validation function used through the codebase. I think that is out of scope for this particular PR, however.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using the same methods as for Server Bind IP will be okay for now.

{
qCritical() << qUtf8Printable ( QString ( "The JSON-RPC address specified is not valid, exiting. " ) );
exit ( 1 );
}
}

#endif
}

// Application/GUI setup ---------------------------------------------------
Expand Down Expand Up @@ -880,7 +915,7 @@ int main ( int argc, char** argv )
qWarning() << "- JSON-RPC: This interface is experimental and is subject to breaking changes even on patch versions "
"(not subject to semantic versioning) during the initial phase.";

pRpcServer = new CRpcServer ( pApp, iJsonRpcPortNumber, strJsonRpcSecret );
pRpcServer = new CRpcServer ( pApp, strJsonRpcBindIP, iJsonRpcPortNumber, strJsonRpcSecret );
if ( !pRpcServer->Start() )
{
qCritical() << qUtf8Printable ( QString ( "- JSON-RPC: Server failed to start. Exiting." ) );
Expand Down Expand Up @@ -1072,6 +1107,7 @@ QString UsageArguments ( char** argv )
" --jsonrpcsecretfile\n"
" path to a single-line file which contains a freely\n"
" chosen secret to authenticate JSON-RPC users.\n"
" --jsonrpcbindip optional network address to bind RPC server. Defaults to 127.0.0.1 (IPv4 only, IPv6 not tested).\n"
" -Q, --qos set the QoS value. Default is 128. Disable with 0\n"
" (see the Jamulus website to enable QoS on Windows)\n"
" -t, --notranslation disable translation (use English language)\n"
Expand Down
5 changes: 3 additions & 2 deletions src/rpcserver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@
#include "global.h"
#include "rpcserver.h"

CRpcServer::CRpcServer ( QObject* parent, int iPort, QString strSecret ) :
CRpcServer::CRpcServer ( QObject* parent, QString strBindIP, int iPort, QString strSecret ) :
QObject ( parent ),
strBindIP ( strBindIP ),
pljones marked this conversation as resolved.
Show resolved Hide resolved
iPort ( iPort ),
strSecret ( strSecret ),
pTransportServer ( new QTcpServer ( this ) )
Expand Down Expand Up @@ -60,7 +61,7 @@ bool CRpcServer::Start()
{
return false;
}
if ( pTransportServer->listen ( QHostAddress ( JSON_RPC_LISTEN_ADDRESS ), iPort ) )
if ( pTransportServer->listen ( QHostAddress ( strBindIP ), iPort ) )
{
qInfo() << qUtf8Printable ( QString ( "- JSON-RPC: Server started on %1:%2" )
.arg ( pTransportServer->serverAddress().toString() )
Expand Down
3 changes: 2 additions & 1 deletion src/rpcserver.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class CRpcServer : public QObject
Q_OBJECT

public:
CRpcServer ( QObject* parent, int iPort, QString secret );
CRpcServer ( QObject* parent, QString strBindIP, int iPort, QString secret );
virtual ~CRpcServer();

bool Start();
Expand All @@ -64,6 +64,7 @@ class CRpcServer : public QObject
static const int iErrUnauthenticated = 401;

private:
QString strBindIP;
int iPort;
QString strSecret;
QTcpServer* pTransportServer;
Expand Down
3 changes: 3 additions & 0 deletions tools/generate_json_rpc_docs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,9 @@ def to_markdown(self):
$ openssl rand -base64 10 > /file/with/a/secret.txt
```
The JSON-RPC server defaults to listening on the local loopback network interface (127.0.0.1). This can be optionally changed by using the `--jsonrpcbindip <ip address>` command line option. **IPv4 only. IPv6 support has not been tested.**
## Wire protocol
The JSON-RPC server is based on the [JSON-RPC 2.0](https://www.jsonrpc.org/specification) protocol, using [streaming newline-delimited JSON over TCP](https://clue.engineering/2018/introducing-reactphp-ndjson) as the transport. There are three main types of messages being exchanged:
Expand Down