Browse Source

NetworkPluginServer: handle argc/argv sanely when spawning backends

- The old code did some really silly things when spawning backends -
  on the level of "make the arguments into one string and then parse" (!)
- We now actually use std::vector and do this the competent way.
- Plus, no more dependency on popt, which I can't believe was ever
  included...!
- execvp() instead of execv() is used, so you don't have to specify the
  full PATH (useful for nixos)
master
eta 3 years ago
parent
commit
2fc832b72e
  1. 3
      CMakeLists.txt
  2. 4
      backends/libpurple/main.cpp
  3. 10
      cmake_modules/Findpopt.cmake
  4. 3
      include/transport/Config.h
  5. 1
      include/transport/NetworkPluginServer.h
  6. 14
      libtransport/Config.cpp
  7. 95
      libtransport/NetworkPluginServer.cpp
  8. 2
      nix/default.nix

3
CMakeLists.txt

@ -55,9 +55,6 @@ if(${Boost_VERSION} GREATER 104999) @@ -55,9 +55,6 @@ if(${Boost_VERSION} GREATER 104999)
add_definitions(-DBOOST_FILESYSTEM_VERSION=3)
endif()
# FIND POPT
find_package(popt REQUIRED)
###### Database ######
# FIND SQLITE3

4
backends/libpurple/main.cpp

