From 51e93a84a8d0248460201f5ac3eb096652830a9b Mon Sep 17 00:00:00 2001 From: Serra Date: Fri, 30 May 2025 11:14:01 -0700 Subject: [PATCH 01/16] Added boost path for Mac Silicon Brew installations --- build.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.rs b/build.rs index 370ee00fb..22b47041d 100644 --- a/build.rs +++ b/build.rs @@ -353,7 +353,7 @@ fn get_qt_lib_prefix(lib_dir: &Path, version_maj: u16) -> Result Result> { - let paths = ["/usr/local/include", "/usr/include"]; + let paths = ["/usr/local/include", "/usr/include", "/opt/homebrew/include"]; let version_filename = "boost/version.hpp"; for path in paths { From 8dd26f26cc92902aaa99f7133c4f71860c238898 Mon Sep 17 00:00:00 2001 From: Serra Date: Mon, 2 Jun 2025 12:12:28 -0700 Subject: [PATCH 02/16] Adds C compatible struct and conversion function --- src/bin/pushpin-handler.rs | 135 +++++++++++++++++++++- src/handler/handlerapp.cpp | 221 +++++++++++++++++++++++++++++++++++++ 2 files changed, 353 insertions(+), 3 deletions(-) diff --git a/src/bin/pushpin-handler.rs b/src/bin/pushpin-handler.rs index 7c816fc02..9b197f3cd 100644 --- a/src/bin/pushpin-handler.rs +++ b/src/bin/pushpin-handler.rs @@ -14,15 +14,144 @@ * limitations under the License. */ -use pushpin::core::call_c_main; +use clap::builder::Str; +use clap::{Parser, arg}; +use pushpin::core::{call_c_main, version}; use pushpin::import_cpp; use std::env; +use std::ffi::CString; use std::process::ExitCode; +// import_cpp! { +// fn handler_main(argc: libc::c_int, argv: *const *const libc::c_char) -> libc::c_int; +// } + import_cpp! { - fn handler_main(argc: libc::c_int, argv: *const *const libc::c_char) -> libc::c_int; + fn handler_main(args: *const CCliArgs) -> libc::c_int; +} + + +// Struct to hold the command line arguments +#[derive(Parser, Debug)] +#[command( + name= "Pushpin Handler", + version = version(), + about = "Pushpin handler component." +)] +pub struct CliArgs { + /// Set path to the configuration file + #[arg(short, long, value_name = "file", default_value = "pushpin.conf")] + pub config_file: String, + + /// Set path to the log file + #[arg(short = 'l', long, value_name = "file")] + pub log_file: Option, + + /// Set log level + #[arg(short = 'L', long, value_name = "x", default_value = "2")] + pub log_level: String, + + /// Override ipc_prefix config option, which is used to add a prefix to all ZeroMQ IPC filenames + #[arg(long, value_name = "prefix")] + pub ipc_prefix: Option, + + /// Override port_offset config option, which is used to increment all ZeroMQ TCP ports and the HTTP control server port + #[arg(long, value_name = "offset")] + pub port_offset: Option, + + /// Enable verbose output. Same as --loglevel=3. + #[arg(short, long, action = clap::ArgAction::SetTrue, default_value_t = false)] + pub verbose: bool, } +impl CliArgs { + /// Verifies the command line arguments and returns a new instance of `CliArgs`. + pub fn verify(self) -> Self { + // Check if the configuration file exists and is readable + // Check if log file is specified and valid + // Check if log level is within the valid range + // Check if ipc_prefix is a valid string + // Check if port_offset is within the valid range + self + } + + pub fn into_c_struct(self) -> CArgsData { + CArgsData { + config_file: CString::new(self.config_file).unwrap().into_raw(), + log_file: self.log_file.as_ref().map_or_else( + || CString::new("").unwrap().into_raw(), + |s| CString::new(s).unwrap().into_raw(), + ), + log_level: self.log_level.parse::().unwrap_or(2), + ipc_prefix: self.ipc_prefix.as_ref().map_or_else( + || CString::new("").unwrap().into_raw(), + |s| CString::new(s).unwrap().into_raw(), + ), + port_offset: self.port_offset + .as_ref() + .and_then(|s| s.parse::().ok()) + .unwrap_or(-1), + verbose: self.verbose, + } + } +} + +impl IntoIterator for CliArgs { + type Item = (String, String); + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + let mut args: Vec<(String, String)> = vec![]; + + args.push(("config-file".to_string(), self.config_file)); + + if let Some(log_file) = self.log_file { + args.push(("log-file".to_string(), log_file)); + } + + args.push(("log-level".to_string(), self.log_level)); + + if let Some(ipc_prefix) = self.ipc_prefix { + args.push(("ipc-prefix".to_string(), ipc_prefix)); + } + + if let Some(port_offset) = self.port_offset { + args.push(("port-offset".to_string(), port_offset)); + } + + if self.verbose { + args.push(("verbose".to_string(), "true".to_string())); + } + + args.into_iter() + } +} + +// C-compatible struct that matches CliArgs in handlerapp.cpp +#[repr(C)] +pub struct CCliArgs { + pub config_file: *const libc::c_char, + pub log_file: *const libc::c_char, + pub log_level: libc::c_int, + pub ipc_prefix: *const libc::c_char, + pub port_offset: libc::c_int, + pub verbose: bool, +} + +// Modify the handler_main function signature to accept our struct +// import_cpp! { +// fn handler_main_with_args(args: *const CArgsData) -> libc::c_int; +// } + fn main() -> ExitCode { - unsafe { ExitCode::from(call_c_main(handler_main, env::args_os())) } + let cli_args = CliArgs::parse().verify(); + + // Call the C++ function with our struct + unsafe { + // We need to keep the CStrings alive until the C++ function returns + let result = handler_main(&cli_args.into_c_struct()); + ExitCode::from(result) + } + + // unsafe { ExitCode::from(call_c_main(handler_main, env::args_os())) } } diff --git a/src/handler/handlerapp.cpp b/src/handler/handlerapp.cpp index 7a7ee85bb..8708bbcaa 100644 --- a/src/handler/handlerapp.cpp +++ b/src/handler/handlerapp.cpp @@ -497,3 +497,224 @@ int HandlerApp::run() { return Private::run(); } + +// Export a function that accepts the Rust-compatible struct +extern "C" { + int handler_main_with_args(const ArgsData* args) + { + // Setup QCoreApplication without command line args + int argc = 1; + char* argv[] = { (char*)"pushpin-handler" }; + QCoreApplication app(argc, argv); + + QCoreApplication::setApplicationName("pushpin-handler"); + QCoreApplication::setApplicationVersion(Config::get().version); + + // Create a local ArgsData from the passed struct + ArgsData localArgs; + + if(args->config_file && *args->config_file) + localArgs.configFile = QString::fromUtf8(args->config_file); + + if(args->log_file && *args->log_file) + localArgs.logFile = QString::fromUtf8(args->log_file); + + localArgs.logLevel = args->log_level; + + if(args->ipc_prefix && *args->ipc_prefix) + localArgs.ipcPrefix = QString::fromUtf8(args->ipc_prefix); + + localArgs.portOffset = args->port_offset; + + // Handle verbose flag + if(args->verbose && localArgs.logLevel < 3) + localArgs.logLevel = 3; + + // Configure logging + if(localArgs.logLevel != -1) + log_setOutputLevel(localArgs.logLevel); + else + log_setOutputLevel(LOG_LEVEL_INFO); + + if(!localArgs.logFile.isEmpty()) + { + if(!log_setFile(localArgs.logFile)) + { + log_error("failed to open log file: %s", qPrintable(localArgs.logFile)); + return 1; + } + } + + log_debug("starting..."); + + QString configFile = localArgs.configFile; + if(configFile.isEmpty()) + configFile = QDir(Config::get().configDir).filePath("pushpin.conf"); + + // QSettings doesn't inform us if the config file doesn't exist, so do that ourselves + { + QFile file(configFile); + if(!file.open(QIODevice::ReadOnly)) + { + log_error("failed to open %s, and --config not passed", qPrintable(configFile)); + return 1; + } + } + + Settings settings(configFile); + + if(!localArgs.ipcPrefix.isEmpty()) + settings.setIpcPrefix(localArgs.ipcPrefix); + + if(localArgs.portOffset != -1) + settings.setPortOffset(localArgs.portOffset); + + QStringList services = settings.value("runner/services").toStringList(); + + QStringList connmgr_in_stream_specs = settings.value("proxy/connmgr_in_stream_specs").toStringList(); + trimlist(&connmgr_in_stream_specs); + QStringList connmgr_out_specs = settings.value("proxy/connmgr_out_specs").toStringList(); + trimlist(&connmgr_out_specs); + QStringList condure_in_stream_specs = settings.value("proxy/condure_in_stream_specs").toStringList(); + trimlist(&condure_in_stream_specs); + connmgr_in_stream_specs += condure_in_stream_specs; + QStringList condure_out_specs = settings.value("proxy/condure_out_specs").toStringList(); + trimlist(&condure_out_specs); + connmgr_out_specs += condure_out_specs; + int proxyWorkerCount = settings.value("proxy/workers", 1).toInt(); + QStringList m2a_in_stream_specs = settings.value("handler/m2a_in_stream_specs").toStringList(); + trimlist(&m2a_in_stream_specs); + QStringList m2a_out_specs = settings.value("handler/m2a_out_specs").toStringList(); + trimlist(&m2a_out_specs); + QStringList intreq_out_specs = settings.value("handler/proxy_intreq_out_specs").toStringList(); + trimlist(&intreq_out_specs); + QStringList intreq_out_stream_specs = settings.value("handler/proxy_intreq_out_stream_specs").toStringList(); + trimlist(&intreq_out_stream_specs); + QStringList intreq_in_specs = settings.value("handler/proxy_intreq_in_specs").toStringList(); + trimlist(&intreq_in_specs); + QStringList proxy_inspect_specs = settings.value("handler/proxy_inspect_specs").toStringList(); + trimlist(&proxy_inspect_specs); + QString proxy_inspect_spec = settings.value("handler/proxy_inspect_spec").toString(); + if(!proxy_inspect_spec.isEmpty()) + proxy_inspect_specs += proxy_inspect_spec; + QStringList proxy_accept_specs = settings.value("handler/proxy_accept_specs").toStringList(); + trimlist(&proxy_accept_specs); + QString proxy_accept_spec = settings.value("handler/proxy_accept_spec").toString(); + if(!proxy_accept_spec.isEmpty()) + proxy_accept_specs += proxy_accept_spec; + QStringList proxy_retry_out_specs = settings.value("handler/proxy_retry_out_specs").toStringList(); + trimlist(&proxy_retry_out_specs); + QString proxy_retry_out_spec = settings.value("handler/proxy_retry_out_spec").toString(); + if(!proxy_retry_out_spec.isEmpty()) + proxy_retry_out_specs += proxy_retry_out_spec; + QStringList ws_control_init_specs = settings.value("handler/proxy_ws_control_init_specs").toStringList(); + trimlist(&ws_control_init_specs); + QStringList ws_control_stream_specs = settings.value("handler/proxy_ws_control_stream_specs").toStringList(); + trimlist(&ws_control_stream_specs); + QString stats_spec = settings.value("handler/stats_spec").toString(); + QString command_spec = settings.value("handler/command_spec").toString(); + QString state_spec = settings.value("handler/state_spec").toString(); + QStringList proxy_stats_specs = settings.value("handler/proxy_stats_specs").toStringList(); + trimlist(&proxy_stats_specs); + QString proxy_stats_spec = settings.value("handler/proxy_stats_spec").toString(); + if(!proxy_stats_spec.isEmpty()) + proxy_stats_specs += proxy_stats_spec; + QString proxy_command_spec = settings.value("handler/proxy_command_spec").toString(); + QString push_in_spec = settings.value("handler/push_in_spec").toString(); + QStringList push_in_sub_specs = settings.value("handler/push_in_sub_specs").toStringList(); + trimlist(&push_in_sub_specs); + QString push_in_sub_spec = settings.value("handler/push_in_sub_spec").toString(); + if(!push_in_sub_spec.isEmpty()) + push_in_sub_specs += push_in_sub_spec; + bool push_in_sub_connect = settings.value("handler/push_in_sub_connect").toBool(); + QString push_in_http_addr = settings.value("handler/push_in_http_addr").toString(); + int push_in_http_port = settings.adjustedPort("handler/push_in_http_port"); + int push_in_http_max_headers_size = settings.value("handler/push_in_http_max_headers_size", DEFAULT_HTTP_MAX_HEADERS_SIZE).toInt(); + int push_in_http_max_body_size = settings.value("handler/push_in_http_max_body_size", DEFAULT_HTTP_MAX_BODY_SIZE).toInt(); + bool ok; + int ipcFileMode = settings.value("handler/ipc_file_mode", -1).toString().toInt(&ok, 8); + bool shareAll = settings.value("handler/share_all").toBool(); + int messageRate = settings.value("handler/message_rate", -1).toInt(); + int messageHwm = settings.value("handler/message_hwm", -1).toInt(); + int messageBlockSize = settings.value("handler/message_block_size", -1).toInt(); + int messageWait = settings.value("handler/message_wait", 5000).toInt(); + int idCacheTtl = settings.value("handler/id_cache_ttl", 0).toInt(); + bool updateOnFirstSubscription = settings.value("handler/update_on_first_subscription", true).toBool(); + int clientMaxconn = settings.value("runner/client_maxconn", 50000).toInt(); + int connectionSubscriptionMax = settings.value("handler/connection_subscription_max", 20).toInt(); + int subscriptionLinger = settings.value("handler/subscription_linger", 60).toInt(); + int statsConnectionSend = settings.value("global/stats_connection_send", true).toBool(); + int statsConnectionTtl = settings.value("global/stats_connection_ttl", 120).toInt(); + int statsSubscriptionTtl = settings.value("handler/stats_subscription_ttl", 60).toInt(); + int statsReportInterval = settings.value("handler/stats_report_interval", 10).toInt(); + QString statsFormat = settings.value("handler/stats_format").toString(); + QString prometheusPort = settings.value("handler/prometheus_port").toString(); + QString prometheusPrefix = settings.value("handler/prometheus_prefix").toString(); + + if(m2a_in_stream_specs.isEmpty() || m2a_out_specs.isEmpty()) + { + log_error("must set m2a_in_stream_specs and m2a_out_specs"); + return 1; + } + + if(proxy_inspect_specs.isEmpty() || proxy_accept_specs.isEmpty() || proxy_retry_out_specs.isEmpty()) + { + log_error("must set proxy_inspect_specs, proxy_accept_specs, and proxy_retry_out_specs"); + return 1; + } + + HandlerEngine::Configuration config; + config.appVersion = Config::get().version; + config.instanceId = "handler_" + QByteArray::number(QCoreApplication::applicationPid()); + if(!services.contains("mongrel2") && (!connmgr_in_stream_specs.isEmpty() || !connmgr_out_specs.isEmpty())) + { + config.serverInStreamSpecs = connmgr_in_stream_specs; + config.serverOutSpecs = connmgr_out_specs; + } + else + { + config.serverInStreamSpecs = m2a_in_stream_specs; + config.serverOutSpecs = m2a_out_specs; + } + config.clientOutSpecs = expandSpecs(intreq_out_specs, proxyWorkerCount); + config.clientOutStreamSpecs = expandSpecs(intreq_out_stream_specs, proxyWorkerCount); + config.clientInSpecs = expandSpecs(intreq_in_specs, proxyWorkerCount); + config.inspectSpecs = expandSpecs(proxy_inspect_specs, proxyWorkerCount); + config.acceptSpecs = expandSpecs(proxy_accept_specs, proxyWorkerCount); + config.retryOutSpecs = expandSpecs(proxy_retry_out_specs, proxyWorkerCount); + config.wsControlInitSpecs = expandSpecs(ws_control_init_specs, proxyWorkerCount); + config.wsControlStreamSpecs = expandSpecs(ws_control_stream_specs, proxyWorkerCount); + config.statsSpec = stats_spec; + config.commandSpec = command_spec; + config.stateSpec = state_spec; + config.proxyStatsSpecs = expandSpecs(proxy_stats_specs, proxyWorkerCount); + config.proxyCommandSpec = firstSpec(proxy_command_spec, proxyWorkerCount); + config.pushInSpec = push_in_spec; + config.pushInSubSpecs = push_in_sub_specs; + config.pushInSubConnect = push_in_sub_connect; + config.pushInHttpAddr = QHostAddress(push_in_http_addr); + config.pushInHttpPort = push_in_http_port; + config.pushInHttpMaxHeadersSize = push_in_http_max_headers_size; + config.pushInHttpMaxBodySize = push_in_http_max_body_size; + config.ipcFileMode = ipcFileMode; + config.shareAll = shareAll; + config.messageRate = messageRate; + config.messageHwm = messageHwm; + config.messageBlockSize = messageBlockSize; + config.messageWait = messageWait; + config.idCacheTtl = idCacheTtl; + config.updateOnFirstSubscription = updateOnFirstSubscription; + config.connectionsMax = clientMaxconn; + config.connectionSubscriptionMax = connectionSubscriptionMax; + config.subscriptionLinger = subscriptionLinger; + config.statsConnectionSend = statsConnectionSend; + config.statsConnectionTtl = statsConnectionTtl; + config.statsSubscriptionTtl = statsSubscriptionTtl; + config.statsReportInterval = statsReportInterval; + config.statsFormat = statsFormat; + config.prometheusPort = prometheusPort; + config.prometheusPrefix = prometheusPrefix; + + return runLoop(config, true); + } +} From 1a9c8c3839d67ad3e5a14758e27790d851c31241 Mon Sep 17 00:00:00 2001 From: Serra Date: Fri, 6 Jun 2025 10:51:33 -0700 Subject: [PATCH 03/16] Updates call_c_main call to include new arguments --- src/bin/pushpin-handler.rs | 75 +++++++++++++++++--------------------- 1 file changed, 33 insertions(+), 42 deletions(-) diff --git a/src/bin/pushpin-handler.rs b/src/bin/pushpin-handler.rs index 9b197f3cd..0942f64da 100644 --- a/src/bin/pushpin-handler.rs +++ b/src/bin/pushpin-handler.rs @@ -22,12 +22,8 @@ use std::env; use std::ffi::CString; use std::process::ExitCode; -// import_cpp! { -// fn handler_main(argc: libc::c_int, argv: *const *const libc::c_char) -> libc::c_int; -// } - import_cpp! { - fn handler_main(args: *const CCliArgs) -> libc::c_int; + fn handler_main(argc: libc::c_int, argv: *const *const libc::c_char) -> libc::c_int; } @@ -64,6 +60,17 @@ pub struct CliArgs { pub verbose: bool, } +// C-compatible struct that matches CliArgs in handlerapp.cpp +#[repr(C)] +pub struct CArgsData { + pub config_file: *const libc::c_char, + pub log_file: *const libc::c_char, + pub log_level: *const libc::c_char, + pub ipc_prefix: *const libc::c_char, + pub port_offset: *const libc::c_char, + pub verbose: *const libc::c_char, +} + impl CliArgs { /// Verifies the command line arguments and returns a new instance of `CliArgs`. pub fn verify(self) -> Self { @@ -76,22 +83,20 @@ impl CliArgs { } pub fn into_c_struct(self) -> CArgsData { - CArgsData { - config_file: CString::new(self.config_file).unwrap().into_raw(), - log_file: self.log_file.as_ref().map_or_else( + let to_c_str = |opt: Option| -> *const libc::c_char { + opt.map_or_else( || CString::new("").unwrap().into_raw(), |s| CString::new(s).unwrap().into_raw(), - ), - log_level: self.log_level.parse::().unwrap_or(2), - ipc_prefix: self.ipc_prefix.as_ref().map_or_else( - || CString::new("").unwrap().into_raw(), - |s| CString::new(s).unwrap().into_raw(), - ), - port_offset: self.port_offset - .as_ref() - .and_then(|s| s.parse::().ok()) - .unwrap_or(-1), - verbose: self.verbose, + ) + }; + + CArgsData { + config_file: CString::new(self.config_file).unwrap().into_raw(), + log_file: to_c_str(self.log_file), + log_level: CString::new(self.log_level).unwrap().into_raw(), + ipc_prefix: to_c_str(self.ipc_prefix), + port_offset: to_c_str(self.port_offset), + verbose: CString::new(if self.verbose { "true" } else { "false" }).unwrap().into_raw(), } } } @@ -127,31 +132,17 @@ impl IntoIterator for CliArgs { } } -// C-compatible struct that matches CliArgs in handlerapp.cpp -#[repr(C)] -pub struct CCliArgs { - pub config_file: *const libc::c_char, - pub log_file: *const libc::c_char, - pub log_level: libc::c_int, - pub ipc_prefix: *const libc::c_char, - pub port_offset: libc::c_int, - pub verbose: bool, -} - -// Modify the handler_main function signature to accept our struct -// import_cpp! { -// fn handler_main_with_args(args: *const CArgsData) -> libc::c_int; -// } - fn main() -> ExitCode { - let cli_args = CliArgs::parse().verify(); + let cli_args: CArgsData = CliArgs::parse().verify().into_c_struct(); - // Call the C++ function with our struct unsafe { - // We need to keep the CStrings alive until the C++ function returns - let result = handler_main(&cli_args.into_c_struct()); - ExitCode::from(result) + ExitCode::from(call_c_main(handler_main, ( + cli_args.config_file, + cli_args.log_file, + cli_args.log_level, + cli_args.ipc_prefix, + cli_args.port_offset, + cli_args.verbose + ))) } - - // unsafe { ExitCode::from(call_c_main(handler_main, env::args_os())) } } From a74311f146b14ae46d49848f38c0f5004935ca3a Mon Sep 17 00:00:00 2001 From: Serra Date: Fri, 6 Jun 2025 11:31:47 -0700 Subject: [PATCH 04/16] Changes into_c_struct to into_osstring_vec --- src/bin/pushpin-handler.rs | 81 ++++++++++++++++++++------------------ 1 file changed, 43 insertions(+), 38 deletions(-) diff --git a/src/bin/pushpin-handler.rs b/src/bin/pushpin-handler.rs index 0942f64da..9140b059c 100644 --- a/src/bin/pushpin-handler.rs +++ b/src/bin/pushpin-handler.rs @@ -19,7 +19,7 @@ use clap::{Parser, arg}; use pushpin::core::{call_c_main, version}; use pushpin::import_cpp; use std::env; -use std::ffi::CString; +use std::ffi::{CString, OsString}; use std::process::ExitCode; import_cpp! { @@ -59,18 +59,6 @@ pub struct CliArgs { #[arg(short, long, action = clap::ArgAction::SetTrue, default_value_t = false)] pub verbose: bool, } - -// C-compatible struct that matches CliArgs in handlerapp.cpp -#[repr(C)] -pub struct CArgsData { - pub config_file: *const libc::c_char, - pub log_file: *const libc::c_char, - pub log_level: *const libc::c_char, - pub ipc_prefix: *const libc::c_char, - pub port_offset: *const libc::c_char, - pub verbose: *const libc::c_char, -} - impl CliArgs { /// Verifies the command line arguments and returns a new instance of `CliArgs`. pub fn verify(self) -> Self { @@ -79,25 +67,49 @@ impl CliArgs { // Check if log level is within the valid range // Check if ipc_prefix is a valid string // Check if port_offset is within the valid range + + // if(parser->isSet(logLevelOption)) +// { +// bool ok; +// int x = parser->value(logLevelOption).toInt(&ok); +// if(!ok || x < 0) +// { +// *errorMessage = "error: loglevel must be greater than or equal to 0"; +// return CommandLineError; +// } + +// args->logLevel = x; +// } + +// if(parser->isSet(verboseOption)) +// args->logLevel = 3; + +// if(parser->isSet(ipcPrefixOption)) +// args->ipcPrefix = parser->value(ipcPrefixOption); + +// if(parser->isSet(portOffsetOption)) +// { +// bool ok; +// int x = parser->value(portOffsetOption).toInt(&ok); +// if(!ok || x < 0) +// { +// *errorMessage = "error: port-offset must be greater than or equal to 0"; +// return CommandLineError; +// } + +// args->portOffset = x; +// } + +// return CommandLineOk; +// } + self } - pub fn into_c_struct(self) -> CArgsData { - let to_c_str = |opt: Option| -> *const libc::c_char { - opt.map_or_else( - || CString::new("").unwrap().into_raw(), - |s| CString::new(s).unwrap().into_raw(), - ) - }; - - CArgsData { - config_file: CString::new(self.config_file).unwrap().into_raw(), - log_file: to_c_str(self.log_file), - log_level: CString::new(self.log_level).unwrap().into_raw(), - ipc_prefix: to_c_str(self.ipc_prefix), - port_offset: to_c_str(self.port_offset), - verbose: CString::new(if self.verbose { "true" } else { "false" }).unwrap().into_raw(), - } + pub fn into_osstring_vec(self) -> Vec { + self.into_iter() + .map(|(_, value)| OsString::from(value)) + .collect() } } @@ -133,16 +145,9 @@ impl IntoIterator for CliArgs { } fn main() -> ExitCode { - let cli_args: CArgsData = CliArgs::parse().verify().into_c_struct(); + let cli_args = CliArgs::parse().verify(); unsafe { - ExitCode::from(call_c_main(handler_main, ( - cli_args.config_file, - cli_args.log_file, - cli_args.log_level, - cli_args.ipc_prefix, - cli_args.port_offset, - cli_args.verbose - ))) + ExitCode::from(call_c_main(handler_main, cli_args.into_osstring_vec())) } } From c56ec9d3e1bf45c152596898ca3603d2ca181566 Mon Sep 17 00:00:00 2001 From: Serra Date: Mon, 9 Jun 2025 15:32:03 -0700 Subject: [PATCH 05/16] Updates args to work with existing ffi functions and modules --- src/bin/pushpin-handler.rs | 113 +++++------- src/handler/handlerapp.cpp | 351 ++----------------------------------- src/runner/service.rs | 4 +- 3 files changed, 62 insertions(+), 406 deletions(-) diff --git a/src/bin/pushpin-handler.rs b/src/bin/pushpin-handler.rs index 9140b059c..a05fd1ff7 100644 --- a/src/bin/pushpin-handler.rs +++ b/src/bin/pushpin-handler.rs @@ -14,12 +14,13 @@ * limitations under the License. */ -use clap::builder::Str; use clap::{Parser, arg}; use pushpin::core::{call_c_main, version}; +use pushpin::core::config::get_config_file; use pushpin::import_cpp; use std::env; -use std::ffi::{CString, OsString}; +use std::ffi::{OsString}; +use std::path::PathBuf; use std::process::ExitCode; import_cpp! { @@ -36,72 +37,42 @@ import_cpp! { )] pub struct CliArgs { /// Set path to the configuration file - #[arg(short, long, value_name = "file", default_value = "pushpin.conf")] - pub config_file: String, + #[arg(short, long, value_name = "file")] + pub config_file: Option, /// Set path to the log file #[arg(short = 'l', long, value_name = "file")] pub log_file: Option, - /// Set log level - #[arg(short = 'L', long, value_name = "x", default_value = "2")] - pub log_level: String, + /// Set log level (0=error, 1=warn, 2=info, 3=debug, 4=trace) + #[arg(short = 'L', long, value_name = "x", default_value = "2", value_parser = clap::value_parser!(u32).range(1..=4))] + pub log_level: u32, /// Override ipc_prefix config option, which is used to add a prefix to all ZeroMQ IPC filenames #[arg(long, value_name = "prefix")] pub ipc_prefix: Option, /// Override port_offset config option, which is used to increment all ZeroMQ TCP ports and the HTTP control server port - #[arg(long, value_name = "offset")] - pub port_offset: Option, - - /// Enable verbose output. Same as --loglevel=3. - #[arg(short, long, action = clap::ArgAction::SetTrue, default_value_t = false)] - pub verbose: bool, + #[arg(long, value_name = "offset", value_parser = clap::value_parser!(u32))] + pub port_offset: Option, } impl CliArgs { - /// Verifies the command line arguments and returns a new instance of `CliArgs`. - pub fn verify(self) -> Self { - // Check if the configuration file exists and is readable - // Check if log file is specified and valid - // Check if log level is within the valid range - // Check if ipc_prefix is a valid string - // Check if port_offset is within the valid range - - // if(parser->isSet(logLevelOption)) -// { -// bool ok; -// int x = parser->value(logLevelOption).toInt(&ok); -// if(!ok || x < 0) -// { -// *errorMessage = "error: loglevel must be greater than or equal to 0"; -// return CommandLineError; -// } - -// args->logLevel = x; -// } - -// if(parser->isSet(verboseOption)) -// args->logLevel = 3; - -// if(parser->isSet(ipcPrefixOption)) -// args->ipcPrefix = parser->value(ipcPrefixOption); - -// if(parser->isSet(portOffsetOption)) -// { -// bool ok; -// int x = parser->value(portOffsetOption).toInt(&ok); -// if(!ok || x < 0) -// { -// *errorMessage = "error: port-offset must be greater than or equal to 0"; -// return CommandLineError; -// } - -// args->portOffset = x; -// } - -// return CommandLineOk; -// } + /// Verifies the command line arguments. + pub fn verify(mut self) -> Self { + // Get current working directory + let work_dir = env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); + + // Convert config_file Option to Option + let config_path = self.config_file.as_ref().map(|s| PathBuf::from(s)); + + // Use get_config_file to find the config file + self.config_file = match get_config_file(&work_dir, config_path) { + Ok(path) => Some(path.to_string_lossy().to_string()), + Err(e) => { + eprintln!("error: failed to find configuration file: {}", e); + std::process::exit(1); + } + }; self } @@ -120,25 +91,27 @@ impl IntoIterator for CliArgs { fn into_iter(self) -> Self::IntoIter { let mut args: Vec<(String, String)> = vec![]; - args.push(("config-file".to_string(), self.config_file)); - - if let Some(log_file) = self.log_file { - args.push(("log-file".to_string(), log_file)); - } + args.push(( + "config-file".to_string(), + self.config_file.unwrap_or_else(|| "".to_string()) + )); - args.push(("log-level".to_string(), self.log_level)); + args.push(( + "log-file".to_string(), + self.log_file.as_ref().map(|s| s.to_string()).unwrap_or_else(|| "".to_string()), + )); - if let Some(ipc_prefix) = self.ipc_prefix { - args.push(("ipc-prefix".to_string(), ipc_prefix)); - } + args.push(("log-level".to_string(), self.log_level.to_string())); - if let Some(port_offset) = self.port_offset { - args.push(("port-offset".to_string(), port_offset)); - } + args.push(( + "ipc-prefix".to_string(), + self.ipc_prefix.as_ref().map(|s| s.to_string()).unwrap_or_else(|| "".to_string()), + )); - if self.verbose { - args.push(("verbose".to_string(), "true".to_string())); - } + args.push(( + "port-offset".to_string(), + self.port_offset.as_ref().map(|s| s.to_string()).unwrap_or_else(|| "".to_string()), + )); args.into_iter() } diff --git a/src/handler/handlerapp.cpp b/src/handler/handlerapp.cpp index 8708bbcaa..70540aaad 100644 --- a/src/handler/handlerapp.cpp +++ b/src/handler/handlerapp.cpp @@ -42,6 +42,7 @@ #include "settings.h" #include "handlerengine.h" #include "config.h" +#include #define DEFAULT_HTTP_MAX_HEADERS_SIZE 10000 #define DEFAULT_HTTP_MAX_BODY_SIZE 1000000 @@ -81,15 +82,6 @@ static QString firstSpec(const QString &s, int peerCount) return s; } - -enum CommandLineParseResult -{ - CommandLineOk, - CommandLineError, - CommandLineVersionRequested, - CommandLineHelpRequested -}; - class ArgsData { public: @@ -98,85 +90,8 @@ class ArgsData int logLevel; QString ipcPrefix; int portOffset; - - ArgsData() : - logLevel(-1), - portOffset(-1) - { - } }; -static CommandLineParseResult parseCommandLine(QCommandLineParser *parser, ArgsData *args, QString *errorMessage) -{ - parser->setSingleDashWordOptionMode(QCommandLineParser::ParseAsLongOptions); - const QCommandLineOption configFileOption("config", "Config file.", "file"); - parser->addOption(configFileOption); - const QCommandLineOption logFileOption("logfile", "File to log to.", "file"); - parser->addOption(logFileOption); - const QCommandLineOption logLevelOption("loglevel", "Log level (default: 2).", "x"); - parser->addOption(logLevelOption); - const QCommandLineOption verboseOption("verbose", "Verbose output. Same as --loglevel=3."); - parser->addOption(verboseOption); - const QCommandLineOption ipcPrefixOption("ipc-prefix", "Override ipc_prefix config option.", "prefix"); - parser->addOption(ipcPrefixOption); - const QCommandLineOption portOffsetOption("port-offset", "Override port_offset config option.", "offset"); - parser->addOption(portOffsetOption); - const QCommandLineOption helpOption = parser->addHelpOption(); - const QCommandLineOption versionOption = parser->addVersionOption(); - - if(!parser->parse(QCoreApplication::arguments())) - { - *errorMessage = parser->errorText(); - return CommandLineError; - } - - if(parser->isSet(versionOption)) - return CommandLineVersionRequested; - - if(parser->isSet(helpOption)) - return CommandLineHelpRequested; - - if(parser->isSet(configFileOption)) - args->configFile = parser->value(configFileOption); - - if(parser->isSet(logFileOption)) - args->logFile = parser->value(logFileOption); - - if(parser->isSet(logLevelOption)) - { - bool ok; - int x = parser->value(logLevelOption).toInt(&ok); - if(!ok || x < 0) - { - *errorMessage = "error: loglevel must be greater than or equal to 0"; - return CommandLineError; - } - - args->logLevel = x; - } - - if(parser->isSet(verboseOption)) - args->logLevel = 3; - - if(parser->isSet(ipcPrefixOption)) - args->ipcPrefix = parser->value(ipcPrefixOption); - - if(parser->isSet(portOffsetOption)) - { - bool ok; - int x = parser->value(portOffsetOption).toInt(&ok); - if(!ok || x < 0) - { - *errorMessage = "error: port-offset must be greater than or equal to 0"; - return CommandLineError; - } - - args->portOffset = x; - } - - return CommandLineOk; -} - class HandlerApp::Private { public: @@ -185,26 +100,19 @@ class HandlerApp::Private QCoreApplication::setApplicationName("pushpin-handler"); QCoreApplication::setApplicationVersion(Config::get().version); - QCommandLineParser parser; - parser.setApplicationDescription("Pushpin handler component."); - - ArgsData args; - QString errorMessage; - switch(parseCommandLine(&parser, &args, &errorMessage)) + QStringList extArgs = QCoreApplication::arguments(); + if(extArgs.isEmpty()) { - case CommandLineOk: - break; - case CommandLineError: - fprintf(stderr, "%s\n\n%s", qPrintable(errorMessage), qPrintable(parser.helpText())); - return 1; - case CommandLineVersionRequested: - printf("%s %s\n", qPrintable(QCoreApplication::applicationName()), - qPrintable(QCoreApplication::applicationVersion())); - return 0; - case CommandLineHelpRequested: - parser.showHelp(); - Q_UNREACHABLE(); + log_error("Error processing arguments. Use --help for usage."); + return 1; } + ArgsData args { + .configFile = extArgs[0], + .logFile = (!extArgs[1].isEmpty()) ? extArgs[1] : QString(), + .logLevel = extArgs[2].toInt(), + .ipcPrefix = extArgs[3], + .portOffset = (!extArgs[4].isEmpty()) ? extArgs[4].toInt() : -1, + }; if(args.logLevel != -1) log_setOutputLevel(args.logLevel); @@ -222,21 +130,17 @@ class HandlerApp::Private log_debug("starting..."); - QString configFile = args.configFile; - if(configFile.isEmpty()) - configFile = QDir(Config::get().configDir).filePath("pushpin.conf"); - - // QSettings doesn't inform us if the config file doesn't exist, so do that ourselves + // QSettings doesn't inform us if the config file can't be opened, so do that ourselves { - QFile file(configFile); + QFile file(args.configFile); if(!file.open(QIODevice::ReadOnly)) { - log_error("failed to open %s, and --config not passed", qPrintable(configFile)); + log_error("failed to open %s", qPrintable(args.configFile)); return 1; } } - Settings settings(configFile); + Settings settings(args.configFile); if(!args.ipcPrefix.isEmpty()) settings.setIpcPrefix(args.ipcPrefix); @@ -496,225 +400,4 @@ HandlerApp::~HandlerApp() = default; int HandlerApp::run() { return Private::run(); -} - -// Export a function that accepts the Rust-compatible struct -extern "C" { - int handler_main_with_args(const ArgsData* args) - { - // Setup QCoreApplication without command line args - int argc = 1; - char* argv[] = { (char*)"pushpin-handler" }; - QCoreApplication app(argc, argv); - - QCoreApplication::setApplicationName("pushpin-handler"); - QCoreApplication::setApplicationVersion(Config::get().version); - - // Create a local ArgsData from the passed struct - ArgsData localArgs; - - if(args->config_file && *args->config_file) - localArgs.configFile = QString::fromUtf8(args->config_file); - - if(args->log_file && *args->log_file) - localArgs.logFile = QString::fromUtf8(args->log_file); - - localArgs.logLevel = args->log_level; - - if(args->ipc_prefix && *args->ipc_prefix) - localArgs.ipcPrefix = QString::fromUtf8(args->ipc_prefix); - - localArgs.portOffset = args->port_offset; - - // Handle verbose flag - if(args->verbose && localArgs.logLevel < 3) - localArgs.logLevel = 3; - - // Configure logging - if(localArgs.logLevel != -1) - log_setOutputLevel(localArgs.logLevel); - else - log_setOutputLevel(LOG_LEVEL_INFO); - - if(!localArgs.logFile.isEmpty()) - { - if(!log_setFile(localArgs.logFile)) - { - log_error("failed to open log file: %s", qPrintable(localArgs.logFile)); - return 1; - } - } - - log_debug("starting..."); - - QString configFile = localArgs.configFile; - if(configFile.isEmpty()) - configFile = QDir(Config::get().configDir).filePath("pushpin.conf"); - - // QSettings doesn't inform us if the config file doesn't exist, so do that ourselves - { - QFile file(configFile); - if(!file.open(QIODevice::ReadOnly)) - { - log_error("failed to open %s, and --config not passed", qPrintable(configFile)); - return 1; - } - } - - Settings settings(configFile); - - if(!localArgs.ipcPrefix.isEmpty()) - settings.setIpcPrefix(localArgs.ipcPrefix); - - if(localArgs.portOffset != -1) - settings.setPortOffset(localArgs.portOffset); - - QStringList services = settings.value("runner/services").toStringList(); - - QStringList connmgr_in_stream_specs = settings.value("proxy/connmgr_in_stream_specs").toStringList(); - trimlist(&connmgr_in_stream_specs); - QStringList connmgr_out_specs = settings.value("proxy/connmgr_out_specs").toStringList(); - trimlist(&connmgr_out_specs); - QStringList condure_in_stream_specs = settings.value("proxy/condure_in_stream_specs").toStringList(); - trimlist(&condure_in_stream_specs); - connmgr_in_stream_specs += condure_in_stream_specs; - QStringList condure_out_specs = settings.value("proxy/condure_out_specs").toStringList(); - trimlist(&condure_out_specs); - connmgr_out_specs += condure_out_specs; - int proxyWorkerCount = settings.value("proxy/workers", 1).toInt(); - QStringList m2a_in_stream_specs = settings.value("handler/m2a_in_stream_specs").toStringList(); - trimlist(&m2a_in_stream_specs); - QStringList m2a_out_specs = settings.value("handler/m2a_out_specs").toStringList(); - trimlist(&m2a_out_specs); - QStringList intreq_out_specs = settings.value("handler/proxy_intreq_out_specs").toStringList(); - trimlist(&intreq_out_specs); - QStringList intreq_out_stream_specs = settings.value("handler/proxy_intreq_out_stream_specs").toStringList(); - trimlist(&intreq_out_stream_specs); - QStringList intreq_in_specs = settings.value("handler/proxy_intreq_in_specs").toStringList(); - trimlist(&intreq_in_specs); - QStringList proxy_inspect_specs = settings.value("handler/proxy_inspect_specs").toStringList(); - trimlist(&proxy_inspect_specs); - QString proxy_inspect_spec = settings.value("handler/proxy_inspect_spec").toString(); - if(!proxy_inspect_spec.isEmpty()) - proxy_inspect_specs += proxy_inspect_spec; - QStringList proxy_accept_specs = settings.value("handler/proxy_accept_specs").toStringList(); - trimlist(&proxy_accept_specs); - QString proxy_accept_spec = settings.value("handler/proxy_accept_spec").toString(); - if(!proxy_accept_spec.isEmpty()) - proxy_accept_specs += proxy_accept_spec; - QStringList proxy_retry_out_specs = settings.value("handler/proxy_retry_out_specs").toStringList(); - trimlist(&proxy_retry_out_specs); - QString proxy_retry_out_spec = settings.value("handler/proxy_retry_out_spec").toString(); - if(!proxy_retry_out_spec.isEmpty()) - proxy_retry_out_specs += proxy_retry_out_spec; - QStringList ws_control_init_specs = settings.value("handler/proxy_ws_control_init_specs").toStringList(); - trimlist(&ws_control_init_specs); - QStringList ws_control_stream_specs = settings.value("handler/proxy_ws_control_stream_specs").toStringList(); - trimlist(&ws_control_stream_specs); - QString stats_spec = settings.value("handler/stats_spec").toString(); - QString command_spec = settings.value("handler/command_spec").toString(); - QString state_spec = settings.value("handler/state_spec").toString(); - QStringList proxy_stats_specs = settings.value("handler/proxy_stats_specs").toStringList(); - trimlist(&proxy_stats_specs); - QString proxy_stats_spec = settings.value("handler/proxy_stats_spec").toString(); - if(!proxy_stats_spec.isEmpty()) - proxy_stats_specs += proxy_stats_spec; - QString proxy_command_spec = settings.value("handler/proxy_command_spec").toString(); - QString push_in_spec = settings.value("handler/push_in_spec").toString(); - QStringList push_in_sub_specs = settings.value("handler/push_in_sub_specs").toStringList(); - trimlist(&push_in_sub_specs); - QString push_in_sub_spec = settings.value("handler/push_in_sub_spec").toString(); - if(!push_in_sub_spec.isEmpty()) - push_in_sub_specs += push_in_sub_spec; - bool push_in_sub_connect = settings.value("handler/push_in_sub_connect").toBool(); - QString push_in_http_addr = settings.value("handler/push_in_http_addr").toString(); - int push_in_http_port = settings.adjustedPort("handler/push_in_http_port"); - int push_in_http_max_headers_size = settings.value("handler/push_in_http_max_headers_size", DEFAULT_HTTP_MAX_HEADERS_SIZE).toInt(); - int push_in_http_max_body_size = settings.value("handler/push_in_http_max_body_size", DEFAULT_HTTP_MAX_BODY_SIZE).toInt(); - bool ok; - int ipcFileMode = settings.value("handler/ipc_file_mode", -1).toString().toInt(&ok, 8); - bool shareAll = settings.value("handler/share_all").toBool(); - int messageRate = settings.value("handler/message_rate", -1).toInt(); - int messageHwm = settings.value("handler/message_hwm", -1).toInt(); - int messageBlockSize = settings.value("handler/message_block_size", -1).toInt(); - int messageWait = settings.value("handler/message_wait", 5000).toInt(); - int idCacheTtl = settings.value("handler/id_cache_ttl", 0).toInt(); - bool updateOnFirstSubscription = settings.value("handler/update_on_first_subscription", true).toBool(); - int clientMaxconn = settings.value("runner/client_maxconn", 50000).toInt(); - int connectionSubscriptionMax = settings.value("handler/connection_subscription_max", 20).toInt(); - int subscriptionLinger = settings.value("handler/subscription_linger", 60).toInt(); - int statsConnectionSend = settings.value("global/stats_connection_send", true).toBool(); - int statsConnectionTtl = settings.value("global/stats_connection_ttl", 120).toInt(); - int statsSubscriptionTtl = settings.value("handler/stats_subscription_ttl", 60).toInt(); - int statsReportInterval = settings.value("handler/stats_report_interval", 10).toInt(); - QString statsFormat = settings.value("handler/stats_format").toString(); - QString prometheusPort = settings.value("handler/prometheus_port").toString(); - QString prometheusPrefix = settings.value("handler/prometheus_prefix").toString(); - - if(m2a_in_stream_specs.isEmpty() || m2a_out_specs.isEmpty()) - { - log_error("must set m2a_in_stream_specs and m2a_out_specs"); - return 1; - } - - if(proxy_inspect_specs.isEmpty() || proxy_accept_specs.isEmpty() || proxy_retry_out_specs.isEmpty()) - { - log_error("must set proxy_inspect_specs, proxy_accept_specs, and proxy_retry_out_specs"); - return 1; - } - - HandlerEngine::Configuration config; - config.appVersion = Config::get().version; - config.instanceId = "handler_" + QByteArray::number(QCoreApplication::applicationPid()); - if(!services.contains("mongrel2") && (!connmgr_in_stream_specs.isEmpty() || !connmgr_out_specs.isEmpty())) - { - config.serverInStreamSpecs = connmgr_in_stream_specs; - config.serverOutSpecs = connmgr_out_specs; - } - else - { - config.serverInStreamSpecs = m2a_in_stream_specs; - config.serverOutSpecs = m2a_out_specs; - } - config.clientOutSpecs = expandSpecs(intreq_out_specs, proxyWorkerCount); - config.clientOutStreamSpecs = expandSpecs(intreq_out_stream_specs, proxyWorkerCount); - config.clientInSpecs = expandSpecs(intreq_in_specs, proxyWorkerCount); - config.inspectSpecs = expandSpecs(proxy_inspect_specs, proxyWorkerCount); - config.acceptSpecs = expandSpecs(proxy_accept_specs, proxyWorkerCount); - config.retryOutSpecs = expandSpecs(proxy_retry_out_specs, proxyWorkerCount); - config.wsControlInitSpecs = expandSpecs(ws_control_init_specs, proxyWorkerCount); - config.wsControlStreamSpecs = expandSpecs(ws_control_stream_specs, proxyWorkerCount); - config.statsSpec = stats_spec; - config.commandSpec = command_spec; - config.stateSpec = state_spec; - config.proxyStatsSpecs = expandSpecs(proxy_stats_specs, proxyWorkerCount); - config.proxyCommandSpec = firstSpec(proxy_command_spec, proxyWorkerCount); - config.pushInSpec = push_in_spec; - config.pushInSubSpecs = push_in_sub_specs; - config.pushInSubConnect = push_in_sub_connect; - config.pushInHttpAddr = QHostAddress(push_in_http_addr); - config.pushInHttpPort = push_in_http_port; - config.pushInHttpMaxHeadersSize = push_in_http_max_headers_size; - config.pushInHttpMaxBodySize = push_in_http_max_body_size; - config.ipcFileMode = ipcFileMode; - config.shareAll = shareAll; - config.messageRate = messageRate; - config.messageHwm = messageHwm; - config.messageBlockSize = messageBlockSize; - config.messageWait = messageWait; - config.idCacheTtl = idCacheTtl; - config.updateOnFirstSubscription = updateOnFirstSubscription; - config.connectionsMax = clientMaxconn; - config.connectionSubscriptionMax = connectionSubscriptionMax; - config.subscriptionLinger = subscriptionLinger; - config.statsConnectionSend = statsConnectionSend; - config.statsConnectionTtl = statsConnectionTtl; - config.statsSubscriptionTtl = statsSubscriptionTtl; - config.statsReportInterval = statsReportInterval; - config.statsFormat = statsFormat; - config.prometheusPort = prometheusPort; - config.prometheusPrefix = prometheusPrefix; - - return runLoop(config, true); - } -} +} \ No newline at end of file diff --git a/src/runner/service.rs b/src/runner/service.rs index bc1eddea4..d4e4586ba 100644 --- a/src/runner/service.rs +++ b/src/runner/service.rs @@ -362,7 +362,7 @@ impl PushpinHandlerService { let service_name = "handler"; args.push(settings.handler_bin.display().to_string()); - args.push(format!("--config={}", settings.config_file.display())); + args.push(format!("--config-file={}", settings.config_file.display())); if settings.port_offset > 0 { args.push(format!("--port-offset={}", settings.port_offset)); @@ -374,7 +374,7 @@ impl PushpinHandlerService { Some(&x) => x, None => settings.log_levels.get("default").unwrap().to_owned(), }; - args.push(format!("--loglevel={}", log_level)); + args.push(format!("--log-level={}", log_level)); Self { service: Service::new(String::from(service_name), log_level), From 7326fb78ee080d3c8cf0649f1114ebc7d2d93c0b Mon Sep 17 00:00:00 2001 From: Serra Date: Mon, 9 Jun 2025 17:34:10 -0700 Subject: [PATCH 06/16] Adds unit test for Rust handler args --- Cargo.lock | 101 +++++++++++++++++++++++++++---------- Cargo.toml | 3 +- src/bin/pushpin-handler.rs | 62 +++++++++++++++++++++++ 3 files changed, 138 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bf98b1624..f79b19921 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -317,7 +317,7 @@ version = "0.1.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f9d839f2a20b0aee515dc581a6172f2321f96cab76c1a38a4c584a194955390e" dependencies = [ - "getrandom", + "getrandom 0.2.11", "once_cell", "tiny-keccak", ] @@ -512,12 +512,12 @@ checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" [[package]] name = "errno" -version = "0.3.9" +version = "0.3.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "534c5cf6194dfab3db3242765c03bbe257cf92f22b38f6bc0c58d59108a820ba" +checksum = "cea14ef9355e3beab063703aa9dab15afd25f0667c341310c1e5274bb1d0da18" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -528,9 +528,9 @@ checksum = "d9435d864e017c3c6afeac1654189b06cdb491cf2ff73dbf0d73b0f292f42ff8" [[package]] name = "fastrand" -version = "2.1.0" +version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9fc0510504f03c51ada170672ac806f1f105a88aa97a5281117e1ddc3368e51a" +checksum = "37909eebbb50d72f9059c3b6d82c0463f2ff062c9e95845c43a6c9c0355411be" [[package]] name = "filetime" @@ -596,10 +596,22 @@ dependencies = [ "cfg-if", "js-sys", "libc", - "wasi", + "wasi 0.11.0+wasi-snapshot-preview1", "wasm-bindgen", ] +[[package]] +name = "getrandom" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "26145e563e54f2cadc477553f1ec5ee650b00862f0a58bcd12cbdc5f0ea2d2f4" +dependencies = [ + "cfg-if", + "libc", + "r-efi", + "wasi 0.14.2+wasi-0.2.4", +] + [[package]] name = "half" version = "1.8.2" @@ -939,7 +951,7 @@ checksum = "c0ff37bd590ca25063e35af745c343cb7a0271906fb7b37e4813e8f79f00268d" dependencies = [ "bitflags 2.9.0", "libc", - "redox_syscall 0.5.10", + "redox_syscall", ] [[package]] @@ -954,6 +966,12 @@ version = "0.4.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "969488b55f8ac402214f3f5fd243ebb7206cf82de60d3172994707a4bcc2b829" +[[package]] +name = "linux-raw-sys" +version = "0.9.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cd945864f07fe9f5371a27ad7b52a172b4b499999f1d97574c9fa68373937e12" + [[package]] name = "litemap" version = "0.7.4" @@ -1016,7 +1034,7 @@ dependencies = [ "hermit-abi", "libc", "log", - "wasi", + "wasi 0.11.0+wasi-snapshot-preview1", "windows-sys 0.52.0", ] @@ -1105,9 +1123,9 @@ dependencies = [ [[package]] name = "once_cell" -version = "1.18.0" +version = "1.21.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dd8b5dd2ae5ed71462c540258bedcb51965123ad7e7ccf4b9a8cafaa4a63576d" +checksum = "42f5e15c9953c5e4ccceeb2e7382a716482c34515315f7b03532b8b4e8393d2d" [[package]] name = "oorandom" @@ -1321,6 +1339,7 @@ dependencies = [ "signal-hook", "slab", "socket2", + "tempfile", "test-log", "thiserror", "time", @@ -1337,6 +1356,12 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "r-efi" +version = "5.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "74765f6d916ee2faa39bc8e68e4f3ed8949b48cccdac59983d287a7cb71ce9c5" + [[package]] name = "rayon" version = "1.8.0" @@ -1357,15 +1382,6 @@ dependencies = [ "crossbeam-utils", ] -[[package]] -name = "redox_syscall" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4722d768eff46b75989dd134e5c353f0d6296e5aaa3132e776cbdb56be7731aa" -dependencies = [ - "bitflags 1.3.2", -] - [[package]] name = "redox_syscall" version = "0.5.10" @@ -1412,7 +1428,7 @@ checksum = "ed9b823fa29b721a59671b41d6b06e66b29e0628e207e8b1c3ceeda701ec928d" dependencies = [ "cc", "cfg-if", - "getrandom", + "getrandom 0.2.11", "libc", "untrusted", "windows-sys 0.52.0", @@ -1467,6 +1483,19 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "rustix" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c71e83d6afe7ff64890ec6b71d6a69bb8a610ab78ce364b3352876bb4c801266" +dependencies = [ + "bitflags 2.9.0", + "errno", + "libc", + "linux-raw-sys 0.9.4", + "windows-sys 0.59.0", +] + [[package]] name = "rustls" version = "0.21.11" @@ -1733,15 +1762,15 @@ dependencies = [ [[package]] name = "tempfile" -version = "3.9.0" +version = "3.20.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "01ce4141aa927a6d1bd34a041795abd0db1cccba5d5f24b009f694bdf3a1f3fa" +checksum = "e8a64e3985349f2441a1a9ef0b853f869006c3855f2cda6862a94d26ebb9d6a1" dependencies = [ - "cfg-if", "fastrand", - "redox_syscall 0.4.1", - "rustix 0.38.28", - "windows-sys 0.52.0", + "getrandom 0.3.3", + "once_cell", + "rustix 1.0.7", + "windows-sys 0.59.0", ] [[package]] @@ -1974,6 +2003,15 @@ version = "0.11.0+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" +[[package]] +name = "wasi" +version = "0.14.2+wasi-0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9683f9a5a998d873c0d21fcbe3c083009670149a8fab228644b8bd36b2c48cb3" +dependencies = [ + "wit-bindgen-rt", +] + [[package]] name = "wasm-bindgen" version = "0.2.92" @@ -2226,6 +2264,15 @@ dependencies = [ "memchr", ] +[[package]] +name = "wit-bindgen-rt" +version = "0.39.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6f42320e61fe2cfd34354ecb597f86f413484a798ba44a8ca1165c58d42da6c1" +dependencies = [ + "bitflags 2.9.0", +] + [[package]] name = "write16" version = "1.0.0" diff --git a/Cargo.toml b/Cargo.toml index d5799052d..462dae1d7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,6 +50,7 @@ zmq = "0.9" [dev-dependencies] criterion = "0.5" env_logger = { version = "0.9", default-features = false } +tempfile = "3.20.0" test-log = "0.2" [build-dependencies] @@ -82,7 +83,7 @@ bench = false [[bin]] name = "pushpin-handler" -test = false +test = true bench = false [[bin]] diff --git a/src/bin/pushpin-handler.rs b/src/bin/pushpin-handler.rs index a05fd1ff7..bcffc083a 100644 --- a/src/bin/pushpin-handler.rs +++ b/src/bin/pushpin-handler.rs @@ -124,3 +124,65 @@ fn main() -> ExitCode { ExitCode::from(call_c_main(handler_main, cli_args.into_osstring_vec())) } } + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::NamedTempFile; + + #[test] + fn test_cli_args() { + // Create mock config file + let file = NamedTempFile::new().unwrap(); + let config_test_file = file.path().to_str().unwrap().to_string(); + + // Test the verification of command line arguments + let args = CliArgs { + config_file: Some(config_test_file.clone()), + log_file: Some("pushpin.log".to_string()), + log_level: 3, + ipc_prefix: Some("ipc".to_string()), + port_offset: Some(8080), + }; + let verified_args = args.verify(); + assert_eq!(verified_args.config_file, Some(config_test_file.clone())); + assert_eq!(verified_args.log_file, Some("pushpin.log".to_string())); + assert_eq!(verified_args.log_level, 3); + assert_eq!(verified_args.ipc_prefix, Some("ipc".to_string())); + assert_eq!(verified_args.port_offset, Some(8080)); + + // Test the conversion to OsString vector + let osstring_vec = verified_args.into_osstring_vec(); + assert_eq!(osstring_vec.len(), 5); + assert_eq!(osstring_vec[0], OsString::from(config_test_file)); + assert_eq!(osstring_vec[1], OsString::from("pushpin.log")); + assert_eq!(osstring_vec[2], OsString::from("3")); + assert_eq!(osstring_vec[3], OsString::from("ipc")); + assert_eq!(osstring_vec[4], OsString::from("8080")); + + // Test empty command line arguments + let empty_args = CliArgs { + config_file: None, + log_file: None, + log_level: 2, + ipc_prefix: None, + port_offset: None, + }; + let verified_empty_args = empty_args.verify(); + let default_config_file = get_config_file(&env::current_dir().unwrap(), None).unwrap().to_string_lossy().to_string(); + assert_eq!(verified_empty_args.config_file, Some(default_config_file.clone())); + assert_eq!(verified_empty_args.log_file, None); + assert_eq!(verified_empty_args.log_level, 2); + assert_eq!(verified_empty_args.ipc_prefix, None); + assert_eq!(verified_empty_args.port_offset, None); + + // Test the conversion to OsString vector + let empty_osstring_vec = verified_empty_args.into_osstring_vec(); + assert_eq!(empty_osstring_vec.len(), 5); + assert_eq!(empty_osstring_vec[0], OsString::from(default_config_file)); + assert_eq!(empty_osstring_vec[1], OsString::from("")); + assert_eq!(empty_osstring_vec[2], OsString::from("2")); + assert_eq!(empty_osstring_vec[3], OsString::from("")); + assert_eq!(empty_osstring_vec[4], OsString::from("")); + } +} \ No newline at end of file From 6df9c3e8395c5d866dd25e3584de3b382c31aef3 Mon Sep 17 00:00:00 2001 From: Serra Date: Tue, 10 Jun 2025 17:18:19 -0700 Subject: [PATCH 07/16] Adds loading settings from cli and unit testing --- src/core/settings.cpp | 7 ++++ src/core/settings.h | 1 + src/handler/argstest.cpp | 79 ++++++++++++++++++++++++++++++++++++++ src/handler/handlerapp.cpp | 62 +++++++++++++++++++++++++++++- src/handler/handlerapp.h | 2 + src/handler/mod.rs | 10 +++++ src/handler/tests.pri | 3 +- src/lib.rs | 1 + 8 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 src/handler/argstest.cpp diff --git a/src/core/settings.cpp b/src/core/settings.cpp index f45fee29f..2ed5125ee 100644 --- a/src/core/settings.cpp +++ b/src/core/settings.cpp @@ -189,3 +189,10 @@ void Settings::setPortOffset(int x) { portOffset_ = x; } + +bool Settings::operator==(const Settings& other) const { + return ipcPrefix_ == other.ipcPrefix_ && + portOffset_ == other.portOffset_ && + libdir_ == other.libdir_ && + rundir_ == other.rundir_; +} \ No newline at end of file diff --git a/src/core/settings.h b/src/core/settings.h index 0caef610d..820e44eaf 100644 --- a/src/core/settings.h +++ b/src/core/settings.h @@ -41,6 +41,7 @@ class Settings void setIpcPrefix(const QString &s); void setPortOffset(int x); + bool operator==(const Settings& other) const; private: QSettings *main_; diff --git a/src/handler/argstest.cpp b/src/handler/argstest.cpp new file mode 100644 index 000000000..8de5b7512 --- /dev/null +++ b/src/handler/argstest.cpp @@ -0,0 +1,79 @@ +/* + * Copyright (C) 2025 Fastly, Inc. + * + * This file is part of Pushpin. + * + * $FANOUT_BEGIN_LICENSE:APACHE2$ + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * $FANOUT_END_LICENSE$ + */ + + +#include "config.h" +#include "handlerapp.h" +#include "settings.h" +#include "test.h" + +void testArguments() +{ + // Get file for example config + std::string configFile = "examples/config/pushpin.conf"; + + // Set up the command line arguments + int argc = 5; + char * argv[] = { + (char *) configFile.c_str(), // configFile + (char *) "log.txt", // logFile + (char *) "2", // logLevel + (char *) "ipc:prefix", // ipcPrefix + (char *) "80" // portOffset + }; + + // Set up the QCoreApplication + QCoreApplication qapp(argc, argv); + HandlerApp app; + QStringList args = qapp.arguments(); + + // Test the arguments + TEST_ASSERT_EQ(args[0], QString("examples/config/pushpin.conf")); + TEST_ASSERT_EQ(args[1], QString("log.txt")); + TEST_ASSERT_EQ(args[2], QString("2")); + TEST_ASSERT_EQ(args[3], QString("ipc:prefix")); + TEST_ASSERT_EQ(args[4], QString("80")); + + // Set up the correct mock settings + Settings mock_settings(args[0]); + mock_settings.setIpcPrefix(args[3]); + mock_settings.setPortOffset(args[4].toInt()); + + // Load settings from command line arguments + Settings settings = app.loadSettingsFromCliArgs(); + + // This should match + TEST_ASSERT_EQ(mock_settings, settings); + + // Change the port offset to a different value + mock_settings.setPortOffset(60); + + // This shouldn't match + TEST_ASSERT(!(mock_settings == settings)); +} + +extern "C" int args_test(ffi::TestException *out_ex) +{ + TEST_CATCH(testArguments()); + + return 0; +} \ No newline at end of file diff --git a/src/handler/handlerapp.cpp b/src/handler/handlerapp.cpp index 70540aaad..6e1c19251 100644 --- a/src/handler/handlerapp.cpp +++ b/src/handler/handlerapp.cpp @@ -111,7 +111,7 @@ class HandlerApp::Private .logFile = (!extArgs[1].isEmpty()) ? extArgs[1] : QString(), .logLevel = extArgs[2].toInt(), .ipcPrefix = extArgs[3], - .portOffset = (!extArgs[4].isEmpty()) ? extArgs[4].toInt() : -1, + .portOffset = (!extArgs[4].isEmpty()) ? extArgs[4].toInt() : -1 }; if(args.logLevel != -1) @@ -400,4 +400,64 @@ HandlerApp::~HandlerApp() = default; int HandlerApp::run() { return Private::run(); +} + +Settings HandlerApp::loadSettingsFromCliArgs() +{ + QCoreApplication::setApplicationName("pushpin-handler"); + QCoreApplication::setApplicationVersion(Config::get().version); + + // Load the command line arguments + QStringList extArgs = QCoreApplication::arguments(); + if(extArgs.isEmpty()) + { + log_error("Error processing arguments. Use --help for usage."); + throw std::exception(); + } + ArgsData args { + .configFile = extArgs[0], + .logFile = (!extArgs[1].isEmpty()) ? extArgs[1] : QString(), + .logLevel = extArgs[2].toInt(), + .ipcPrefix = extArgs[3], + .portOffset = (!extArgs[4].isEmpty()) ? extArgs[4].toInt() : -1, + }; + + // Set the log level + if(args.logLevel != -1) + log_setOutputLevel(args.logLevel); + else + log_setOutputLevel(LOG_LEVEL_INFO); + + // Set the log file if specified + if(!args.logFile.isEmpty()) + { + if(!log_setFile(args.logFile)) + { + log_error("failed to open log file: %s", qPrintable(args.logFile)); + throw std::exception(); + } + } + + log_debug("starting..."); + + // QSettings doesn't inform us if the config file can't be opened, so do that ourselves + { + QFile file(args.configFile); + if(!file.open(QIODevice::ReadOnly)) + { + log_error("failed to open %s", qPrintable(args.configFile)); + throw std::exception(); + } + } + + // Create and set up the Settings object + Settings settings(args.configFile); + + if(!args.ipcPrefix.isEmpty()) + settings.setIpcPrefix(args.ipcPrefix); + + if(args.portOffset != -1) + settings.setPortOffset(args.portOffset); + + return settings; } \ No newline at end of file diff --git a/src/handler/handlerapp.h b/src/handler/handlerapp.h index f0b5d6205..bd5fceb36 100644 --- a/src/handler/handlerapp.h +++ b/src/handler/handlerapp.h @@ -23,6 +23,7 @@ #ifndef HANDLERAPP_H #define HANDLERAPP_H +#include "settings.h" class HandlerApp { @@ -31,6 +32,7 @@ class HandlerApp ~HandlerApp(); int run(); + static Settings loadSettingsFromCliArgs(); private: class Private; diff --git a/src/handler/mod.rs b/src/handler/mod.rs index 0553068c6..c4b7710aa 100644 --- a/src/handler/mod.rs +++ b/src/handler/mod.rs @@ -54,6 +54,11 @@ mod tests { unsafe { ffi::handlerengine_test(out_ex) == 0 } } + fn args_test(out_ex: &mut TestException) -> bool { + // SAFETY: safe to call + unsafe { ffi::args_test(out_ex) == 0 } + } + #[test] fn filter() { run_serial(filter_test); @@ -88,4 +93,9 @@ mod tests { fn handlerengine() { run_serial(handlerengine_test); } + + #[test] + fn args() { + run_serial(args_test); + } } diff --git a/src/handler/tests.pri b/src/handler/tests.pri index 52a6ac7d8..4e36bf7bd 100644 --- a/src/handler/tests.pri +++ b/src/handler/tests.pri @@ -5,4 +5,5 @@ SOURCES += \ $$PWD/idformattest.cpp \ $$PWD/publishformattest.cpp \ $$PWD/publishitemtest.cpp \ - $$PWD/handlerenginetest.cpp + $$PWD/handlerenginetest.cpp \ + $$PWD/argstest.cpp \ No newline at end of file diff --git a/src/lib.rs b/src/lib.rs index 07b4515cf..3ef1deae1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -148,6 +148,7 @@ pub mod ffi { pub fn publishformat_test(out_ex: *mut TestException) -> libc::c_int; pub fn publishitem_test(out_ex: *mut TestException) -> libc::c_int; pub fn handlerengine_test(out_ex: *mut TestException) -> libc::c_int; + pub fn args_test(out_ex: *mut TestException) -> libc::c_int; pub fn template_test(out_ex: *mut TestException) -> libc::c_int; } } From 3f4f54b7a021440d52437a93d75bfdf84d1e33fb Mon Sep 17 00:00:00 2001 From: Serra Date: Wed, 11 Jun 2025 15:07:04 -0700 Subject: [PATCH 08/16] changed name of test fxn --- log.txt | 0 src/handler/{argstest.cpp => cliargstest.cpp} | 6 +++--- src/handler/mod.rs | 8 ++++---- src/handler/tests.pri | 2 +- src/lib.rs | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) create mode 100644 log.txt rename src/handler/{argstest.cpp => cliargstest.cpp} (95%) diff --git a/log.txt b/log.txt new file mode 100644 index 000000000..e69de29bb diff --git a/src/handler/argstest.cpp b/src/handler/cliargstest.cpp similarity index 95% rename from src/handler/argstest.cpp rename to src/handler/cliargstest.cpp index 8de5b7512..77b12cbb8 100644 --- a/src/handler/argstest.cpp +++ b/src/handler/cliargstest.cpp @@ -26,7 +26,7 @@ #include "settings.h" #include "test.h" -void testArguments() +void cliArgsTest() { // Get file for example config std::string configFile = "examples/config/pushpin.conf"; @@ -71,9 +71,9 @@ void testArguments() TEST_ASSERT(!(mock_settings == settings)); } -extern "C" int args_test(ffi::TestException *out_ex) +extern "C" int cliargs_test(ffi::TestException *out_ex) { - TEST_CATCH(testArguments()); + TEST_CATCH(cliArgsTest()); return 0; } \ No newline at end of file diff --git a/src/handler/mod.rs b/src/handler/mod.rs index c4b7710aa..d709eaeff 100644 --- a/src/handler/mod.rs +++ b/src/handler/mod.rs @@ -54,9 +54,9 @@ mod tests { unsafe { ffi::handlerengine_test(out_ex) == 0 } } - fn args_test(out_ex: &mut TestException) -> bool { + fn cliargs_test(out_ex: &mut TestException) -> bool { // SAFETY: safe to call - unsafe { ffi::args_test(out_ex) == 0 } + unsafe { ffi::cliargs_test(out_ex) == 0 } } #[test] @@ -95,7 +95,7 @@ mod tests { } #[test] - fn args() { - run_serial(args_test); + fn cliargs() { + run_serial(cliargs_test); } } diff --git a/src/handler/tests.pri b/src/handler/tests.pri index 4e36bf7bd..bb527f2fd 100644 --- a/src/handler/tests.pri +++ b/src/handler/tests.pri @@ -6,4 +6,4 @@ SOURCES += \ $$PWD/publishformattest.cpp \ $$PWD/publishitemtest.cpp \ $$PWD/handlerenginetest.cpp \ - $$PWD/argstest.cpp \ No newline at end of file + $$PWD/cliargstest.cpp \ No newline at end of file diff --git a/src/lib.rs b/src/lib.rs index 3ef1deae1..0b0ee449e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -148,7 +148,7 @@ pub mod ffi { pub fn publishformat_test(out_ex: *mut TestException) -> libc::c_int; pub fn publishitem_test(out_ex: *mut TestException) -> libc::c_int; pub fn handlerengine_test(out_ex: *mut TestException) -> libc::c_int; - pub fn args_test(out_ex: *mut TestException) -> libc::c_int; + pub fn cliargs_test(out_ex: *mut TestException) -> libc::c_int; pub fn template_test(out_ex: *mut TestException) -> libc::c_int; } } From 1cf7cee95f76a8f27daf28e25582a16bfacebc7b Mon Sep 17 00:00:00 2001 From: Serra Date: Mon, 16 Jun 2025 12:08:52 -0700 Subject: [PATCH 09/16] Adds new arg vars to test mod --- src/bin/pushpin-handler.rs | 165 +------------- src/bin/pushpin-proxy.rs | 4 + src/core/ccliargs.rs | 209 ++++++++++++++++++ src/core/mod.rs | 2 + src/core/tests.pri | 1 + src/handler/argstest.cpp | 0 .../{cliargstest.cpp => handlerargstest.cpp} | 6 +- src/handler/mod.rs | 9 +- src/handler/tests.pri | 2 +- src/lib.rs | 2 +- src/runner/mod.rs | 1 + 11 files changed, 232 insertions(+), 169 deletions(-) create mode 100644 src/core/ccliargs.rs create mode 100644 src/handler/argstest.cpp rename src/handler/{cliargstest.cpp => handlerargstest.cpp} (94%) diff --git a/src/bin/pushpin-handler.rs b/src/bin/pushpin-handler.rs index bcffc083a..03af9ffdd 100644 --- a/src/bin/pushpin-handler.rs +++ b/src/bin/pushpin-handler.rs @@ -14,175 +14,20 @@ * limitations under the License. */ -use clap::{Parser, arg}; -use pushpin::core::{call_c_main, version}; -use pushpin::core::config::get_config_file; +use pushpin::core::call_c_main; use pushpin::import_cpp; -use std::env; -use std::ffi::{OsString}; -use std::path::PathBuf; use std::process::ExitCode; +use pushpin::core::ccliargs::CCliArgs; +use clap::Parser; import_cpp! { fn handler_main(argc: libc::c_int, argv: *const *const libc::c_char) -> libc::c_int; } - -// Struct to hold the command line arguments -#[derive(Parser, Debug)] -#[command( - name= "Pushpin Handler", - version = version(), - about = "Pushpin handler component." -)] -pub struct CliArgs { - /// Set path to the configuration file - #[arg(short, long, value_name = "file")] - pub config_file: Option, - - /// Set path to the log file - #[arg(short = 'l', long, value_name = "file")] - pub log_file: Option, - - /// Set log level (0=error, 1=warn, 2=info, 3=debug, 4=trace) - #[arg(short = 'L', long, value_name = "x", default_value = "2", value_parser = clap::value_parser!(u32).range(1..=4))] - pub log_level: u32, - - /// Override ipc_prefix config option, which is used to add a prefix to all ZeroMQ IPC filenames - #[arg(long, value_name = "prefix")] - pub ipc_prefix: Option, - - /// Override port_offset config option, which is used to increment all ZeroMQ TCP ports and the HTTP control server port - #[arg(long, value_name = "offset", value_parser = clap::value_parser!(u32))] - pub port_offset: Option, -} -impl CliArgs { - /// Verifies the command line arguments. - pub fn verify(mut self) -> Self { - // Get current working directory - let work_dir = env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); - - // Convert config_file Option to Option - let config_path = self.config_file.as_ref().map(|s| PathBuf::from(s)); - - // Use get_config_file to find the config file - self.config_file = match get_config_file(&work_dir, config_path) { - Ok(path) => Some(path.to_string_lossy().to_string()), - Err(e) => { - eprintln!("error: failed to find configuration file: {}", e); - std::process::exit(1); - } - }; - - self - } - - pub fn into_osstring_vec(self) -> Vec { - self.into_iter() - .map(|(_, value)| OsString::from(value)) - .collect() - } -} - -impl IntoIterator for CliArgs { - type Item = (String, String); - type IntoIter = std::vec::IntoIter; - - fn into_iter(self) -> Self::IntoIter { - let mut args: Vec<(String, String)> = vec![]; - - args.push(( - "config-file".to_string(), - self.config_file.unwrap_or_else(|| "".to_string()) - )); - - args.push(( - "log-file".to_string(), - self.log_file.as_ref().map(|s| s.to_string()).unwrap_or_else(|| "".to_string()), - )); - - args.push(("log-level".to_string(), self.log_level.to_string())); - - args.push(( - "ipc-prefix".to_string(), - self.ipc_prefix.as_ref().map(|s| s.to_string()).unwrap_or_else(|| "".to_string()), - )); - - args.push(( - "port-offset".to_string(), - self.port_offset.as_ref().map(|s| s.to_string()).unwrap_or_else(|| "".to_string()), - )); - - args.into_iter() - } -} - fn main() -> ExitCode { - let cli_args = CliArgs::parse().verify(); + let c_cli_args = CCliArgs::parse().verify(); unsafe { - ExitCode::from(call_c_main(handler_main, cli_args.into_osstring_vec())) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use tempfile::NamedTempFile; - - #[test] - fn test_cli_args() { - // Create mock config file - let file = NamedTempFile::new().unwrap(); - let config_test_file = file.path().to_str().unwrap().to_string(); - - // Test the verification of command line arguments - let args = CliArgs { - config_file: Some(config_test_file.clone()), - log_file: Some("pushpin.log".to_string()), - log_level: 3, - ipc_prefix: Some("ipc".to_string()), - port_offset: Some(8080), - }; - let verified_args = args.verify(); - assert_eq!(verified_args.config_file, Some(config_test_file.clone())); - assert_eq!(verified_args.log_file, Some("pushpin.log".to_string())); - assert_eq!(verified_args.log_level, 3); - assert_eq!(verified_args.ipc_prefix, Some("ipc".to_string())); - assert_eq!(verified_args.port_offset, Some(8080)); - - // Test the conversion to OsString vector - let osstring_vec = verified_args.into_osstring_vec(); - assert_eq!(osstring_vec.len(), 5); - assert_eq!(osstring_vec[0], OsString::from(config_test_file)); - assert_eq!(osstring_vec[1], OsString::from("pushpin.log")); - assert_eq!(osstring_vec[2], OsString::from("3")); - assert_eq!(osstring_vec[3], OsString::from("ipc")); - assert_eq!(osstring_vec[4], OsString::from("8080")); - - // Test empty command line arguments - let empty_args = CliArgs { - config_file: None, - log_file: None, - log_level: 2, - ipc_prefix: None, - port_offset: None, - }; - let verified_empty_args = empty_args.verify(); - let default_config_file = get_config_file(&env::current_dir().unwrap(), None).unwrap().to_string_lossy().to_string(); - assert_eq!(verified_empty_args.config_file, Some(default_config_file.clone())); - assert_eq!(verified_empty_args.log_file, None); - assert_eq!(verified_empty_args.log_level, 2); - assert_eq!(verified_empty_args.ipc_prefix, None); - assert_eq!(verified_empty_args.port_offset, None); - - // Test the conversion to OsString vector - let empty_osstring_vec = verified_empty_args.into_osstring_vec(); - assert_eq!(empty_osstring_vec.len(), 5); - assert_eq!(empty_osstring_vec[0], OsString::from(default_config_file)); - assert_eq!(empty_osstring_vec[1], OsString::from("")); - assert_eq!(empty_osstring_vec[2], OsString::from("2")); - assert_eq!(empty_osstring_vec[3], OsString::from("")); - assert_eq!(empty_osstring_vec[4], OsString::from("")); + ExitCode::from(call_c_main(handler_main, c_cli_args.into_osstring_vec())) } } \ No newline at end of file diff --git a/src/bin/pushpin-proxy.rs b/src/bin/pushpin-proxy.rs index 59ea3da22..3761361f4 100644 --- a/src/bin/pushpin-proxy.rs +++ b/src/bin/pushpin-proxy.rs @@ -18,11 +18,15 @@ use pushpin::core::call_c_main; use pushpin::import_cpp; use std::env; use std::process::ExitCode; +use pushpin::core::ccliargs::CCliArgs; +use clap::Parser; import_cpp! { fn proxy_main(argc: libc::c_int, argv: *const *const libc::c_char) -> libc::c_int; } fn main() -> ExitCode { + let c_cli_args = CCliArgs::parse().verify(); + unsafe { ExitCode::from(call_c_main(proxy_main, env::args_os())) } } diff --git a/src/core/ccliargs.rs b/src/core/ccliargs.rs new file mode 100644 index 000000000..8bcc1152d --- /dev/null +++ b/src/core/ccliargs.rs @@ -0,0 +1,209 @@ +/* + * Copyright (C) 2023 Fastly, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +use std::env; +use std::ffi::{OsString}; +use std::path::PathBuf; +use clap::{Parser, arg}; +use crate::core::version; +use crate::core::config::get_config_file; + +// Struct to hold the command line arguments +#[derive(Parser, Debug)] +#[command( + name= "Pushpin Handler", + version = version(), + about = "Pushpin handler component." +)] +pub struct CCliArgs { + /// Set path to the configuration file + #[arg(short, long, value_name = "file")] + pub config_file: Option, + + /// Set path to the log file + #[arg(short = 'l', long, value_name = "file")] + pub log_file: Option, + + /// Set log level (0=error, 1=warn, 2=info, 3=debug, 4=trace) + #[arg(short = 'L', long, value_name = "x", default_value = "2", value_parser = clap::value_parser!(u32).range(1..=4))] + pub log_level: u32, + + /// Override ipc_prefix config option, which is used to add a prefix to all ZeroMQ IPC filenames + #[arg(long, value_name = "prefix")] + pub ipc_prefix: Option, + + /// Override port_offset config option, which is used to increment all ZeroMQ TCP ports and the HTTP control server port + #[arg(long, value_name = "offset", value_parser = clap::value_parser!(u32))] + pub port_offset: Option, + + /// Add route (overrides routes file) + #[arg(long, value_name = "route")] + pub route: Option>, + + /// Log update checks in Zurl as debug level + #[arg(long, value_name = "quiet-check", default_value_t = false)] + pub quiet_check: bool, +} + +impl CCliArgs { + /// Verifies the command line arguments. + pub fn verify(mut self) -> Self { + // Get current working directory + let work_dir = env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); + + // Convert config_file Option to Option + let config_path = self.config_file.as_ref().map(|s| PathBuf::from(s)); + + // Use get_config_file to find the config file + self.config_file = match get_config_file(&work_dir, config_path) { + Ok(path) => Some(path.to_string_lossy().to_string()), + Err(e) => { + eprintln!("error: failed to find configuration file: {}", e); + std::process::exit(1); + } + }; + + self + } + + pub fn into_osstring_vec(self) -> Vec { + self.into_iter() + .map(|(_, value)| OsString::from(value)) + .collect() + } +} + +impl IntoIterator for CCliArgs { + type Item = (String, String); + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + let mut args: Vec<(String, String)> = vec![]; + + args.push(( + "config-file".to_string(), + self.config_file.unwrap_or_else(|| "".to_string()) + )); + + args.push(( + "log-file".to_string(), + self.log_file.as_ref().map(|s| s.to_string()).unwrap_or_else(|| "".to_string()), + )); + + args.push(("log-level".to_string(), self.log_level.to_string())); + + args.push(( + "ipc-prefix".to_string(), + self.ipc_prefix.as_ref().map(|s| s.to_string()).unwrap_or_else(|| "".to_string()), + )); + + args.push(( + "port-offset".to_string(), + self.port_offset.as_ref().map(|s| s.to_string()).unwrap_or_else(|| "".to_string()), + )); + + args.push(( + "routes".to_string(), + self.route.iter().map(|r| r.join(",")).collect::(), + )); + + args.push(( + "quiet-check".to_string(), + self.quiet_check.to_string(), + )); + + args.into_iter() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::NamedTempFile; + + #[test] + fn test_ccli_args() { + // Create mock config file + let file = NamedTempFile::new().unwrap(); + let config_test_file = file.path().to_str().unwrap().to_string(); + let expected_arg_count = 7; + + // Create valid CCliArgs + let args = CCliArgs { + config_file: Some(config_test_file.clone()), + log_file: Some("pushpin.log".to_string()), + log_level: 3, + ipc_prefix: Some("ipc".to_string()), + port_offset: Some(8080), + route: Some(vec!["route1".to_string(), "route2".to_string()]), + quiet_check: true, + }; + + // Verify verify() + let verified_args = args.verify(); + assert_eq!(verified_args.config_file, Some(config_test_file.clone())); + assert_eq!(verified_args.log_file, Some("pushpin.log".to_string())); + assert_eq!(verified_args.log_level, 3); + assert_eq!(verified_args.ipc_prefix, Some("ipc".to_string())); + assert_eq!(verified_args.port_offset, Some(8080)); + assert_eq!(verified_args.route, Some(vec!["route1".to_string(), "route2".to_string()])); + assert_eq!(verified_args.quiet_check, true); + + // Verify conversion to OsString vector + let osstring_vec = verified_args.into_osstring_vec(); + assert_eq!(osstring_vec.len(), expected_arg_count); + assert_eq!(osstring_vec[0], OsString::from(config_test_file)); + assert_eq!(osstring_vec[1], OsString::from("pushpin.log")); + assert_eq!(osstring_vec[2], OsString::from("3")); + assert_eq!(osstring_vec[3], OsString::from("ipc")); + assert_eq!(osstring_vec[4], OsString::from("8080")); + assert_eq!(osstring_vec[5], OsString::from("route1,route2")); + assert_eq!(osstring_vec[6], OsString::from("true")); + + // Create valid empty CCliArgs + let empty_args = CCliArgs { + config_file: None, + log_file: None, + log_level: 2, + ipc_prefix: None, + port_offset: None, + route: None, + quiet_check: false, + }; + + // Verify verify() + let verified_empty_args = empty_args.verify(); + let default_config_file = get_config_file(&env::current_dir().unwrap(), None).unwrap().to_string_lossy().to_string(); + assert_eq!(verified_empty_args.config_file, Some(default_config_file.clone())); + assert_eq!(verified_empty_args.log_file, None); + assert_eq!(verified_empty_args.log_level, 2); + assert_eq!(verified_empty_args.ipc_prefix, None); + assert_eq!(verified_empty_args.port_offset, None); + assert_eq!(verified_empty_args.route, None); + assert_eq!(verified_empty_args.quiet_check, false); + + // Verify conversion to OsString vector + let empty_osstring_vec = verified_empty_args.into_osstring_vec(); + assert_eq!(empty_osstring_vec.len(), expected_arg_count); + assert_eq!(empty_osstring_vec[0], OsString::from(default_config_file)); + assert_eq!(empty_osstring_vec[1], OsString::from("")); + assert_eq!(empty_osstring_vec[2], OsString::from("2")); + assert_eq!(empty_osstring_vec[3], OsString::from("")); + assert_eq!(empty_osstring_vec[4], OsString::from("")); + assert_eq!(empty_osstring_vec[5], OsString::from("")); + assert_eq!(empty_osstring_vec[6], OsString::from("false")); + } +} \ No newline at end of file diff --git a/src/core/mod.rs b/src/core/mod.rs index 03aed3465..68904b6d1 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -39,6 +39,7 @@ pub mod timer; pub mod tnetstring; pub mod waker; pub mod zmq; +pub mod ccliargs; use std::env; use std::ffi::{CString, OsStr}; @@ -153,6 +154,7 @@ mod tests { unsafe { ffi::eventloop_test(out_ex) == 0 } } + #[test] fn httpheaders() { run_serial(httpheaders_test); diff --git a/src/core/tests.pri b/src/core/tests.pri index 0adf1c63d..1ffb5f38a 100644 --- a/src/core/tests.pri +++ b/src/core/tests.pri @@ -6,3 +6,4 @@ SOURCES += \ $$PWD/tcpstreamtest.cpp \ $$PWD/unixstreamtest.cpp \ $$PWD/eventlooptest.cpp + diff --git a/src/handler/argstest.cpp b/src/handler/argstest.cpp new file mode 100644 index 000000000..e69de29bb diff --git a/src/handler/cliargstest.cpp b/src/handler/handlerargstest.cpp similarity index 94% rename from src/handler/cliargstest.cpp rename to src/handler/handlerargstest.cpp index 77b12cbb8..de29cbfdb 100644 --- a/src/handler/cliargstest.cpp +++ b/src/handler/handlerargstest.cpp @@ -26,7 +26,7 @@ #include "settings.h" #include "test.h" -void cliArgsTest() +void handlerargstest() { // Get file for example config std::string configFile = "examples/config/pushpin.conf"; @@ -71,9 +71,9 @@ void cliArgsTest() TEST_ASSERT(!(mock_settings == settings)); } -extern "C" int cliargs_test(ffi::TestException *out_ex) +extern "C" int handlerargs_test(ffi::TestException *out_ex) { - TEST_CATCH(cliArgsTest()); + TEST_CATCH(handlerargstest()); return 0; } \ No newline at end of file diff --git a/src/handler/mod.rs b/src/handler/mod.rs index d709eaeff..0201fe366 100644 --- a/src/handler/mod.rs +++ b/src/handler/mod.rs @@ -54,9 +54,9 @@ mod tests { unsafe { ffi::handlerengine_test(out_ex) == 0 } } - fn cliargs_test(out_ex: &mut TestException) -> bool { + fn handlerargs_test(out_ex: &mut TestException) -> bool { // SAFETY: safe to call - unsafe { ffi::cliargs_test(out_ex) == 0 } + unsafe { ffi::handlerargs_test(out_ex) == 0 } } #[test] @@ -95,7 +95,8 @@ mod tests { } #[test] - fn cliargs() { - run_serial(cliargs_test); + fn handlerargs() { + run_serial(handlerargs_test); } + } diff --git a/src/handler/tests.pri b/src/handler/tests.pri index bb527f2fd..5f4c0704c 100644 --- a/src/handler/tests.pri +++ b/src/handler/tests.pri @@ -6,4 +6,4 @@ SOURCES += \ $$PWD/publishformattest.cpp \ $$PWD/publishitemtest.cpp \ $$PWD/handlerenginetest.cpp \ - $$PWD/cliargstest.cpp \ No newline at end of file + $$PWD/handlerargstest.cpp \ No newline at end of file diff --git a/src/lib.rs b/src/lib.rs index 0b0ee449e..f2976e73c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -148,7 +148,7 @@ pub mod ffi { pub fn publishformat_test(out_ex: *mut TestException) -> libc::c_int; pub fn publishitem_test(out_ex: *mut TestException) -> libc::c_int; pub fn handlerengine_test(out_ex: *mut TestException) -> libc::c_int; - pub fn cliargs_test(out_ex: *mut TestException) -> libc::c_int; + pub fn handlerargs_test(out_ex: *mut TestException) -> libc::c_int; pub fn template_test(out_ex: *mut TestException) -> libc::c_int; } } diff --git a/src/runner/mod.rs b/src/runner/mod.rs index bd2856b2f..ec4ddeee2 100644 --- a/src/runner/mod.rs +++ b/src/runner/mod.rs @@ -37,6 +37,7 @@ use crate::core::{is_debug_build, version}; version = version(), about = "Reverse proxy for realtime web services." )] + pub struct CliArgs { #[arg(long, value_name = "file", help = "Config file.")] pub config: Option, From a8d5978080cc4fe4bbae689fbbf8f22108c6b210 Mon Sep 17 00:00:00 2001 From: Serra Date: Mon, 16 Jun 2025 13:40:45 -0700 Subject: [PATCH 10/16] Adds ArgsData to proxy --- src/bin/pushpin-proxy.rs | 3 +- src/core/argsdata.cpp | 88 +++++++++++++++++ src/core/argsdata.h | 42 ++++++++ src/core/ccliargs.rs | 2 +- src/core/core.pri | 4 +- src/handler/handlerapp.cpp | 118 +--------------------- src/handler/handlerapp.h | 1 - src/handler/handlerargstest.cpp | 104 ++++++++++++++++---- src/proxy/app.cpp | 153 ++--------------------------- src/runner/pushpinproxyservice.cpp | 8 +- 10 files changed, 236 insertions(+), 287 deletions(-) create mode 100644 src/core/argsdata.cpp create mode 100644 src/core/argsdata.h diff --git a/src/bin/pushpin-proxy.rs b/src/bin/pushpin-proxy.rs index 3761361f4..509f15532 100644 --- a/src/bin/pushpin-proxy.rs +++ b/src/bin/pushpin-proxy.rs @@ -16,7 +16,6 @@ use pushpin::core::call_c_main; use pushpin::import_cpp; -use std::env; use std::process::ExitCode; use pushpin::core::ccliargs::CCliArgs; use clap::Parser; @@ -28,5 +27,5 @@ import_cpp! { fn main() -> ExitCode { let c_cli_args = CCliArgs::parse().verify(); - unsafe { ExitCode::from(call_c_main(proxy_main, env::args_os())) } + unsafe { ExitCode::from(call_c_main(proxy_main, c_cli_args.into_osstring_vec())) } } diff --git a/src/core/argsdata.cpp b/src/core/argsdata.cpp new file mode 100644 index 000000000..9381077b5 --- /dev/null +++ b/src/core/argsdata.cpp @@ -0,0 +1,88 @@ +/* + * Copyright (C) 2015-2022 Fanout, Inc. + * Copyright (C) 2024-2025 Fastly, Inc. + * + * This file is part of Pushpin. + * + * $FANOUT_BEGIN_LICENSE:APACHE2$ + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * $FANOUT_END_LICENSE$ + */ + +#include "argsdata.h" +#include "settings.h" +#include "config.h" +#include "log.h" +#include +#include + + +Settings ArgsData::loadIntoSettings() +{ + Settings settings(configFile); + + if(!ipcPrefix.isEmpty()) + settings.setIpcPrefix(ipcPrefix); + + if(portOffset != -1) + settings.setPortOffset(portOffset); + + return settings; +} + +ArgsData::ArgsData(QStringList &extArgs) +{ + if(extArgs.isEmpty()) + { + log_error("Error processing arguments. Use --help for usage."); + throw std::exception(); + } + + configFile = extArgs[0]; + logFile = !extArgs[1].isEmpty() ? extArgs[1] : QString(); + logLevel = extArgs[2].toInt(); + ipcPrefix = extArgs[3]; + portOffset = !extArgs[4].isEmpty() ? extArgs[4].toInt() : -1; + routeLines = extArgs[5].isEmpty() ? QStringList() : extArgs[5].split(','); + quietCheck = extArgs[6] == "true" ? true : false; + + // Set the log level + if(logLevel != -1) + log_setOutputLevel(logLevel); + else + log_setOutputLevel(LOG_LEVEL_INFO); + + // Set the log file if specified + if(!logFile.isEmpty()) + { + if(!log_setFile(logFile)) + { + log_error("failed to open log file: %s", qPrintable(logFile)); + throw std::exception(); + } + } + + log_debug("starting..."); + + // QSettings doesn't inform us if the config file can't be opened, so do that ourselves + { + QFile file(configFile); + if(!file.open(QIODevice::ReadOnly)) + { + log_error("failed to open %s", qPrintable(configFile)); + throw std::exception(); + } + } +} \ No newline at end of file diff --git a/src/core/argsdata.h b/src/core/argsdata.h new file mode 100644 index 000000000..a5a8cdd34 --- /dev/null +++ b/src/core/argsdata.h @@ -0,0 +1,42 @@ +/* + * Copyright (C) 2015-2022 Fanout, Inc. + * Copyright (C) 2024-2025 Fastly, Inc. + * + * This file is part of Pushpin. + * + * $FANOUT_BEGIN_LICENSE:APACHE2$ + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * $FANOUT_END_LICENSE$ + */ + +#include +#include +#include "settings.h" + +class ArgsData +{ + public: + QString configFile; + QString logFile; + int logLevel; + QString ipcPrefix; + int portOffset; + QStringList routeLines; + bool quietCheck; + + ArgsData(QStringList &extArgs); + + Settings loadIntoSettings(); +}; \ No newline at end of file diff --git a/src/core/ccliargs.rs b/src/core/ccliargs.rs index 8bcc1152d..1f3a19901 100644 --- a/src/core/ccliargs.rs +++ b/src/core/ccliargs.rs @@ -38,7 +38,7 @@ pub struct CCliArgs { pub log_file: Option, /// Set log level (0=error, 1=warn, 2=info, 3=debug, 4=trace) - #[arg(short = 'L', long, value_name = "x", default_value = "2", value_parser = clap::value_parser!(u32).range(1..=4))] + #[arg(short = 'L', long, value_name = "x", default_value_t = 2, value_parser = clap::value_parser!(u32).range(1..=4))] pub log_level: u32, /// Override ipc_prefix config option, which is used to add a prefix to all ZeroMQ IPC filenames diff --git a/src/core/core.pri b/src/core/core.pri index cecc9e5d9..ad7befda8 100644 --- a/src/core/core.pri +++ b/src/core/core.pri @@ -81,10 +81,12 @@ HEADERS += \ $$PWD/simplehttpserver.h \ $$PWD/stats.h \ $$PWD/statsmanager.h \ - $$PWD/settings.h + $$PWD/settings.h \ + $$PWD/argsdata.h SOURCES += \ $$PWD/config.cpp \ + $$PWD/argsdata.cpp \ $$PWD/timerwheel.cpp \ $$PWD/jwt.cpp \ $$PWD/timer.cpp \ diff --git a/src/handler/handlerapp.cpp b/src/handler/handlerapp.cpp index 6e1c19251..89a37827b 100644 --- a/src/handler/handlerapp.cpp +++ b/src/handler/handlerapp.cpp @@ -22,6 +22,7 @@ */ #include "handlerapp.h" +#include "argsdata.h" #include #include @@ -82,15 +83,6 @@ static QString firstSpec(const QString &s, int peerCount) return s; } -class ArgsData -{ -public: - QString configFile; - QString logFile; - int logLevel; - QString ipcPrefix; - int portOffset; -}; class HandlerApp::Private { @@ -101,52 +93,8 @@ class HandlerApp::Private QCoreApplication::setApplicationVersion(Config::get().version); QStringList extArgs = QCoreApplication::arguments(); - if(extArgs.isEmpty()) - { - log_error("Error processing arguments. Use --help for usage."); - return 1; - } - ArgsData args { - .configFile = extArgs[0], - .logFile = (!extArgs[1].isEmpty()) ? extArgs[1] : QString(), - .logLevel = extArgs[2].toInt(), - .ipcPrefix = extArgs[3], - .portOffset = (!extArgs[4].isEmpty()) ? extArgs[4].toInt() : -1 - }; - - if(args.logLevel != -1) - log_setOutputLevel(args.logLevel); - else - log_setOutputLevel(LOG_LEVEL_INFO); - - if(!args.logFile.isEmpty()) - { - if(!log_setFile(args.logFile)) - { - log_error("failed to open log file: %s", qPrintable(args.logFile)); - return 1; - } - } - - log_debug("starting..."); - - // QSettings doesn't inform us if the config file can't be opened, so do that ourselves - { - QFile file(args.configFile); - if(!file.open(QIODevice::ReadOnly)) - { - log_error("failed to open %s", qPrintable(args.configFile)); - return 1; - } - } - - Settings settings(args.configFile); - - if(!args.ipcPrefix.isEmpty()) - settings.setIpcPrefix(args.ipcPrefix); - - if(args.portOffset != -1) - settings.setPortOffset(args.portOffset); + ArgsData args(extArgs); + Settings settings = args.loadIntoSettings(); QStringList services = settings.value("runner/services").toStringList(); @@ -400,64 +348,4 @@ HandlerApp::~HandlerApp() = default; int HandlerApp::run() { return Private::run(); -} - -Settings HandlerApp::loadSettingsFromCliArgs() -{ - QCoreApplication::setApplicationName("pushpin-handler"); - QCoreApplication::setApplicationVersion(Config::get().version); - - // Load the command line arguments - QStringList extArgs = QCoreApplication::arguments(); - if(extArgs.isEmpty()) - { - log_error("Error processing arguments. Use --help for usage."); - throw std::exception(); - } - ArgsData args { - .configFile = extArgs[0], - .logFile = (!extArgs[1].isEmpty()) ? extArgs[1] : QString(), - .logLevel = extArgs[2].toInt(), - .ipcPrefix = extArgs[3], - .portOffset = (!extArgs[4].isEmpty()) ? extArgs[4].toInt() : -1, - }; - - // Set the log level - if(args.logLevel != -1) - log_setOutputLevel(args.logLevel); - else - log_setOutputLevel(LOG_LEVEL_INFO); - - // Set the log file if specified - if(!args.logFile.isEmpty()) - { - if(!log_setFile(args.logFile)) - { - log_error("failed to open log file: %s", qPrintable(args.logFile)); - throw std::exception(); - } - } - - log_debug("starting..."); - - // QSettings doesn't inform us if the config file can't be opened, so do that ourselves - { - QFile file(args.configFile); - if(!file.open(QIODevice::ReadOnly)) - { - log_error("failed to open %s", qPrintable(args.configFile)); - throw std::exception(); - } - } - - // Create and set up the Settings object - Settings settings(args.configFile); - - if(!args.ipcPrefix.isEmpty()) - settings.setIpcPrefix(args.ipcPrefix); - - if(args.portOffset != -1) - settings.setPortOffset(args.portOffset); - - return settings; } \ No newline at end of file diff --git a/src/handler/handlerapp.h b/src/handler/handlerapp.h index bd5fceb36..d68994c80 100644 --- a/src/handler/handlerapp.h +++ b/src/handler/handlerapp.h @@ -32,7 +32,6 @@ class HandlerApp ~HandlerApp(); int run(); - static Settings loadSettingsFromCliArgs(); private: class Private; diff --git a/src/handler/handlerargstest.cpp b/src/handler/handlerargstest.cpp index de29cbfdb..88b9f95b3 100644 --- a/src/handler/handlerargstest.cpp +++ b/src/handler/handlerargstest.cpp @@ -25,50 +25,116 @@ #include "handlerapp.h" #include "settings.h" #include "test.h" +#include "argsdata.h" void handlerargstest() { // Get file for example config std::string configFile = "examples/config/pushpin.conf"; - // Set up the command line arguments - int argc = 5; + // Set up valid command line arguments + int argc = 7; char * argv[] = { (char *) configFile.c_str(), // configFile (char *) "log.txt", // logFile (char *) "2", // logLevel (char *) "ipc:prefix", // ipcPrefix - (char *) "80" // portOffset + (char *) "80", // portOffset + (char *) "route1,route2", // routeLines + (char *) "true", // quietCheck }; - // Set up the QCoreApplication + // Set up QCoreApplication QCoreApplication qapp(argc, argv); HandlerApp app; - QStringList args = qapp.arguments(); + QStringList extArgs = qapp.arguments(); - // Test the arguments - TEST_ASSERT_EQ(args[0], QString("examples/config/pushpin.conf")); - TEST_ASSERT_EQ(args[1], QString("log.txt")); - TEST_ASSERT_EQ(args[2], QString("2")); - TEST_ASSERT_EQ(args[3], QString("ipc:prefix")); - TEST_ASSERT_EQ(args[4], QString("80")); - - // Set up the correct mock settings - Settings mock_settings(args[0]); - mock_settings.setIpcPrefix(args[3]); - mock_settings.setPortOffset(args[4].toInt()); + // Verify the arguments + TEST_ASSERT_EQ(extArgs[0], QString("examples/config/pushpin.conf")); + TEST_ASSERT_EQ(extArgs[1], QString("log.txt")); + TEST_ASSERT_EQ(extArgs[2], QString("2")); + TEST_ASSERT_EQ(extArgs[3], QString("ipc:prefix")); + TEST_ASSERT_EQ(extArgs[4], QString("80")); + TEST_ASSERT_EQ(extArgs[5], QString("route1,route2")); + TEST_ASSERT_EQ(extArgs[6], QString("true")); + + // Verify ArgsData parsing + ArgsData args(extArgs); + TEST_ASSERT_EQ(args.configFile, QString("examples/config/pushpin.conf")); + TEST_ASSERT_EQ(args.logFile, QString("log.txt")); + TEST_ASSERT_EQ(args.logLevel, 2); + TEST_ASSERT_EQ(args.ipcPrefix, QString("ipc:prefix")); + TEST_ASSERT_EQ(args.portOffset, 80); + TEST_ASSERT_EQ(args.routeLines, QStringList({"route1", "route2"})); + TEST_ASSERT_EQ(args.quietCheck, true); + + // Set up mock settings + Settings mock_settings(args.configFile); + if (!args.ipcPrefix.isEmpty()) + mock_settings.setIpcPrefix(args.ipcPrefix);; + if (args.portOffset != -1) + mock_settings.setPortOffset(args.portOffset); // Load settings from command line arguments - Settings settings = app.loadSettingsFromCliArgs(); + Settings settings = args.loadIntoSettings(); - // This should match + // Verify mock settings match the loaded settings TEST_ASSERT_EQ(mock_settings, settings); // Change the port offset to a different value mock_settings.setPortOffset(60); - // This shouldn't match + // Verify that the mock settings no longer match the loaded settings TEST_ASSERT(!(mock_settings == settings)); + + // Set up valid empty command line arguments + int argc_empty = 7; + char * argv_empty[] = { + (char *) configFile.c_str(), // configFile + (char *) "", // logFile + (char *) "2", // logLevel + (char *) "", // ipcPrefix + (char *) "", // portOffset + (char *) "", // routeLines + (char *) "false", // quietCheck + }; + + // Set up QCoreApplication with empty arguments + QCoreApplication qapp_empty(argc_empty, argv_empty); + HandlerApp app_empty; + QStringList extArgs_empty = qapp_empty.arguments(); + + // Verify the arguments + TEST_ASSERT_EQ(extArgs[0], QString("examples/config/pushpin.conf")); + TEST_ASSERT_EQ(extArgs[1], QString("")); + TEST_ASSERT_EQ(extArgs[2], QString("2")); + TEST_ASSERT_EQ(extArgs[3], QString("")); + TEST_ASSERT_EQ(extArgs[4], QString("")); + TEST_ASSERT_EQ(extArgs[5], QString("")); + TEST_ASSERT_EQ(extArgs[6], QString("false")); + + // Verify ArgsData parsing with empty arguments + ArgsData args_empty(extArgs_empty); + TEST_ASSERT_EQ(args_empty.configFile, QString("examples/config/pushpin.conf")); + TEST_ASSERT_EQ(args_empty.logFile, QString("")); + TEST_ASSERT_EQ(args_empty.logLevel, 2); + TEST_ASSERT_EQ(args_empty.ipcPrefix, QString("")); + TEST_ASSERT_EQ(args_empty.portOffset, -1); + TEST_ASSERT_EQ(args_empty.routeLines, QStringList()); + TEST_ASSERT_EQ(args_empty.quietCheck, false); + + // Load settings from empty command line arguments + Settings settings_empty = args_empty.loadIntoSettings(); + + // Set up mock settings + Settings mock_settings_empty(args_empty.configFile); + if (!args_empty.ipcPrefix.isEmpty()) + mock_settings_empty.setIpcPrefix(args_empty.ipcPrefix); + if (args_empty.portOffset != -1) + mock_settings_empty.setPortOffset(args_empty.portOffset); + + // Verify mock settings match the loaded settings + TEST_ASSERT_EQ(mock_settings_empty, settings_empty); } extern "C" int handlerargs_test(ffi::TestException *out_ex) diff --git a/src/proxy/app.cpp b/src/proxy/app.cpp index fafc0cf44..424ebea29 100644 --- a/src/proxy/app.cpp +++ b/src/proxy/app.cpp @@ -22,6 +22,7 @@ */ #include "app.h" +#include "argsdata.h" #include #include @@ -44,6 +45,9 @@ #include "domainmap.h" #include "engine.h" #include "config.h" +#include +#include +#include static void trimlist(QStringList *list) { @@ -101,92 +105,6 @@ enum CommandLineParseResult CommandLineHelpRequested }; -class ArgsData -{ -public: - QString configFile; - QString logFile; - int logLevel; - QString ipcPrefix; - QStringList routeLines; - bool quietCheck; - - ArgsData() : - logLevel(-1), - quietCheck(false) - { - } -}; - -static CommandLineParseResult parseCommandLine(QCommandLineParser *parser, ArgsData *args, QString *errorMessage) -{ - parser->setSingleDashWordOptionMode(QCommandLineParser::ParseAsLongOptions); - const QCommandLineOption configFileOption("config", "Config file.", "file"); - parser->addOption(configFileOption); - const QCommandLineOption logFileOption("logfile", "File to log to.", "file"); - parser->addOption(logFileOption); - const QCommandLineOption logLevelOption("loglevel", "Log level (default: 2).", "x"); - parser->addOption(logLevelOption); - const QCommandLineOption verboseOption("verbose", "Verbose output. Same as --loglevel=3."); - parser->addOption(verboseOption); - const QCommandLineOption ipcPrefixOption("ipc-prefix", "Override ipc_prefix config option.", "prefix"); - parser->addOption(ipcPrefixOption); - const QCommandLineOption routeOption("route", "Add route (overrides routes file).", "line"); - parser->addOption(routeOption); - const QCommandLineOption quietCheckOption("quiet-check", "Log update checks in Zurl as debug level."); - parser->addOption(quietCheckOption); - const QCommandLineOption helpOption = parser->addHelpOption(); - const QCommandLineOption versionOption = parser->addVersionOption(); - - if(!parser->parse(QCoreApplication::arguments())) - { - *errorMessage = parser->errorText(); - return CommandLineError; - } - - if(parser->isSet(versionOption)) - return CommandLineVersionRequested; - - if(parser->isSet(helpOption)) - return CommandLineHelpRequested; - - if(parser->isSet(configFileOption)) - args->configFile = parser->value(configFileOption); - - if(parser->isSet(logFileOption)) - args->logFile = parser->value(logFileOption); - - if(parser->isSet(logLevelOption)) - { - bool ok; - int x = parser->value(logLevelOption).toInt(&ok); - if(!ok || x < 0) - { - *errorMessage = "error: loglevel must be greater than or equal to 0"; - return CommandLineError; - } - - args->logLevel = x; - } - - if(parser->isSet(verboseOption)) - args->logLevel = 3; - - if(parser->isSet(ipcPrefixOption)) - args->ipcPrefix = parser->value(ipcPrefixOption); - - if(parser->isSet(routeOption)) - { - foreach(const QString &r, parser->values(routeOption)) - args->routeLines += r; - } - - if(parser->isSet(quietCheckOption)) - args->quietCheck = true; - - return CommandLineOk; -} - class EngineWorker { public: @@ -396,64 +314,11 @@ class App::Private QCoreApplication::setApplicationName("pushpin-proxy"); QCoreApplication::setApplicationVersion(Config::get().version); - QCommandLineParser parser; - parser.setApplicationDescription("Pushpin proxy component."); - - ArgsData args; - QString errorMessage; - switch(parseCommandLine(&parser, &args, &errorMessage)) - { - case CommandLineOk: - break; - case CommandLineError: - fprintf(stderr, "%s\n\n%s", qPrintable(errorMessage), qPrintable(parser.helpText())); - return 1; - case CommandLineVersionRequested: - printf("%s %s\n", qPrintable(QCoreApplication::applicationName()), - qPrintable(QCoreApplication::applicationVersion())); - return 0; - case CommandLineHelpRequested: - parser.showHelp(); - Q_UNREACHABLE(); - } - - if(args.logLevel != -1) - log_setOutputLevel(args.logLevel); - else - log_setOutputLevel(LOG_LEVEL_INFO); - - if(!args.logFile.isEmpty()) - { - if(!log_setFile(args.logFile)) - { - log_error("failed to open log file: %s", qPrintable(args.logFile)); - return 1; - } - } - - log_debug("starting..."); - - QString configFile = args.configFile; - if(configFile.isEmpty()) - configFile = QDir(Config::get().configDir).filePath("pushpin.conf"); - - // QSettings doesn't inform us if the config file doesn't exist, so do that ourselves - { - QFile file(configFile); - if(!file.open(QIODevice::ReadOnly)) - { - log_error("failed to open %s, and --config not passed", qPrintable(configFile)); - return 1; - } - } - - QDir configDir = QFileInfo(configFile).absoluteDir(); - - Settings settings(configFile); - - if(!args.ipcPrefix.isEmpty()) - settings.setIpcPrefix(args.ipcPrefix); + QStringList extArgs = QCoreApplication::arguments(); + ArgsData args(extArgs); + Settings settings = args.loadIntoSettings(); + QDir configDir = QFileInfo(args.configFile).absoluteDir(); QStringList services = settings.value("runner/services").toStringList(); int workerCount = settings.value("proxy/workers", 1).toInt(); @@ -791,4 +656,4 @@ App::~App() = default; int App::run() { return Private::run(); -} +} \ No newline at end of file diff --git a/src/runner/pushpinproxyservice.cpp b/src/runner/pushpinproxyservice.cpp index 44ea50a2a..f40be6d6f 100644 --- a/src/runner/pushpinproxyservice.cpp +++ b/src/runner/pushpinproxyservice.cpp @@ -37,22 +37,22 @@ PushpinProxyService::PushpinProxyService( bool quietCheck) { args_ += binFile; - args_ += "--config=" + configFile; + args_ += "--config-file=" + configFile; if(!ipcPrefix.isEmpty()) args_ += "--ipc-prefix=" + ipcPrefix; if(!logDir.isEmpty()) { - args_ += "--logfile=" + QDir(logDir).filePath(filePrefix + "pushpin-proxy.log"); + args_ += "--log-file=" + QDir(logDir).filePath(filePrefix + "pushpin-proxy.log"); setStandardOutputFile(QProcess::nullDevice()); } if(logLevel >= 0) - args_ += "--loglevel=" + QString::number(logLevel); + args_ += "--log-level=" + QString::number(logLevel); foreach(const QString &route, routeLines) - args_ += "--route=" + route; + args_ += "--routes=" + route; if(quietCheck) args_ += "--quiet-check"; From 2ee4081619cb3d831613deb967ca0d61feae1b6a Mon Sep 17 00:00:00 2001 From: Serra Date: Wed, 18 Jun 2025 13:57:16 -0700 Subject: [PATCH 11/16] Updated args values in runner --- src/core/ccliargs.rs | 16 ++++++++-------- src/handler/handlerargstest.cpp | 14 +++++++------- src/runner/runnermain.cpp | 1 + src/runner/service.rs | 23 ++++++++++++++++++----- 4 files changed, 34 insertions(+), 20 deletions(-) diff --git a/src/core/ccliargs.rs b/src/core/ccliargs.rs index 1f3a19901..d9bf23c42 100644 --- a/src/core/ccliargs.rs +++ b/src/core/ccliargs.rs @@ -49,9 +49,9 @@ pub struct CCliArgs { #[arg(long, value_name = "offset", value_parser = clap::value_parser!(u32))] pub port_offset: Option, - /// Add route (overrides routes file) - #[arg(long, value_name = "route")] - pub route: Option>, + /// Add routes (overrides routes file) + #[arg(long, value_name = "routes")] + pub routes: Option>, /// Log update checks in Zurl as debug level #[arg(long, value_name = "quiet-check", default_value_t = false)] @@ -117,7 +117,7 @@ impl IntoIterator for CCliArgs { args.push(( "routes".to_string(), - self.route.iter().map(|r| r.join(",")).collect::(), + self.routes.iter().map(|r| r.join(",")).collect::(), )); args.push(( @@ -148,7 +148,7 @@ mod tests { log_level: 3, ipc_prefix: Some("ipc".to_string()), port_offset: Some(8080), - route: Some(vec!["route1".to_string(), "route2".to_string()]), + routes: Some(vec!["route1".to_string(), "route2".to_string()]), quiet_check: true, }; @@ -159,7 +159,7 @@ mod tests { assert_eq!(verified_args.log_level, 3); assert_eq!(verified_args.ipc_prefix, Some("ipc".to_string())); assert_eq!(verified_args.port_offset, Some(8080)); - assert_eq!(verified_args.route, Some(vec!["route1".to_string(), "route2".to_string()])); + assert_eq!(verified_args.routes, Some(vec!["route1".to_string(), "route2".to_string()])); assert_eq!(verified_args.quiet_check, true); // Verify conversion to OsString vector @@ -180,7 +180,7 @@ mod tests { log_level: 2, ipc_prefix: None, port_offset: None, - route: None, + routes: None, quiet_check: false, }; @@ -192,7 +192,7 @@ mod tests { assert_eq!(verified_empty_args.log_level, 2); assert_eq!(verified_empty_args.ipc_prefix, None); assert_eq!(verified_empty_args.port_offset, None); - assert_eq!(verified_empty_args.route, None); + assert_eq!(verified_empty_args.routes, None); assert_eq!(verified_empty_args.quiet_check, false); // Verify conversion to OsString vector diff --git a/src/handler/handlerargstest.cpp b/src/handler/handlerargstest.cpp index 88b9f95b3..7a7fa8c08 100644 --- a/src/handler/handlerargstest.cpp +++ b/src/handler/handlerargstest.cpp @@ -105,13 +105,13 @@ void handlerargstest() QStringList extArgs_empty = qapp_empty.arguments(); // Verify the arguments - TEST_ASSERT_EQ(extArgs[0], QString("examples/config/pushpin.conf")); - TEST_ASSERT_EQ(extArgs[1], QString("")); - TEST_ASSERT_EQ(extArgs[2], QString("2")); - TEST_ASSERT_EQ(extArgs[3], QString("")); - TEST_ASSERT_EQ(extArgs[4], QString("")); - TEST_ASSERT_EQ(extArgs[5], QString("")); - TEST_ASSERT_EQ(extArgs[6], QString("false")); + TEST_ASSERT_EQ(extArgs_empty[0], QString("examples/config/pushpin.conf")); + TEST_ASSERT_EQ(extArgs_empty[1], QString("")); + TEST_ASSERT_EQ(extArgs_empty[2], QString("2")); + TEST_ASSERT_EQ(extArgs_empty[3], QString("")); + TEST_ASSERT_EQ(extArgs_empty[4], QString("")); + TEST_ASSERT_EQ(extArgs_empty[5], QString("")); + TEST_ASSERT_EQ(extArgs_empty[6], QString("false")); // Verify ArgsData parsing with empty arguments ArgsData args_empty(extArgs_empty); diff --git a/src/runner/runnermain.cpp b/src/runner/runnermain.cpp index 5be2a6ff7..02495c6a6 100644 --- a/src/runner/runnermain.cpp +++ b/src/runner/runnermain.cpp @@ -48,6 +48,7 @@ extern "C" { int runner_main(int argc, char **argv) { QCoreApplication qapp(argc, argv); + printf("This gets called\n"); RunnerAppMain appMain; QTimer::singleShot(0, [&appMain]() {appMain.start();}); diff --git a/src/runner/service.rs b/src/runner/service.rs index d4e4586ba..81e9286fd 100644 --- a/src/runner/service.rs +++ b/src/runner/service.rs @@ -322,21 +322,26 @@ impl PushpinProxyService { let mut args: Vec = vec![]; let service_name = "proxy"; + // Proxy bin path args.push(settings.proxy_bin.display().to_string()); - args.push(format!("--config={}", settings.config_file.display())); + // Config file + args.push(format!("--config-file={}", settings.config_file.display())); + + // Ipc prefix if !settings.ipc_prefix.is_empty() { args.push(format!("--ipc-prefix={}", settings.ipc_prefix)); } + + // Log level let log_level = match settings.log_levels.get("proxy") { Some(&x) => x, None => settings.log_levels.get("default").unwrap().to_owned(), }; - args.push(format!("--loglevel={}", log_level)); + args.push(format!("--log-level={}", log_level)); - for route in settings.route_lines.clone() { - args.push(format!("--route={}", route)); - } + // Routes + args.push(format!("--routes={}", settings.route_lines.join(","))); Self { service: Service::new(String::from(service_name), log_level), @@ -361,15 +366,23 @@ impl PushpinHandlerService { let mut args: Vec = vec![]; let service_name = "handler"; + // Handler bin path args.push(settings.handler_bin.display().to_string()); + + // Config file args.push(format!("--config-file={}", settings.config_file.display())); + // Port offset if settings.port_offset > 0 { args.push(format!("--port-offset={}", settings.port_offset)); } + + // Ipc prefix if !settings.ipc_prefix.is_empty() { args.push(format!("--ipc-prefix={}", settings.ipc_prefix)); } + + // Log level let log_level = match settings.log_levels.get("handler") { Some(&x) => x, None => settings.log_levels.get("default").unwrap().to_owned(), From 8f1830442f618cace6b05a0176fac6ce430071f8 Mon Sep 17 00:00:00 2001 From: Serra Date: Thu, 3 Jul 2025 16:53:20 -0400 Subject: [PATCH 12/16] Adds proxy argument handling unit tests --- src/lib.rs | 1 + src/proxy/mod.rs | 10 +++ src/proxy/proxyargstest.cpp | 145 ++++++++++++++++++++++++++++++++++++ src/proxy/tests.pri | 4 +- 4 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 src/proxy/proxyargstest.cpp diff --git a/src/lib.rs b/src/lib.rs index f2976e73c..9d7d64085 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -149,6 +149,7 @@ pub mod ffi { pub fn publishitem_test(out_ex: *mut TestException) -> libc::c_int; pub fn handlerengine_test(out_ex: *mut TestException) -> libc::c_int; pub fn handlerargs_test(out_ex: *mut TestException) -> libc::c_int; + pub fn proxyargs_test(out_ex: *mut TestException) -> libc::c_int; pub fn template_test(out_ex: *mut TestException) -> libc::c_int; } } diff --git a/src/proxy/mod.rs b/src/proxy/mod.rs index d5b0815a4..65accc359 100644 --- a/src/proxy/mod.rs +++ b/src/proxy/mod.rs @@ -34,6 +34,11 @@ mod tests { unsafe { ffi::proxyengine_test(out_ex) == 0 } } + fn proxyargs_test(out_ex: &mut TestException) -> bool { + // SAFETY: safe to call + unsafe { ffi::proxyargs_test(out_ex) == 0 } + } + #[test] fn websocketoverhttp() { run_serial(websocketoverhttp_test); @@ -48,4 +53,9 @@ mod tests { fn proxyengine() { run_serial(proxyengine_test); } + + #[test] + fn proxyargs() { + run_serial(proxyargs_test); + } } diff --git a/src/proxy/proxyargstest.cpp b/src/proxy/proxyargstest.cpp new file mode 100644 index 000000000..845d6eebc --- /dev/null +++ b/src/proxy/proxyargstest.cpp @@ -0,0 +1,145 @@ +/* + * Copyright (C) 2025 Fastly, Inc. + * + * This file is part of Pushpin. + * + * $FANOUT_BEGIN_LICENSE:APACHE2$ + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * $FANOUT_END_LICENSE$ + */ + + + #include "config.h" + #include "app.h" + #include "settings.h" + #include "test.h" + #include "argsdata.h" + + void proxyargstest() + { + // Get file for example config + std::string configFile = "examples/config/pushpin.conf"; + + // Set up valid command line arguments + int argc = 7; + char * argv[] = { + (char *) configFile.c_str(), // configFile + (char *) "log.txt", // logFile + (char *) "2", // logLevel + (char *) "ipc:prefix", // ipcPrefix + (char *) "80", // portOffset + (char *) "route1,route2", // routeLines + (char *) "true", // quietCheck + }; + + // Set up QCoreApplication + QCoreApplication qapp(argc, argv); + App app; + QStringList extArgs = qapp.arguments(); + + // Verify the arguments + TEST_ASSERT_EQ(extArgs[0], QString("examples/config/pushpin.conf")); + TEST_ASSERT_EQ(extArgs[1], QString("log.txt")); + TEST_ASSERT_EQ(extArgs[2], QString("2")); + TEST_ASSERT_EQ(extArgs[3], QString("ipc:prefix")); + TEST_ASSERT_EQ(extArgs[4], QString("80")); + TEST_ASSERT_EQ(extArgs[5], QString("route1,route2")); + TEST_ASSERT_EQ(extArgs[6], QString("true")); + + // Verify ArgsData parsing + ArgsData args(extArgs); + TEST_ASSERT_EQ(args.configFile, QString("examples/config/pushpin.conf")); + TEST_ASSERT_EQ(args.logFile, QString("log.txt")); + TEST_ASSERT_EQ(args.logLevel, 2); + TEST_ASSERT_EQ(args.ipcPrefix, QString("ipc:prefix")); + TEST_ASSERT_EQ(args.portOffset, 80); + TEST_ASSERT_EQ(args.routeLines, QStringList({"route1", "route2"})); + TEST_ASSERT_EQ(args.quietCheck, true); + + // Set up mock settings + Settings mock_settings(args.configFile); + if (!args.ipcPrefix.isEmpty()) + mock_settings.setIpcPrefix(args.ipcPrefix);; + if (args.portOffset != -1) + mock_settings.setPortOffset(args.portOffset); + + // Load settings from command line arguments + Settings settings = args.loadIntoSettings(); + + // Verify mock settings match the loaded settings + TEST_ASSERT_EQ(mock_settings, settings); + + // Change the port offset to a different value + mock_settings.setPortOffset(60); + + // Verify that the mock settings no longer match the loaded settings + TEST_ASSERT(!(mock_settings == settings)); + + // Set up valid empty command line arguments + int argc_empty = 7; + char * argv_empty[] = { + (char *) configFile.c_str(), // configFile + (char *) "", // logFile + (char *) "2", // logLevel + (char *) "", // ipcPrefix + (char *) "", // portOffset + (char *) "", // routeLines + (char *) "false", // quietCheck + }; + + // Set up QCoreApplication with empty arguments + QCoreApplication qapp_empty(argc_empty, argv_empty); + App app_empty; + QStringList extArgs_empty = qapp_empty.arguments(); + + // Verify the arguments + TEST_ASSERT_EQ(extArgs_empty[0], QString("examples/config/pushpin.conf")); + TEST_ASSERT_EQ(extArgs_empty[1], QString("")); + TEST_ASSERT_EQ(extArgs_empty[2], QString("2")); + TEST_ASSERT_EQ(extArgs_empty[3], QString("")); + TEST_ASSERT_EQ(extArgs_empty[4], QString("")); + TEST_ASSERT_EQ(extArgs_empty[5], QString("")); + TEST_ASSERT_EQ(extArgs_empty[6], QString("false")); + + // Verify ArgsData parsing with empty arguments + ArgsData args_empty(extArgs_empty); + TEST_ASSERT_EQ(args_empty.configFile, QString("examples/config/pushpin.conf")); + TEST_ASSERT_EQ(args_empty.logFile, QString("")); + TEST_ASSERT_EQ(args_empty.logLevel, 2); + TEST_ASSERT_EQ(args_empty.ipcPrefix, QString("")); + TEST_ASSERT_EQ(args_empty.portOffset, -1); + TEST_ASSERT_EQ(args_empty.routeLines, QStringList()); + TEST_ASSERT_EQ(args_empty.quietCheck, false); + + // Load settings from empty command line arguments + Settings settings_empty = args_empty.loadIntoSettings(); + + // Set up mock settings + Settings mock_settings_empty(args_empty.configFile); + if (!args_empty.ipcPrefix.isEmpty()) + mock_settings_empty.setIpcPrefix(args_empty.ipcPrefix); + if (args_empty.portOffset != -1) + mock_settings_empty.setPortOffset(args_empty.portOffset); + + // Verify mock settings match the loaded settings + TEST_ASSERT_EQ(mock_settings_empty, settings_empty); + } + + extern "C" int proxyargs_test(ffi::TestException *out_ex) + { + TEST_CATCH(proxyargstest()); + + return 0; + } \ No newline at end of file diff --git a/src/proxy/tests.pri b/src/proxy/tests.pri index a6738761f..4548550b8 100644 --- a/src/proxy/tests.pri +++ b/src/proxy/tests.pri @@ -1,4 +1,6 @@ SOURCES += \ $$PWD/websocketoverhttptest.cpp \ $$PWD/routesfiletest.cpp \ - $$PWD/proxyenginetest.cpp + $$PWD/proxyenginetest.cpp \ + $$PWD/proxyargstest.cpp + From 5b24d0555e061b276461115789a21b7b75114785 Mon Sep 17 00:00:00 2001 From: Serra Date: Mon, 7 Jul 2025 11:56:48 -0400 Subject: [PATCH 13/16] Resolves clippy warnings and removes reduntant comments --- src/core/ccliargs.rs | 64 ++++++++++++-------------------------------- 1 file changed, 17 insertions(+), 47 deletions(-) diff --git a/src/core/ccliargs.rs b/src/core/ccliargs.rs index d9bf23c42..26d361f04 100644 --- a/src/core/ccliargs.rs +++ b/src/core/ccliargs.rs @@ -61,13 +61,10 @@ pub struct CCliArgs { impl CCliArgs { /// Verifies the command line arguments. pub fn verify(mut self) -> Self { - // Get current working directory - let work_dir = env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); + let work_dir = env::current_dir().unwrap_or_default(); + let config_path: Option = self.config_file.as_ref().map(PathBuf::from); - // Convert config_file Option to Option - let config_path = self.config_file.as_ref().map(|s| PathBuf::from(s)); - - // Use get_config_file to find the config file + // Resolve the config file path using get_config_file self.config_file = match get_config_file(&work_dir, config_path) { Ok(path) => Some(path.to_string_lossy().to_string()), Err(e) => { @@ -91,40 +88,16 @@ impl IntoIterator for CCliArgs { type IntoIter = std::vec::IntoIter; fn into_iter(self) -> Self::IntoIter { - let mut args: Vec<(String, String)> = vec![]; - - args.push(( - "config-file".to_string(), - self.config_file.unwrap_or_else(|| "".to_string()) - )); - - args.push(( - "log-file".to_string(), - self.log_file.as_ref().map(|s| s.to_string()).unwrap_or_else(|| "".to_string()), - )); - - args.push(("log-level".to_string(), self.log_level.to_string())); - - args.push(( - "ipc-prefix".to_string(), - self.ipc_prefix.as_ref().map(|s| s.to_string()).unwrap_or_else(|| "".to_string()), - )); - - args.push(( - "port-offset".to_string(), - self.port_offset.as_ref().map(|s| s.to_string()).unwrap_or_else(|| "".to_string()), - )); - - args.push(( - "routes".to_string(), - self.routes.iter().map(|r| r.join(",")).collect::(), - )); - - args.push(( - "quiet-check".to_string(), - self.quiet_check.to_string(), - )); - + let args: Vec<(String, String)> = vec![ + ("config-file".to_string(), self.config_file.unwrap_or_default()), + ("log-file".to_string(), self.log_file.unwrap_or_default()), + ("log-level".to_string(), self.log_level.to_string()), + ("ipc-prefix".to_string(), self.ipc_prefix.unwrap_or_default()), + ("port-offset".to_string(), self.port_offset.map_or("".to_string(), |p| p.to_string())), + ("routes".to_string(), self.routes.map_or("".to_string(), |r| r.join(","))), + ("quiet-check".to_string(), self.quiet_check.to_string()), + ]; + args.into_iter() } } @@ -136,12 +109,11 @@ mod tests { #[test] fn test_ccli_args() { - // Create mock config file + // Create mock values let file = NamedTempFile::new().unwrap(); let config_test_file = file.path().to_str().unwrap().to_string(); let expected_arg_count = 7; - // Create valid CCliArgs let args = CCliArgs { config_file: Some(config_test_file.clone()), log_file: Some("pushpin.log".to_string()), @@ -152,7 +124,7 @@ mod tests { quiet_check: true, }; - // Verify verify() + // Test verify() method let verified_args = args.verify(); assert_eq!(verified_args.config_file, Some(config_test_file.clone())); assert_eq!(verified_args.log_file, Some("pushpin.log".to_string())); @@ -162,7 +134,7 @@ mod tests { assert_eq!(verified_args.routes, Some(vec!["route1".to_string(), "route2".to_string()])); assert_eq!(verified_args.quiet_check, true); - // Verify conversion to OsString vector + // Test OsString conversion let osstring_vec = verified_args.into_osstring_vec(); assert_eq!(osstring_vec.len(), expected_arg_count); assert_eq!(osstring_vec[0], OsString::from(config_test_file)); @@ -173,7 +145,7 @@ mod tests { assert_eq!(osstring_vec[5], OsString::from("route1,route2")); assert_eq!(osstring_vec[6], OsString::from("true")); - // Create valid empty CCliArgs + // Test with empty/default values let empty_args = CCliArgs { config_file: None, log_file: None, @@ -184,7 +156,6 @@ mod tests { quiet_check: false, }; - // Verify verify() let verified_empty_args = empty_args.verify(); let default_config_file = get_config_file(&env::current_dir().unwrap(), None).unwrap().to_string_lossy().to_string(); assert_eq!(verified_empty_args.config_file, Some(default_config_file.clone())); @@ -195,7 +166,6 @@ mod tests { assert_eq!(verified_empty_args.routes, None); assert_eq!(verified_empty_args.quiet_check, false); - // Verify conversion to OsString vector let empty_osstring_vec = verified_empty_args.into_osstring_vec(); assert_eq!(empty_osstring_vec.len(), expected_arg_count); assert_eq!(empty_osstring_vec[0], OsString::from(default_config_file)); From 153588ac154fb70d62c49ecad40169e4bd4aa733 Mon Sep 17 00:00:00 2001 From: Serra Date: Mon, 7 Jul 2025 12:06:48 -0400 Subject: [PATCH 14/16] Resolves linter issues --- build.rs | 6 ++++- src/bin/pushpin-handler.rs | 10 +++---- src/bin/pushpin-proxy.rs | 6 ++--- src/core/ccliargs.rs | 53 ++++++++++++++++++++++++++------------ src/core/mod.rs | 3 +-- src/handler/mod.rs | 1 - 6 files changed, 50 insertions(+), 29 deletions(-) diff --git a/build.rs b/build.rs index 22b47041d..ce4c3c494 100644 --- a/build.rs +++ b/build.rs @@ -353,7 +353,11 @@ fn get_qt_lib_prefix(lib_dir: &Path, version_maj: u16) -> Result Result> { - let paths = ["/usr/local/include", "/usr/include", "/opt/homebrew/include"]; + let paths = [ + "/usr/local/include", + "/usr/include", + "/opt/homebrew/include", + ]; let version_filename = "boost/version.hpp"; for path in paths { diff --git a/src/bin/pushpin-handler.rs b/src/bin/pushpin-handler.rs index 03af9ffdd..0562349e6 100644 --- a/src/bin/pushpin-handler.rs +++ b/src/bin/pushpin-handler.rs @@ -14,11 +14,11 @@ * limitations under the License. */ +use clap::Parser; use pushpin::core::call_c_main; +use pushpin::core::ccliargs::CCliArgs; use pushpin::import_cpp; use std::process::ExitCode; -use pushpin::core::ccliargs::CCliArgs; -use clap::Parser; import_cpp! { fn handler_main(argc: libc::c_int, argv: *const *const libc::c_char) -> libc::c_int; @@ -27,7 +27,5 @@ import_cpp! { fn main() -> ExitCode { let c_cli_args = CCliArgs::parse().verify(); - unsafe { - ExitCode::from(call_c_main(handler_main, c_cli_args.into_osstring_vec())) - } -} \ No newline at end of file + unsafe { ExitCode::from(call_c_main(handler_main, c_cli_args.into_osstring_vec())) } +} diff --git a/src/bin/pushpin-proxy.rs b/src/bin/pushpin-proxy.rs index 509f15532..790d85629 100644 --- a/src/bin/pushpin-proxy.rs +++ b/src/bin/pushpin-proxy.rs @@ -14,11 +14,11 @@ * limitations under the License. */ +use clap::Parser; use pushpin::core::call_c_main; +use pushpin::core::ccliargs::CCliArgs; use pushpin::import_cpp; use std::process::ExitCode; -use pushpin::core::ccliargs::CCliArgs; -use clap::Parser; import_cpp! { fn proxy_main(argc: libc::c_int, argv: *const *const libc::c_char) -> libc::c_int; @@ -26,6 +26,6 @@ import_cpp! { fn main() -> ExitCode { let c_cli_args = CCliArgs::parse().verify(); - + unsafe { ExitCode::from(call_c_main(proxy_main, c_cli_args.into_osstring_vec())) } } diff --git a/src/core/ccliargs.rs b/src/core/ccliargs.rs index 26d361f04..d9e1b42e5 100644 --- a/src/core/ccliargs.rs +++ b/src/core/ccliargs.rs @@ -14,12 +14,12 @@ * limitations under the License. */ +use crate::core::config::get_config_file; +use crate::core::version; +use clap::{arg, Parser}; use std::env; -use std::ffi::{OsString}; +use std::ffi::OsString; use std::path::PathBuf; -use clap::{Parser, arg}; -use crate::core::version; -use crate::core::config::get_config_file; // Struct to hold the command line arguments #[derive(Parser, Debug)] @@ -63,7 +63,7 @@ impl CCliArgs { pub fn verify(mut self) -> Self { let work_dir = env::current_dir().unwrap_or_default(); let config_path: Option = self.config_file.as_ref().map(PathBuf::from); - + // Resolve the config file path using get_config_file self.config_file = match get_config_file(&work_dir, config_path) { Ok(path) => Some(path.to_string_lossy().to_string()), @@ -89,15 +89,27 @@ impl IntoIterator for CCliArgs { fn into_iter(self) -> Self::IntoIter { let args: Vec<(String, String)> = vec![ - ("config-file".to_string(), self.config_file.unwrap_or_default()), + ( + "config-file".to_string(), + self.config_file.unwrap_or_default(), + ), ("log-file".to_string(), self.log_file.unwrap_or_default()), ("log-level".to_string(), self.log_level.to_string()), - ("ipc-prefix".to_string(), self.ipc_prefix.unwrap_or_default()), - ("port-offset".to_string(), self.port_offset.map_or("".to_string(), |p| p.to_string())), - ("routes".to_string(), self.routes.map_or("".to_string(), |r| r.join(","))), + ( + "ipc-prefix".to_string(), + self.ipc_prefix.unwrap_or_default(), + ), + ( + "port-offset".to_string(), + self.port_offset.map_or("".to_string(), |p| p.to_string()), + ), + ( + "routes".to_string(), + self.routes.map_or("".to_string(), |r| r.join(",")), + ), ("quiet-check".to_string(), self.quiet_check.to_string()), ]; - + args.into_iter() } } @@ -131,9 +143,12 @@ mod tests { assert_eq!(verified_args.log_level, 3); assert_eq!(verified_args.ipc_prefix, Some("ipc".to_string())); assert_eq!(verified_args.port_offset, Some(8080)); - assert_eq!(verified_args.routes, Some(vec!["route1".to_string(), "route2".to_string()])); + assert_eq!( + verified_args.routes, + Some(vec!["route1".to_string(), "route2".to_string()]) + ); assert_eq!(verified_args.quiet_check, true); - + // Test OsString conversion let osstring_vec = verified_args.into_osstring_vec(); assert_eq!(osstring_vec.len(), expected_arg_count); @@ -144,7 +159,7 @@ mod tests { assert_eq!(osstring_vec[4], OsString::from("8080")); assert_eq!(osstring_vec[5], OsString::from("route1,route2")); assert_eq!(osstring_vec[6], OsString::from("true")); - + // Test with empty/default values let empty_args = CCliArgs { config_file: None, @@ -157,8 +172,14 @@ mod tests { }; let verified_empty_args = empty_args.verify(); - let default_config_file = get_config_file(&env::current_dir().unwrap(), None).unwrap().to_string_lossy().to_string(); - assert_eq!(verified_empty_args.config_file, Some(default_config_file.clone())); + let default_config_file = get_config_file(&env::current_dir().unwrap(), None) + .unwrap() + .to_string_lossy() + .to_string(); + assert_eq!( + verified_empty_args.config_file, + Some(default_config_file.clone()) + ); assert_eq!(verified_empty_args.log_file, None); assert_eq!(verified_empty_args.log_level, 2); assert_eq!(verified_empty_args.ipc_prefix, None); @@ -176,4 +197,4 @@ mod tests { assert_eq!(empty_osstring_vec[5], OsString::from("")); assert_eq!(empty_osstring_vec[6], OsString::from("false")); } -} \ No newline at end of file +} diff --git a/src/core/mod.rs b/src/core/mod.rs index 68904b6d1..d60ada1e5 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -16,6 +16,7 @@ pub mod arena; pub mod buffer; +pub mod ccliargs; pub mod channel; pub mod config; pub mod defer; @@ -39,7 +40,6 @@ pub mod timer; pub mod tnetstring; pub mod waker; pub mod zmq; -pub mod ccliargs; use std::env; use std::ffi::{CString, OsStr}; @@ -154,7 +154,6 @@ mod tests { unsafe { ffi::eventloop_test(out_ex) == 0 } } - #[test] fn httpheaders() { run_serial(httpheaders_test); diff --git a/src/handler/mod.rs b/src/handler/mod.rs index 0201fe366..f49499cbe 100644 --- a/src/handler/mod.rs +++ b/src/handler/mod.rs @@ -98,5 +98,4 @@ mod tests { fn handlerargs() { run_serial(handlerargs_test); } - } From 63d61093c45051926c0d33435d9ced34cc38c7da Mon Sep 17 00:00:00 2001 From: Serra Date: Mon, 7 Jul 2025 12:21:52 -0400 Subject: [PATCH 15/16] Resolves clippy warning --- src/runner/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/runner/mod.rs b/src/runner/mod.rs index ec4ddeee2..bd2856b2f 100644 --- a/src/runner/mod.rs +++ b/src/runner/mod.rs @@ -37,7 +37,6 @@ use crate::core::{is_debug_build, version}; version = version(), about = "Reverse proxy for realtime web services." )] - pub struct CliArgs { #[arg(long, value_name = "file", help = "Config file.")] pub config: Option, From ce7de89888fc74c901f4f8ed38ccdfcd423145e7 Mon Sep 17 00:00:00 2001 From: Serra Date: Wed, 16 Jul 2025 10:53:47 -0700 Subject: [PATCH 16/16] Removes stray debuggging statement --- src/runner/runnermain.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/runner/runnermain.cpp b/src/runner/runnermain.cpp index 02495c6a6..5be2a6ff7 100644 --- a/src/runner/runnermain.cpp +++ b/src/runner/runnermain.cpp @@ -48,7 +48,6 @@ extern "C" { int runner_main(int argc, char **argv) { QCoreApplication qapp(argc, argv); - printf("This gets called\n"); RunnerAppMain appMain; QTimer::singleShot(0, [&appMain]() {appMain.start();});