Skip to content

websocketserver: Allow the user to bind to a specific address #1279

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+29 −3
Open

websocketserver: Allow the user to bind to a specific address #1279

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
460003f
websocketserver: Allow the user to bind to a specific address
silasary Jan 22, 2025
75a4670
Address feedback
silasary Mar 13, 2025
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
17 changes: 17 additions & 0 deletions src/Config.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,13 @@ with this program. If not, see <https://www.gnu.org/licenses/>
#define PARAM_FIRSTLOAD "first_load"
#define PARAM_ENABLED "server_enabled"
#define PARAM_PORT "server_port"
#define PARAM_HOST "server_host"
#define PARAM_ALERTS "alerts_enabled"
#define PARAM_AUTHREQUIRED "auth_required"
#define PARAM_PASSWORD "server_password"

#define CMDLINE_WEBSOCKET_PORT "websocket_port"
#define CMDLINE_WEBSOCKET_HOST "websocket_host"
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 that I would like this to use address instead of host. address would likely be less ambiguous.

Copy link
Author

Choose a reason for hiding this comment

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

I used host because that's what websocketpp called it, but agreed, address is a better name.

#define CMDLINE_WEBSOCKET_IPV4_ONLY "websocket_ipv4_only"
#define CMDLINE_WEBSOCKET_PASSWORD "websocket_password"
#define CMDLINE_WEBSOCKET_DEBUG "websocket_debug"
Expand Down Expand Up @@ -72,6 +74,8 @@ void Config::Load(json config)
AuthRequired = config[PARAM_AUTHREQUIRED];
if (config.contains(PARAM_PASSWORD) && config[PARAM_PASSWORD].is_string())
ServerPassword = config[PARAM_PASSWORD];
if (config.contains(PARAM_HOST) && config[PARAM_HOST].is_string())
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: Move this and other occurrences above the port override

ServerHost = config[PARAM_HOST];

// Set server password and save it to the config before processing overrides,
// so that there is always a true configured password regardless of if
Expand Down Expand Up @@ -105,6 +109,17 @@ void Config::Load(json config)
}
}

// Process `--websocket_host` override
QString host_argument = Utils::Platform::GetCommandLineArgument(CMDLINE_WEBSOCKET_HOST);
if (host_argument != "") {
blog(LOG_INFO, "[Config::Load] --websocket_host passed. Overriding WebSocket host with: %s",
host_argument.toStdString().c_str());
ServerHost = host_argument.toStdString();
HostOverridden = true;
}



// Process `--websocket_ipv4_only` override
if (Utils::Platform::GetCommandLineFlagSet(CMDLINE_WEBSOCKET_IPV4_ONLY)) {
blog(LOG_INFO, "[Config::Load] --websocket_ipv4_only passed. Binding only to IPv4 interfaces.");
Expand Down Expand Up @@ -139,6 +154,8 @@ void Config::Save()
config[PARAM_ENABLED] = ServerEnabled.load();
if (!PortOverridden)
config[PARAM_PORT] = ServerPort.load();
if (!HostOverridden)
config[PARAM_HOST] = ServerHost;
config[PARAM_ALERTS] = AlertsEnabled.load();
if (!PasswordOverridden) {
config[PARAM_AUTHREQUIRED] = AuthRequired.load();
Expand Down
2 changes: 2 additions & 0 deletions src/Config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@ struct Config {

std::atomic<bool> PortOverridden = false;
std::atomic<bool> PasswordOverridden = false;
std::atomic<bool> HostOverridden = false;

std::atomic<bool> FirstLoad = true;
std::atomic<bool> ServerEnabled = false;
std::string ServerHost;
std::atomic<uint16_t> ServerPort = 4455;
std::atomic<bool> Ipv4Only = false;
std::atomic<bool> DebugEnabled = false;
Expand Down
2 changes: 2 additions & 0 deletions src/forms/ConnectInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ void ConnectInfo::RefreshData()
}

QString serverIp = QString::fromStdString(Utils::Platform::GetLocalAddress());
if (conf->ServerHost != "")
serverIp = QString::fromStdString(conf->ServerHost);
ui->serverIpLineEdit->setText(serverIp);

QString serverPort = QString::number(conf->ServerPort);
Expand Down
5 changes: 4 additions & 1 deletion src/websocketserver/WebSocketServer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,10 @@ void WebSocketServer::Start()
_server.reset();

websocketpp::lib::error_code errorCode;
if (conf->Ipv4Only) {
if (conf->ServerHost != "") {
blog(LOG_INFO, "[WebSocketServer::Start] Locked to %s", conf->ServerHost);
_server.listen(conf->ServerHost, std::to_string(conf->ServerPort), errorCode);
Copy link
Member

Choose a reason for hiding this comment

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

I'm looking at the websocketpp documentation, and I'm not seeing a matching function for listen(). Do you know which function it's being implicitly cast to? I'm guessing listen(InternetProtocol const &internet_protocol, uint16_t port, lib::error_code &ec)

It may be safer in this specific situation to first create an InternetProtocol object, then pass it to the listen function.

Copy link
Author

@silasary silasary Feb 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I'm using listen( std::string const &host, std::string const &service, lib::error_code &ec) docs here, no implicit cast.

As far as I could tell, InternetProtocol doesn't let you bind to a specific address.

Copy link
Member

Choose a reason for hiding this comment

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

This feedback thread is still unresolved. What is the path forward here?

Copy link
Author

Choose a reason for hiding this comment

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

The feedback was incorrect. Nothing to change.

} else if (conf->Ipv4Only) {
blog(LOG_INFO, "[WebSocketServer::Start] Locked to IPv4 bindings");
_server.listen(websocketpp::lib::asio::ip::tcp::v4(), conf->ServerPort, errorCode);
} else {
Expand Down