@ -263,10 +263,6 @@ class SpectrumNetworkPlugin : public NetworkPlugin { @@ -263,10 +263,6 @@ class SpectrumNetworkPlugin : public NetworkPlugin {
void getProtocolAndName(const std::string &legacyName, std::string &name, std::string &protocol) {
name = legacyName;
protocol = CONFIG_STRING(config, "service.protocol");
if (protocol == "any") {
protocol = name.substr(0, name.find("."));
name = name.substr(name.find(".") + 1);
}
}
void setDefaultAvatar(PurpleAccount *account, const std::string &legacyName) {

10
cmake_modules/Findpopt.cmake

@ -1,10 +0,0 @@ @@ -1,10 +0,0 @@
FIND_LIBRARY(POPT_LIBRARY NAMES popt)
FIND_PATH(POPT_INCLUDE_DIR NAMES "popt.h")
if( POPT_LIBRARY AND POPT_INCLUDE_DIR )
message( STATUS "Found popt: ${POPT_LIBRARY}, ${POPT_INCLUDE_DIR}")
set( POPT_FOUND 1 )
else( POPT_LIBRARY AND POPT_INCLUDE_DIR )
message( STATUS "Could NOT find popt" )
endif( POPT_LIBRARY AND POPT_INCLUDE_DIR )

3
include/transport/Config.h

@ -123,7 +123,8 @@ class Config { @@ -123,7 +123,8 @@ class Config {
SectionValuesCont getSectionValues(const std::string &sectionName);
std::string getCommandLineArgs() const;
int getArgc();
char **getArgv();
/// Returns path to config file from which data were loaded.
const std::string &getConfigFile() {

1
include/transport/NetworkPluginServer.h

@ -105,6 +105,7 @@ class NetworkPluginServer : Swift::XMPPParserClient { @@ -105,6 +105,7 @@ class NetworkPluginServer : Swift::XMPPParserClient {
void handleMessageReceived(NetworkConversation *conv, std::shared_ptr<Swift::Message> &message);
public:
unsigned long spawnBackend(const char *backend_id);
void handleNewClientConnection(std::shared_ptr<Swift::Connection> c);
void handleSessionFinished(Backend *c);
void handlePongReceived(Backend *c);

14
libtransport/Config.cpp

@ -330,16 +330,12 @@ Config::SectionValuesCont Config::getSectionValues(const std::string &sectionNam @@ -330,16 +330,12 @@ Config::SectionValuesCont Config::getSectionValues(const std::string &sectionNam
return sectionValues;
}
std::string Config::getCommandLineArgs() const {
std::ostringstream commandLineArgs;
// Return the command-line arguments that were passed to us originally (but
// remove the initial .exe part)
for (int i = 1; i < m_argc; ++i) {
commandLineArgs << "\"" << m_argv[i] << "\" ";
}
int Config::getArgc() {
return m_argc;
}
return commandLineArgs.str();
char **Config::getArgv() {
return m_argv;
}
void Config::updateBackendConfig(const std::string &backendConfig) {

95
libtransport/NetworkPluginServer.cpp

@ -58,7 +58,6 @@ @@ -58,7 +58,6 @@
#include <signal.h>
#include <sys/types.h>
#include "popt.h"
#include "sys/signal.h"
#include "sys/wait.h"
@ -134,49 +133,6 @@ class NetworkFactory : public Factory { @@ -134,49 +133,6 @@ class NetworkFactory : public Factory {
wrap.SerializeToString(&MESSAGE);
// Executes new backend
static unsigned long exec_(const std::string &exePath, const char *host, const char *port, const char *log_id,
const char *cmdlineArgs) {
// BACKEND_ID is replaced with unique ID. The ID is increasing for every
// backend.
std::string finalExePath =
boost::replace_all_copy(exePath, "BACKEND_ID", boost::lexical_cast<std::string>(backend_id++));
// Add host and port.
finalExePath +=
std::string(" --host ") + host + " --port " + port + " --service.backend_id=" + log_id + " " + cmdlineArgs;
LOG4CXX_INFO(logger, "Starting new backend " << finalExePath);
// Create array of char * from string using -lpopt library
char *p = (char *)malloc(finalExePath.size() + 1);
strcpy(p, finalExePath.c_str());
int argc;
char **argv;
poptParseArgvString(p, &argc, (const char ***)&argv);
// fork and exec
pid_t pid = fork();
if (pid == 0) {
setsid();
// close all files
int maxfd = sysconf(_SC_OPEN_MAX);
for (int fd = 3; fd < maxfd; fd++) {
close(fd);
}
// child process
errno = 0;
int ret = execv(argv[0], argv);
if (ret == -1) {
exit(errno);
}
exit(0);
} else if (pid < 0) {
LOG4CXX_ERROR(logger, "Fork failed");
}
free(p);
return (unsigned long)pid;
}
static void SigCatcher(int n) {
pid_t result;
int status;
@ -304,6 +260,46 @@ NetworkPluginServer::~NetworkPluginServer() { @@ -304,6 +260,46 @@ NetworkPluginServer::~NetworkPluginServer() {
// delete m_rosterResponder;
// delete m_blockResponder;
}
unsigned long NetworkPluginServer::spawnBackend(const char *backend_id) {
std::string binaryPath = CONFIG_STRING(m_config, "service.backend");
std::vector<const char *> binaryArgs = {binaryPath.c_str()};
binaryArgs.push_back("--host");
binaryArgs.push_back(CONFIG_STRING(m_config, "service.backend_host").c_str());
binaryArgs.push_back("--port");
binaryArgs.push_back(CONFIG_STRING(m_config, "service.backend_port").c_str());
char **origArgv = m_config->getArgv();
for (int i = 1; i < m_config->getArgc(); i++) {
binaryArgs.push_back(origArgv[i]);
}
std::string debugBackendCommand = "";
for (const char *thing : binaryArgs) {
debugBackendCommand += thing;
debugBackendCommand += " ";
}
LOG4CXX_INFO(logger, "Executing: " << debugBackendCommand);
binaryArgs.push_back(NULL); // indicate end of array
const char **argv = binaryArgs.data();
// fork and exec
pid_t pid = fork();
if (pid == 0) {
setsid();
// child process
int ret = execvp(argv[0], (char *const *)argv);
if (ret == -1) {
perror("execvp");
exit(errno);
}
} else if (pid < 0) {
LOG4CXX_ERROR(logger, "Fork failed");
}
return (unsigned long)pid;
}
void NetworkPluginServer::start() {
m_server->start();
@ -312,9 +308,7 @@ void NetworkPluginServer::start() { @@ -312,9 +308,7 @@ void NetworkPluginServer::start() {
<< CONFIG_STRING(m_config, "service.backend_port"));
while (true) {
unsigned long pid =
exec_(CONFIG_STRING(m_config, "service.backend"), CONFIG_STRING(m_config, "service.backend_host").c_str(),
CONFIG_STRING(m_config, "service.backend_port").c_str(), "1", m_config->getCommandLineArgs().c_str());
unsigned long pid = spawnBackend("1");
LOG4CXX_INFO(logger, "Tried to spawn first backend with pid " << pid);
LOG4CXX_INFO(logger, "Backend should now connect to Spectrum2 instance. Spectrum2 "
"won't accept any connection before backend connects");
@ -2016,7 +2010,6 @@ void NetworkPluginServer::handlePIDTerminated(unsigned long pid) { @@ -2016,7 +2010,6 @@ void NetworkPluginServer::handlePIDTerminated(unsigned long pid) {
}
}
static int sig_block_count = 0;
static sigset_t block_mask;
@ -2056,7 +2049,6 @@ static void __unblock_signals(void) { @@ -2056,7 +2049,6 @@ static void __unblock_signals(void) {
}
}
NetworkPluginServer::Backend *NetworkPluginServer::getFreeClient(bool acceptUsers, bool longRun, bool check) {
NetworkPluginServer::Backend *c = NULL;
@ -2107,10 +2099,7 @@ NetworkPluginServer::Backend *NetworkPluginServer::getFreeClient(bool acceptUser @@ -2107,10 +2099,7 @@ NetworkPluginServer::Backend *NetworkPluginServer::getFreeClient(bool acceptUser
} else {
log_id = boost::lexical_cast<std::string>(log_id_it - m_pids.begin() + 1);
}
unsigned long pid =
exec_(CONFIG_STRING(m_config, "service.backend"), CONFIG_STRING(m_config, "service.backend_host").c_str(),
CONFIG_STRING(m_config, "service.backend_port").c_str(), log_id.c_str(),
m_config->getCommandLineArgs().c_str());
unsigned long pid = spawnBackend(log_id.c_str());
if (log_id_it == m_pids.end()) {
m_pids.push_back(pid);
} else {

2
nix/default.nix

@ -23,5 +23,5 @@ stdenv.mkDerivation { @@ -23,5 +23,5 @@ stdenv.mkDerivation {
(path: type: !(lib.strings.hasSuffix (baseNameOf path) "nix") && filter path type)
../.;
nativeBuildInputs = [ cmake pkg-config ];
buildInputs = [ pkgs-eta.swiften zlib expat boost pqxx pqxx.pkgconfig postgresql.lib libpurple libev sqlite log4cxx glib pcre boost protobuf curl jsoncpp popt ];
buildInputs = [ pkgs-eta.swiften zlib expat boost pqxx pqxx.pkgconfig postgresql.lib libpurple libev sqlite log4cxx glib pcre boost protobuf curl jsoncpp ];
}

Loading…
Cancel
Save