diff --git a/src/core/config.rs b/src/core/config.rs index c979e67a..2e608197 100644 --- a/src/core/config.rs +++ b/src/core/config.rs @@ -426,6 +426,10 @@ impl CustomConfig { } } +pub fn default_config_file() -> PathBuf { + PathBuf::from(env!("CONFIG_DIR")).join("pushpin.conf") +} + pub fn get_config_file( work_dir: &Path, arg_config: Option, @@ -446,10 +450,7 @@ pub fn get_config_file( .join("pushpin.conf"), ); // default - config_files.push(PathBuf::from(format!( - "{}/pushpin.conf", - env!("CONFIG_DIR") - ))); + config_files.push(default_config_file()); } } diff --git a/src/handler/cliargs.rs b/src/handler/cliargs.rs index 48cd09e4..6fd65aeb 100644 --- a/src/handler/cliargs.rs +++ b/src/handler/cliargs.rs @@ -14,10 +14,9 @@ * limitations under the License. */ -use crate::core::config::get_config_file; +use crate::core::config::default_config_file; use crate::core::version; use clap::{arg, Parser}; -use std::env; use std::ffi::CString; use std::path::PathBuf; @@ -57,17 +56,18 @@ pub struct CliArgs { impl CliArgs { /// Verifies the command line arguments. 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()), - Err(e) => { - eprintln!("error: failed to find configuration file: {}", e); - std::process::exit(1); - } - }; + let config_file = self + .config_file + .map(PathBuf::from) + .unwrap_or_else(default_config_file); + + // FIXME: don't put back as a string after resolving + self.config_file = Some( + config_file + .to_str() + .expect("path sourced from string should convert") + .to_string(), + ); if self.verbose { self.log_level = 3; @@ -81,12 +81,7 @@ impl CliArgs { .config_file .as_ref() .map_or_else( - || { - let work_dir = std::env::current_dir().unwrap_or_default(); - let default_config = get_config_file(&work_dir, None) - .unwrap_or_else(|_| "examples/config/pushpin.conf".into()); - CString::new(default_config.to_string_lossy().to_string()).unwrap() - }, + || CString::new("").unwrap(), |s| CString::new(s.as_str()).unwrap(), ) .into_raw(); @@ -120,13 +115,15 @@ impl CliArgs { } pub mod ffi { + use std::ffi::{c_char, c_int, c_uint}; + #[repr(C)] pub struct HandlerCliArgs { - pub config_file: *mut libc::c_char, - pub log_file: *mut libc::c_char, - pub log_level: libc::c_uint, - pub ipc_prefix: *mut libc::c_char, - pub port_offset: libc::c_int, + pub config_file: *mut c_char, + pub log_file: *mut c_char, + pub log_level: c_uint, + pub ipc_prefix: *mut c_char, + pub port_offset: c_int, } } @@ -174,8 +171,6 @@ mod tests { port_offset: Some(8080), }; - let args_ffi = args.to_ffi(); - // Test verify() method let verified_args = args.verify(); assert_eq!(verified_args.config_file, Some(config_test_file.clone())); @@ -185,6 +180,8 @@ mod tests { assert_eq!(verified_args.ipc_prefix, Some("ipc".to_string())); assert_eq!(verified_args.port_offset, Some(8080)); + let args_ffi = verified_args.to_ffi(); + // Test conversion to C++-compatible struct unsafe { assert_eq!( @@ -224,14 +221,9 @@ mod tests { port_offset: None, }; - let empty_args_ffi = empty_args.to_ffi(); - // Test verify() with empty args 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(); + let default_config_file = default_config_file().to_str().unwrap().to_string(); assert_eq!( verified_empty_args.config_file, Some(default_config_file.clone()) @@ -242,6 +234,8 @@ mod tests { assert_eq!(verified_empty_args.ipc_prefix, None); assert_eq!(verified_empty_args.port_offset, None); + let empty_args_ffi = verified_empty_args.to_ffi(); + // Test conversion to C++-compatible struct unsafe { assert_eq!( diff --git a/src/handler/handlerapp.cpp b/src/handler/handlerapp.cpp index f94ea482..b721d7c4 100644 --- a/src/handler/handlerapp.cpp +++ b/src/handler/handlerapp.cpp @@ -104,9 +104,6 @@ class HandlerApp::Private QCoreApplication::setApplicationVersion(Config::get().version); HandlerArgsData args(argsFfi); - Settings settings(args.configFile); - if (!args.ipcPrefix.isEmpty()) settings.setIpcPrefix(args.ipcPrefix); - if (args.portOffset != -1) settings.setPortOffset(args.portOffset); // Set the log level if(args.logLevel != -1) @@ -131,11 +128,15 @@ class HandlerApp::Private QFile file(args.configFile); if(!file.open(QIODevice::ReadOnly)) { - log_error("failed to open %s", qPrintable(args.configFile)); + log_error("failed to open config file: %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); + QStringList services = settings.value("runner/services").toStringList(); QStringList connmgr_in_stream_specs = settings.value("proxy/connmgr_in_stream_specs").toStringList(); diff --git a/src/proxy/cliargs.rs b/src/proxy/cliargs.rs index 83862a3e..050caae0 100644 --- a/src/proxy/cliargs.rs +++ b/src/proxy/cliargs.rs @@ -14,12 +14,12 @@ * limitations under the License. */ -use crate::core::config::get_config_file; +use crate::core::config::default_config_file; use crate::core::version; use clap::{arg, Parser}; -use std::env; -use std::ffi::CString; +use std::ffi::{c_char, CString}; use std::path::PathBuf; +use std::slice; // Struct to hold the command line arguments #[derive(Parser, Debug)] @@ -57,17 +57,18 @@ pub struct CliArgs { impl CliArgs { /// Verifies the command line arguments. 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()), - Err(e) => { - eprintln!("error: failed to find configuration file: {}", e); - std::process::exit(1); - } - }; + let config_file = self + .config_file + .map(PathBuf::from) + .unwrap_or_else(default_config_file); + + // FIXME: don't put back as a string after resolving + self.config_file = Some( + config_file + .to_str() + .expect("path sourced from string should convert") + .to_string(), + ); if self.verbose { self.log_level = 3; @@ -81,12 +82,7 @@ impl CliArgs { .config_file .as_ref() .map_or_else( - || { - let work_dir = std::env::current_dir().unwrap_or_default(); - let default_config = get_config_file(&work_dir, None) - .unwrap_or_else(|_| "examples/config/pushpin.conf".into()); - CString::new(default_config.to_string_lossy().to_string()).unwrap() - }, + || CString::new("").unwrap(), |s| CString::new(s.as_str()).unwrap(), ) .into_raw(); @@ -109,47 +105,47 @@ impl CliArgs { ) .into_raw(); - let (routes, routes_count) = if !self.route.is_empty() { - // Allocate array of string pointers - let routes_array = unsafe { - libc::malloc(self.route.len() * std::mem::size_of::<*mut libc::c_char>()) - as *mut *mut libc::c_char - }; - - // Convert each route to CString and store pointer in array - for (i, item) in self.route.iter().enumerate() { - let c_string = CString::new(item.to_string()).unwrap().into_raw(); - unsafe { - *routes_array.add(i) = c_string; - } - } - - (routes_array, self.route.len() as libc::c_uint) - } else { - let routes_array = unsafe { libc::malloc(0) as *mut *mut libc::c_char }; - (routes_array, 0) - }; + let mut routes: Vec<*mut c_char> = Vec::new(); + + // Convert each route to CString and store pointer in array + for item in &self.route { + let item = item.as_bytes(); + + // ensure no trailing nul byte + let end = item.iter().position(|b| *b == 0).unwrap_or(item.len()); + let item = &item[..end]; + + let c_string = CString::new(item).unwrap().into_raw(); + routes.push(c_string); + } + + let routes = routes.into_boxed_slice(); + + let routes_count = routes.len(); + let routes: *mut [*mut c_char] = Box::into_raw(routes); ffi::ProxyCliArgs { config_file, log_file, log_level: self.log_level, ipc_prefix, - routes, + routes: routes as *mut *mut c_char, routes_count, } } } pub mod ffi { + use std::ffi::{c_char, c_uint}; + #[repr(C)] pub struct ProxyCliArgs { - pub config_file: *mut libc::c_char, - pub log_file: *mut libc::c_char, - pub log_level: libc::c_uint, - pub ipc_prefix: *mut libc::c_char, - pub routes: *mut *mut libc::c_char, - pub routes_count: libc::c_uint, + pub config_file: *mut c_char, + pub log_file: *mut c_char, + pub log_level: c_uint, + pub ipc_prefix: *mut c_char, + pub routes: *mut *mut c_char, + pub routes_count: libc::size_t, } } @@ -177,15 +173,18 @@ pub unsafe extern "C" fn destroy_proxy_cli_args(ffi_args: ffi::ProxyCliArgs) { let _ = CString::from_raw(ffi_args.ipc_prefix); } if !ffi_args.routes.is_null() { + // SAFETY: the raw parts were originally derived from a boxed slice + let routes = unsafe { + Box::from_raw(slice::from_raw_parts_mut( + ffi_args.routes, + ffi_args.routes_count, + )) + }; + // Free each individual route string - for i in 0..ffi_args.routes_count { - let route_ptr = *ffi_args.routes.add(i as usize); - if !route_ptr.is_null() { - let _ = CString::from_raw(route_ptr); - } + for item in routes.iter() { + let _ = CString::from_raw(*item); } - - libc::free(ffi_args.routes as *mut libc::c_void); } } @@ -209,8 +208,6 @@ mod tests { route: vec!["route1".to_string(), "route2".to_string()], }; - let args_ffi = args.to_ffi(); - // Test verify() method let verified_args = args.verify(); assert_eq!(verified_args.config_file, Some(config_test_file.clone())); @@ -223,6 +220,8 @@ mod tests { vec!["route1".to_string(), "route2".to_string()] ); + let args_ffi = verified_args.to_ffi(); + // Test conversion to C++-compatible struct unsafe { assert_eq!( @@ -277,14 +276,9 @@ mod tests { route: Vec::new(), }; - let empty_args_ffi = empty_args.to_ffi(); - // Test verify() with empty args 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(); + let default_config_file = default_config_file().to_str().unwrap().to_string(); assert_eq!( verified_empty_args.config_file, Some(default_config_file.clone()) @@ -295,6 +289,8 @@ mod tests { assert_eq!(verified_empty_args.ipc_prefix, None); assert!(verified_empty_args.route.is_empty()); + let empty_args_ffi = verified_empty_args.to_ffi(); + // Test conversion to C++-compatible struct unsafe { assert_eq!( diff --git a/src/proxy/proxyapp.cpp b/src/proxy/proxyapp.cpp index 6e5bf856..9cd582b2 100644 --- a/src/proxy/proxyapp.cpp +++ b/src/proxy/proxyapp.cpp @@ -327,8 +327,6 @@ class App::Private QCoreApplication::setApplicationVersion(Config::get().version); ProxyArgsData args(argsFfi); - Settings settings(args.configFile); - if (!args.ipcPrefix.isEmpty()) settings.setIpcPrefix(args.ipcPrefix); // Set the log level if(args.logLevel != -1) @@ -353,12 +351,16 @@ class App::Private QFile file(args.configFile); if(!file.open(QIODevice::ReadOnly)) { - log_error("failed to open %s", qPrintable(args.configFile)); + log_error("failed to open config file: %s", qPrintable(args.configFile)); return 1; } } QDir configDir = QFileInfo(args.configFile).absoluteDir(); + + Settings settings(args.configFile); + if (!args.ipcPrefix.isEmpty()) settings.setIpcPrefix(args.ipcPrefix); + QStringList services = settings.value("runner/services").toStringList(); int workerCount = settings.value("proxy/workers", 1).toInt();