diff --git a/src/ngx_module/commands/common.rs b/src/ngx_module/commands/common.rs index d350585..95954fc 100644 --- a/src/ngx_module/commands/common.rs +++ b/src/ngx_module/commands/common.rs @@ -21,7 +21,14 @@ use std::ptr; /// /// * `Some(ngx_str_t)` - Successfully allocated and copied string /// * `None` - Failed to allocate memory -pub(crate) unsafe fn copy_string_to_pool(cf: *mut ngx_conf_t, src: ngx_str_t) -> Option { +/// +/// # Safety +/// +/// The caller must ensure that: +/// * `cf` is a valid pointer to a `ngx_conf_t` structure +/// * `src.data` points to valid memory if `src.len > 0` +/// * The source string is valid UTF-8 if it will be converted to a Rust string +pub unsafe fn copy_string_to_pool(cf: *mut ngx_conf_t, src: ngx_str_t) -> Option { if src.len == 0 { return Some(ngx_str_t { len: 0, diff --git a/src/ngx_module/commands/mod.rs b/src/ngx_module/commands/mod.rs index 52351e0..db9f5c5 100644 --- a/src/ngx_module/commands/mod.rs +++ b/src/ngx_module/commands/mod.rs @@ -24,7 +24,7 @@ mod asset; mod basic; -mod common; +pub mod common; mod network; mod other; diff --git a/src/ngx_module/logging.rs b/src/ngx_module/logging.rs index 683bc58..ac3e601 100644 --- a/src/ngx_module/logging.rs +++ b/src/ngx_module/logging.rs @@ -32,6 +32,9 @@ static INIT: Once = Once::new(); /// Initialize the logger pub fn init() { INIT.call_once(|| { + // Initialize panic handler for FFI safety + crate::ngx_module::panic_handler::init_panic_hook(); + log::set_logger(&LOGGER) .map(|()| log::set_max_level(log::LevelFilter::Debug)) .expect("Failed to initialize logger"); diff --git a/src/ngx_module/mod.rs b/src/ngx_module/mod.rs index b3f2145..527dc97 100644 --- a/src/ngx_module/mod.rs +++ b/src/ngx_module/mod.rs @@ -31,6 +31,7 @@ pub mod handler; pub mod logging; pub mod metrics; pub mod module; +pub mod panic_handler; pub mod request; pub mod requirements; pub mod response; @@ -69,18 +70,25 @@ pub use runtime::{ pub unsafe extern "C" fn x402_metrics_handler( r: *mut ngx::ffi::ngx_http_request_t, ) -> ngx::ffi::ngx_int_t { + use crate::ngx_module::panic_handler::catch_panic_or_default; + if r.is_null() { return ngx::ffi::NGX_ERROR as ngx::ffi::ngx_int_t; } - let req_mut = ngx::http::Request::from_ngx_http_request(r); - - match x402_metrics_handler_impl(req_mut) { - ngx::core::Status::NGX_OK => ngx::ffi::NGX_OK as ngx::ffi::ngx_int_t, - ngx::core::Status::NGX_ERROR => ngx::ffi::NGX_ERROR as ngx::ffi::ngx_int_t, - ngx::core::Status::NGX_DECLINED => ngx::ffi::NGX_DECLINED as ngx::ffi::ngx_int_t, - _ => ngx::ffi::NGX_ERROR as ngx::ffi::ngx_int_t, - } + catch_panic_or_default( + || { + let req_mut = ngx::http::Request::from_ngx_http_request(r); + match x402_metrics_handler_impl(req_mut) { + ngx::core::Status::NGX_OK => ngx::ffi::NGX_OK as ngx::ffi::ngx_int_t, + ngx::core::Status::NGX_ERROR => ngx::ffi::NGX_ERROR as ngx::ffi::ngx_int_t, + ngx::core::Status::NGX_DECLINED => ngx::ffi::NGX_DECLINED as ngx::ffi::ngx_int_t, + _ => ngx::ffi::NGX_ERROR as ngx::ffi::ngx_int_t, + } + }, + "x402_metrics_handler", + ngx::ffi::NGX_ERROR as ngx::ffi::ngx_int_t, + ) } /// Clear x402 content handler if it's set @@ -147,180 +155,193 @@ unsafe fn clear_x402_content_handler(r: *mut ngx::ffi::ngx_http_request_t, reaso pub unsafe extern "C" fn x402_phase_handler( r: *mut ngx::ffi::ngx_http_request_t, ) -> ngx::ffi::ngx_int_t { + use crate::ngx_module::panic_handler::catch_panic_or_default; + // Validate pointer if r.is_null() { return ngx::ffi::NGX_ERROR as ngx::ffi::ngx_int_t; } - // Create Request object safely using ngx-rust's API - // Request is a zero-cost wrapper (repr(transparent)), so we can safely create it from the raw pointer - let req_mut = ngx::http::Request::from_ngx_http_request(r); - - use crate::ngx_module::logging::log_debug; - use crate::ngx_module::module::get_module_config; - use crate::ngx_module::request::{ - get_http_method, is_websocket_request, should_skip_payment_for_method, - }; + // Wrap the entire handler in panic protection + catch_panic_or_default( + || { + // Create Request object safely using ngx-rust's API + // Request is a zero-cost wrapper (repr(transparent)), so we can safely create it from the raw pointer + let req_mut = ngx::http::Request::from_ngx_http_request(r); - // Skip payment verification for special request types - // These requests should bypass payment verification: - // 1. Certain HTTP methods (OPTIONS, HEAD, TRACE) - used for protocol-level operations - // - OPTIONS: CORS preflight requests sent by browsers before cross-origin requests - // - HEAD: Used to check resource existence without retrieving body - // - TRACE: Used for diagnostic and debugging purposes - // 2. WebSocket upgrades - long-lived connections that use special HTTP Upgrade mechanism. - // Payment verification would interfere with WebSocket handshake, and subsequent - // WebSocket frames are not HTTP requests, so payment verification is not applicable. - // Payment should be handled at application layer for WebSocket connections. - // 3. Subrequests (auth_request, etc.) - detected via raw request pointer - // 4. Internal redirects - detected via raw request pointer + use crate::ngx_module::logging::log_debug; + use crate::ngx_module::module::get_module_config; + use crate::ngx_module::request::{ + get_http_method, is_websocket_request, should_skip_payment_for_method, + }; - // Check if HTTP method should skip payment verification - // This check happens early to avoid unnecessary processing - let request_struct = unsafe { &*r }; - let method_id = request_struct.method; - let detected_method = unsafe { get_http_method(r) }; + // Skip payment verification for special request types + // These requests should bypass payment verification: + // 1. Certain HTTP methods (OPTIONS, HEAD, TRACE) - used for protocol-level operations + // - OPTIONS: CORS preflight requests sent by browsers before cross-origin requests + // - HEAD: Used to check resource existence without retrieving body + // - TRACE: Used for diagnostic and debugging purposes + // 2. WebSocket upgrades - long-lived connections that use special HTTP Upgrade mechanism. + // Payment verification would interfere with WebSocket handshake, and subsequent + // WebSocket frames are not HTTP requests, so payment verification is not applicable. + // Payment should be handled at application layer for WebSocket connections. + // 3. Subrequests (auth_request, etc.) - detected via raw request pointer + // 4. Internal redirects - detected via raw request pointer - log_debug( - Some(req_mut), - &format!( - "[x402] Phase handler: method_id=0x{:08x}, detected_method={:?}", - method_id, detected_method - ), - ); + // Check if HTTP method should skip payment verification + // This check happens early to avoid unnecessary processing + let request_struct = unsafe { &*r }; + let method_id = request_struct.method; + let detected_method = unsafe { get_http_method(r) }; - if unsafe { should_skip_payment_for_method(r) } { - let method = detected_method.unwrap_or("UNKNOWN"); - log_debug( - Some(req_mut), - &format!( - "[x402] Phase handler: {} request detected (method_id=0x{:08x}), skipping payment verification", - method, method_id - ), - ); - // Clear content handler if it's x402_ngx_handler to prevent payment verification in CONTENT_PHASE - // When returning NGX_DECLINED, nginx will still proceed to CONTENT_PHASE, so we need to clear - // the content handler to prevent duplicate payment verification - unsafe { - clear_x402_content_handler( - r, - &format!("for {} request to prevent payment verification", method), + log_debug( + Some(req_mut), + &format!( + "[x402] Phase handler: method_id=0x{:08x}, detected_method={:?}", + method_id, detected_method + ), ); - } - return ngx::ffi::NGX_DECLINED as ngx::ffi::ngx_int_t; - } - // Check for WebSocket upgrade (can be detected via headers) - if is_websocket_request(req_mut) { - log_debug( - Some(req_mut), - "[x402] Phase handler: WebSocket upgrade detected, skipping payment verification", - ); - // Clear content handler if it's x402_ngx_handler to prevent payment verification in CONTENT_PHASE - unsafe { - clear_x402_content_handler(r, "for WebSocket request to prevent payment verification"); - } - return ngx::ffi::NGX_DECLINED as ngx::ffi::ngx_int_t; - } + if unsafe { should_skip_payment_for_method(r) } { + let method = detected_method.unwrap_or("UNKNOWN"); + log_debug( + Some(req_mut), + &format!( + "[x402] Phase handler: {} request detected (method_id=0x{:08x}), skipping payment verification", + method, method_id + ), + ); + // Clear content handler if it's x402_ngx_handler to prevent payment verification in CONTENT_PHASE + // When returning NGX_DECLINED, nginx will still proceed to CONTENT_PHASE, so we need to clear + // the content handler to prevent duplicate payment verification + unsafe { + clear_x402_content_handler( + r, + &format!("for {} request to prevent payment verification", method), + ); + } + return ngx::ffi::NGX_DECLINED as ngx::ffi::ngx_int_t; + } - // Check for subrequest using raw request pointer - // Subrequests have r->parent != NULL - unsafe { - let r_raw = r.cast_const(); - if !r_raw.is_null() { - let parent = (*r_raw).parent; - if !parent.is_null() { + // Check for WebSocket upgrade (can be detected via headers) + if is_websocket_request(req_mut) { log_debug( Some(req_mut), - "[x402] Phase handler: Subrequest detected (parent != NULL), skipping payment verification", + "[x402] Phase handler: WebSocket upgrade detected, skipping payment verification", ); + // Clear content handler if it's x402_ngx_handler to prevent payment verification in CONTENT_PHASE + unsafe { + clear_x402_content_handler( + r, + "for WebSocket request to prevent payment verification", + ); + } return ngx::ffi::NGX_DECLINED as ngx::ffi::ngx_int_t; } - } - } - // Check for internal redirect using raw request pointer - // Internal redirects have r->internal = 1 (unsigned flag) - // In nginx C code, internal is a field in ngx_http_request_t structure - unsafe { - let r_raw = r.cast_const(); - if !r_raw.is_null() { - // Access internal field directly from C structure - // internal is an unsigned integer field in ngx_http_request_t - // We need to access it via pointer dereference - // Note: This is accessing the raw C structure, so we need to be careful - let request_struct = &*r_raw; - // Try to access internal field - it should be a field, not a method - // If this doesn't compile, we may need to use offset_of! macro or other approach - // For now, we'll use a workaround: check if uri.data starts with @ (named locations) - // Named locations (like @fallback) are always internal redirects - let uri = request_struct.uri; - if !uri.data.is_null() && uri.len > 0 { - // Check if URI starts with '@' which indicates named location (always internal) - let uri_slice = std::slice::from_raw_parts(uri.data.cast_const(), uri.len.min(1)); - if uri_slice[0] == b'@' { - log_debug( + // Check for subrequest using raw request pointer + // Subrequests have r->parent != NULL + unsafe { + let r_raw = r.cast_const(); + if !r_raw.is_null() { + let parent = (*r_raw).parent; + if !parent.is_null() { + log_debug( Some(req_mut), - "[x402] Phase handler: Internal redirect detected (named location @), skipping payment verification", + "[x402] Phase handler: Subrequest detected (parent != NULL), skipping payment verification", ); - return ngx::ffi::NGX_DECLINED as ngx::ffi::ngx_int_t; + return ngx::ffi::NGX_DECLINED as ngx::ffi::ngx_int_t; + } } } - } - } - // Check if module is enabled for this location - let conf = match get_module_config(req_mut) { - Ok(c) => c, - Err(_) => { - // Module not configured for this location, decline to let other handlers process - return ngx::ffi::NGX_DECLINED as ngx::ffi::ngx_int_t; - } - }; + // Check for internal redirect using raw request pointer + // Internal redirects have r->internal = 1 (unsigned flag) + // In nginx C code, internal is a field in ngx_http_request_t structure + unsafe { + let r_raw = r.cast_const(); + if !r_raw.is_null() { + // Access internal field directly from C structure + // internal is an unsigned integer field in ngx_http_request_t + // We need to access it via pointer dereference + // Note: This is accessing the raw C structure, so we need to be careful + let request_struct = &*r_raw; + // Try to access internal field - it should be a field, not a method + // If this doesn't compile, we may need to use offset_of! macro or other approach + // For now, we'll use a workaround: check if uri.data starts with @ (named locations) + // Named locations (like @fallback) are always internal redirects + let uri = request_struct.uri; + if !uri.data.is_null() && uri.len > 0 { + // Check if URI starts with '@' which indicates named location (always internal) + let uri_slice = + std::slice::from_raw_parts(uri.data.cast_const(), uri.len.min(1)); + if uri_slice[0] == b'@' { + log_debug( + Some(req_mut), + "[x402] Phase handler: Internal redirect detected (named location @), skipping payment verification", + ); + return ngx::ffi::NGX_DECLINED as ngx::ffi::ngx_int_t; + } + } + } + } - // Check if module is enabled - if conf.enabled == 0 { - return ngx::ffi::NGX_DECLINED as ngx::ffi::ngx_int_t; - } + // Check if module is enabled for this location + let conf = match get_module_config(req_mut) { + Ok(c) => c, + Err(_) => { + // Module not configured for this location, decline to let other handlers process + return ngx::ffi::NGX_DECLINED as ngx::ffi::ngx_int_t; + } + }; - // Module is enabled - perform payment verification - // This will verify payment and send 402 if needed, or allow request to proceed - use crate::ngx_module::handler::HandlerResult; - let (status, result) = x402_ngx_handler_impl(req_mut); - match (status, result) { - (ngx::core::Status::NGX_OK, HandlerResult::PaymentValid) => { - // Payment verified - allow request to continue - // If content handler is x402_ngx_handler (no proxy_pass), clear it to prevent - // duplicate payment verification in CONTENT_PHASE. If content handler is something - // else (like proxy_pass), keep it so it runs in CONTENT_PHASE. - unsafe { - clear_x402_content_handler( - r, - "after payment verification to prevent duplicate verification", - ); + // Check if module is enabled + if conf.enabled == 0 { + return ngx::ffi::NGX_DECLINED as ngx::ffi::ngx_int_t; } - // This will proceed to CONTENT_PHASE where proxy_pass handler will run (if set) - ngx::ffi::NGX_OK as ngx::ffi::ngx_int_t - } - (ngx::core::Status::NGX_DECLINED, HandlerResult::ResponseSent) => { - // Response was sent (402 or error) - stop processing - // Return OK to indicate we handled the request and prevent further processing - // This prevents proxy_pass from executing - // Note: In nginx, when a response is sent in ACCESS_PHASE, returning NGX_OK - // tells nginx that we've handled the request and it should not proceed to CONTENT_PHASE - ngx::ffi::NGX_OK as ngx::ffi::ngx_int_t - } - (ngx::core::Status::NGX_ERROR, _) => { - // Error occurred during payment verification - // The handler should have sent an appropriate response (402 or 500) - // Return OK to indicate we handled the request and prevent further processing - ngx::ffi::NGX_OK as ngx::ffi::ngx_int_t - } - _ => { - // Unexpected status - return OK to prevent further processing - ngx::ffi::NGX_OK as ngx::ffi::ngx_int_t - } - } + + // Module is enabled - perform payment verification + // This will verify payment and send 402 if needed, or allow request to proceed + use crate::ngx_module::handler::HandlerResult; + let (status, result) = x402_ngx_handler_impl(req_mut); + match (status, result) { + (ngx::core::Status::NGX_OK, HandlerResult::PaymentValid) => { + // Payment verified - allow request to continue + // If content handler is x402_ngx_handler (no proxy_pass), clear it to prevent + // duplicate payment verification in CONTENT_PHASE. If content handler is something + // else (like proxy_pass), keep it so it runs in CONTENT_PHASE. + unsafe { + clear_x402_content_handler( + r, + "after payment verification to prevent duplicate verification", + ); + } + // This will proceed to CONTENT_PHASE where proxy_pass handler will run (if set) + ngx::ffi::NGX_OK as ngx::ffi::ngx_int_t + } + (ngx::core::Status::NGX_DECLINED, HandlerResult::ResponseSent) => { + // Response was sent (402 or error) - stop processing + // Return OK to indicate we handled the request and prevent further processing + // This prevents proxy_pass from executing + // Note: In nginx, when a response is sent in ACCESS_PHASE, returning NGX_OK + // tells nginx that we've handled the request and it should not proceed to CONTENT_PHASE + ngx::ffi::NGX_OK as ngx::ffi::ngx_int_t + } + (ngx::core::Status::NGX_ERROR, _) => { + // Error occurred during payment verification + // The handler should have sent an appropriate response (402 or 500) + // Return OK to indicate we handled the request and prevent further processing + ngx::ffi::NGX_OK as ngx::ffi::ngx_int_t + } + _ => { + // Unexpected status - return OK to prevent further processing + ngx::ffi::NGX_OK as ngx::ffi::ngx_int_t + } + } + }, + "x402_phase_handler", + ngx::ffi::NGX_ERROR as ngx::ffi::ngx_int_t, + ) } /// Main content handler C export @@ -344,17 +365,24 @@ pub unsafe extern "C" fn x402_phase_handler( pub unsafe extern "C" fn x402_ngx_handler( r: *mut ngx::ffi::ngx_http_request_t, ) -> ngx::ffi::ngx_int_t { + use crate::ngx_module::panic_handler::catch_panic_or_default; + if r.is_null() { return ngx::ffi::NGX_ERROR as ngx::ffi::ngx_int_t; } - let req_mut = ngx::http::Request::from_ngx_http_request(r); - - let (status, _result) = x402_ngx_handler_impl(req_mut); - match status { - ngx::core::Status::NGX_OK => ngx::ffi::NGX_OK as ngx::ffi::ngx_int_t, - ngx::core::Status::NGX_ERROR => ngx::ffi::NGX_ERROR as ngx::ffi::ngx_int_t, - ngx::core::Status::NGX_DECLINED => ngx::ffi::NGX_DECLINED as ngx::ffi::ngx_int_t, - _ => ngx::ffi::NGX_ERROR as ngx::ffi::ngx_int_t, - } + catch_panic_or_default( + || { + let req_mut = ngx::http::Request::from_ngx_http_request(r); + let (status, _result) = x402_ngx_handler_impl(req_mut); + match status { + ngx::core::Status::NGX_OK => ngx::ffi::NGX_OK as ngx::ffi::ngx_int_t, + ngx::core::Status::NGX_ERROR => ngx::ffi::NGX_ERROR as ngx::ffi::ngx_int_t, + ngx::core::Status::NGX_DECLINED => ngx::ffi::NGX_DECLINED as ngx::ffi::ngx_int_t, + _ => ngx::ffi::NGX_ERROR as ngx::ffi::ngx_int_t, + } + }, + "x402_ngx_handler", + ngx::ffi::NGX_ERROR as ngx::ffi::ngx_int_t, + ) } diff --git a/src/ngx_module/module.rs b/src/ngx_module/module.rs index 9970ed1..a176739 100644 --- a/src/ngx_module/module.rs +++ b/src/ngx_module/module.rs @@ -8,6 +8,26 @@ use ngx::http::Request; use std::ffi::c_char; use std::ptr; +/// Macro to merge a string field from previous config to current config +/// +/// This macro safely copies a string field from `prev_conf` to `conf_mut` using +/// the current configuration pool, preventing segfaults from dangling pointers. +/// +/// # Usage +/// +/// ```rust,ignore +/// merge_string_field!(cf, conf_mut, prev_conf, field_name); +/// ``` +macro_rules! merge_string_field { + ($cf:expr, $conf_mut:expr, $prev_conf:expr, $field:ident) => { + if $conf_mut.$field.len == 0 && $prev_conf.$field.len > 0 { + if let Some(copied_str) = copy_string_to_pool($cf, $prev_conf.$field) { + $conf_mut.$field = copied_str; + } + } + }; +} + /// Helper function to get `ngx_http_core_main_conf_t` from `ngx_conf_t` /// /// This is equivalent to `ngx_http_conf_get_module_main_conf(cf`, `ngx_http_core_module`) @@ -139,10 +159,12 @@ unsafe extern "C" fn create_loc_conf(cf: *mut ngx::ffi::ngx_conf_t) -> *mut core /// We cannot set handler here because it causes segmentation faults. /// However, we can verify that handler is still set after merging. unsafe extern "C" fn merge_loc_conf( - _cf: *mut ngx::ffi::ngx_conf_t, + cf: *mut ngx::ffi::ngx_conf_t, prev: *mut core::ffi::c_void, conf: *mut core::ffi::c_void, ) -> *mut c_char { + use crate::ngx_module::commands::common::copy_string_to_pool; + let prev = prev.cast::(); let conf = conf.cast::(); @@ -162,43 +184,21 @@ unsafe extern "C" fn merge_loc_conf( // CRITICAL: Do NOT access clcf in merge_loc_conf - it causes segmentation faults // The handler verification must be done at runtime in the handler itself, not during merge - // Merge string fields: use current if non-empty, otherwise use previous - if conf_mut.amount_str.len == 0 { - conf_mut.amount_str = prev_conf.amount_str; - } - if conf_mut.pay_to_str.len == 0 { - conf_mut.pay_to_str = prev_conf.pay_to_str; - } - if conf_mut.facilitator_url_str.len == 0 { - conf_mut.facilitator_url_str = prev_conf.facilitator_url_str; - } - if conf_mut.description_str.len == 0 { - conf_mut.description_str = prev_conf.description_str; - } - if conf_mut.network_str.len == 0 { - conf_mut.network_str = prev_conf.network_str; - } - if conf_mut.network_id_str.len == 0 { - conf_mut.network_id_str = prev_conf.network_id_str; - } - if conf_mut.resource_str.len == 0 { - conf_mut.resource_str = prev_conf.resource_str; - } - if conf_mut.asset_str.len == 0 { - conf_mut.asset_str = prev_conf.asset_str; - } - if conf_mut.asset_decimals_str.len == 0 { - conf_mut.asset_decimals_str = prev_conf.asset_decimals_str; - } - if conf_mut.timeout_str.len == 0 { - conf_mut.timeout_str = prev_conf.timeout_str; - } - if conf_mut.facilitator_fallback_str.len == 0 { - conf_mut.facilitator_fallback_str = prev_conf.facilitator_fallback_str; - } - if conf_mut.ttl_str.len == 0 { - conf_mut.ttl_str = prev_conf.ttl_str; - } + // Merge string fields: use current if non-empty, otherwise copy from previous using current pool + // IMPORTANT: We must copy strings to the current configuration pool instead of copying pointers + // because prev_conf may use a different memory pool that could be freed, causing segfaults + merge_string_field!(cf, conf_mut, prev_conf, amount_str); + merge_string_field!(cf, conf_mut, prev_conf, pay_to_str); + merge_string_field!(cf, conf_mut, prev_conf, facilitator_url_str); + merge_string_field!(cf, conf_mut, prev_conf, description_str); + merge_string_field!(cf, conf_mut, prev_conf, network_str); + merge_string_field!(cf, conf_mut, prev_conf, network_id_str); + merge_string_field!(cf, conf_mut, prev_conf, resource_str); + merge_string_field!(cf, conf_mut, prev_conf, asset_str); + merge_string_field!(cf, conf_mut, prev_conf, asset_decimals_str); + merge_string_field!(cf, conf_mut, prev_conf, timeout_str); + merge_string_field!(cf, conf_mut, prev_conf, facilitator_fallback_str); + merge_string_field!(cf, conf_mut, prev_conf, ttl_str); // Note: Handler is set in ngx_http_x402 command handler when x402 on; is parsed // We cannot set handler here in merge_loc_conf because accessing clcf during merging diff --git a/src/ngx_module/panic_handler.rs b/src/ngx_module/panic_handler.rs new file mode 100644 index 0000000..6517652 --- /dev/null +++ b/src/ngx_module/panic_handler.rs @@ -0,0 +1,179 @@ +//! Panic handling for FFI boundaries +//! +//! This module provides panic handling utilities to prevent panics from crossing +//! FFI boundaries, which would cause undefined behavior. It also provides detailed +//! logging when panics occur to help with debugging. + +use crate::ngx_module::logging::log_error; +use std::panic::{catch_unwind, AssertUnwindSafe}; +use std::sync::Once; + +static PANIC_HOOK_INIT: Once = Once::new(); + +/// Initialize panic hook for FFI safety +/// +/// This sets up a custom panic hook that logs panics to nginx error log +/// instead of stderr. This is important because panics in FFI code can cause +/// undefined behavior if they cross the FFI boundary. +pub fn init_panic_hook() { + PANIC_HOOK_INIT.call_once(|| { + std::panic::set_hook(Box::new(|panic_info| { + log_panic(panic_info); + })); + }); +} + +/// Log panic information to nginx error log +fn log_panic(panic_info: &std::panic::PanicHookInfo) { + let message = if let Some(s) = panic_info.payload().downcast_ref::<&str>() { + format!("Panic: {}", s) + } else if let Some(s) = panic_info.payload().downcast_ref::() { + format!("Panic: {}", s) + } else { + "Panic: unknown panic payload".to_string() + }; + + let location = if let Some(location) = panic_info.location() { + format!( + "{}:{}:{}", + location.file(), + location.line(), + location.column() + ) + } else { + "unknown location".to_string() + }; + + log_error( + None, + &format!("[x402] FFI PANIC DETECTED: {} at {}", message, location), + ); + + // Try to get backtrace if available (requires RUST_BACKTRACE=1) + if std::env::var("RUST_BACKTRACE").is_ok() { + // Backtrace is automatically captured by panic hook when RUST_BACKTRACE is set + // We can't easily access it here, but it will be printed to stderr + log_error( + None, + "[x402] Panic occurred. Set RUST_BACKTRACE=1 for detailed backtrace.", + ); + } +} + +/// Execute a function with panic protection +/// +/// This wrapper catches any panics and converts them to error logs, +/// preventing panics from crossing FFI boundaries. +/// +/// # Arguments +/// +/// * `f` - The function to execute +/// * `context` - Context string for logging (e.g., function name) +/// +/// # Returns +/// +/// * `Some(T)` - Function result if successful +/// * `None` - If panic occurred +pub fn catch_panic(f: F, context: &str) -> Option +where + F: FnOnce() -> T, +{ + match catch_unwind(AssertUnwindSafe(f)) { + Ok(result) => Some(result), + Err(e) => { + let message = if let Some(s) = e.downcast_ref::<&str>() { + format!("Panic in {}: {}", context, s) + } else if let Some(s) = e.downcast_ref::() { + format!("Panic in {}: {}", context, s) + } else { + format!("Panic in {}: unknown panic payload", context) + }; + + log_error(None, &format!("[x402] {}", message)); + + // Log backtrace hint if RUST_BACKTRACE is set + if std::env::var("RUST_BACKTRACE").is_ok() { + log_error( + None, + &format!( + "[x402] Panic in {}. Backtrace available in stderr (RUST_BACKTRACE=1).", + context + ), + ); + } + + None + } + } +} + +/// Execute a function with panic protection and return a default value on panic +/// +/// This is useful for FFI functions that need to return a specific value +/// (like NGX_ERROR) when a panic occurs. +/// +/// # Arguments +/// +/// * `f` - The function to execute +/// * `context` - Context string for logging +/// * `default` - Default value to return if panic occurs +/// +/// # Returns +/// +/// * Function result if successful, or `default` if panic occurred +pub fn catch_panic_or_default(f: F, context: &str, default: T) -> T +where + F: FnOnce() -> T, +{ + catch_panic(f, context).unwrap_or(default) +} + +/// Execute a function with panic protection and detailed error logging +/// +/// This version includes additional context information in the error log. +/// +/// # Arguments +/// +/// * `f` - The function to execute +/// * `context` - Context string for logging +/// * `additional_info` - Additional information to include in logs +/// +/// # Returns +/// +/// * `Some(T)` - Function result if successful +/// * `None` - If panic occurred +pub fn catch_panic_with_info(f: F, context: &str, additional_info: &str) -> Option +where + F: FnOnce() -> T, +{ + match catch_unwind(AssertUnwindSafe(f)) { + Ok(result) => Some(result), + Err(e) => { + let message = if let Some(s) = e.downcast_ref::<&str>() { + format!("Panic in {}: {} | Context: {}", context, s, additional_info) + } else if let Some(s) = e.downcast_ref::() { + format!("Panic in {}: {} | Context: {}", context, s, additional_info) + } else { + format!( + "Panic in {}: unknown panic payload | Context: {}", + context, additional_info + ) + }; + + log_error(None, &format!("[x402] {}", message)); + + // Log backtrace hint if RUST_BACKTRACE is set + if std::env::var("RUST_BACKTRACE").is_ok() { + log_error( + None, + &format!( + "[x402] Panic in {} ({}). Backtrace available in stderr (RUST_BACKTRACE=1).", + context, additional_info + ), + ); + } + + None + } + } +} diff --git a/tests/docker_integration/mod.rs b/tests/docker_integration/mod.rs index 0d247db..fe03ef1 100644 --- a/tests/docker_integration/mod.rs +++ b/tests/docker_integration/mod.rs @@ -58,3 +58,6 @@ pub mod header_passthrough_tests; #[cfg(feature = "integration-test")] pub mod timestamp_tests; + +#[cfg(feature = "integration-test")] +pub mod ttl_config_tests; diff --git a/tests/docker_integration/ttl_config_tests.rs b/tests/docker_integration/ttl_config_tests.rs new file mode 100644 index 0000000..1bb88fd --- /dev/null +++ b/tests/docker_integration/ttl_config_tests.rs @@ -0,0 +1,147 @@ +//! Integration tests for x402_ttl configuration directive +//! +//! These tests verify that x402_ttl configuration works correctly and doesn't cause segfaults. +//! This is critical because TTL configuration was added recently and may have memory safety issues. + +#[cfg(feature = "integration-test")] +mod tests { + use crate::docker_integration::common::*; + use std::process::Command; + use std::thread; + use std::time::Duration; + + /// Test that x402_ttl configuration doesn't cause segfaults + /// + /// This test verifies that: + /// 1. x402_ttl can be configured without causing segfaults + /// 2. Multiple requests with TTL configuration work correctly + /// 3. Worker processes don't crash + #[test] + #[ignore = "requires Docker"] + fn test_ttl_configuration_no_segfault() { + if !ensure_container_running() { + eprintln!("Failed to start container. Skipping test."); + return; + } + + // Make multiple requests to trigger potential segfaults + // The segfault typically occurs during configuration merging or request processing + for i in 0..10 { + let _ = http_request("/api/protected"); + thread::sleep(Duration::from_millis(100)); + + // Check if nginx is still running (if segfault occurred, nginx would restart) + if i % 3 == 0 { + let status = http_request("/health"); + assert!( + status.is_some() && status.unwrap() == "200", + "Nginx should still be running after request {}. If this fails, segfault may have occurred.", + i + ); + } + } + + println!("✓ TTL configuration test completed without segfaults"); + } + + /// Test that x402_ttl with different values works correctly + /// + /// This test verifies that: + /// 1. TTL configuration is parsed correctly + /// 2. Different TTL values don't cause issues + /// 3. Configuration merging works correctly + #[test] + #[ignore = "requires Docker"] + fn test_ttl_configuration_values() { + if !ensure_container_running() { + eprintln!("Failed to start container. Skipping test."); + return; + } + + // Make a request that should trigger TTL usage + let _ = http_request("/api/protected"); + + // Wait a bit for any segfaults to occur + thread::sleep(Duration::from_millis(500)); + + // Check for segfault indicators in logs + let recent_logs = Command::new("docker") + .args(["logs", "--tail", "100", CONTAINER_NAME]) + .output() + .ok() + .and_then(|output| { + let stdout = String::from_utf8_lossy(&output.stdout).to_string(); + Some(stdout) + }) + .unwrap_or_default(); + + // Check for segfault indicators + assert!( + !recent_logs.contains("signal 11") && !recent_logs.contains("core dumped"), + "Segfault detected in logs! Logs: {}", + recent_logs.chars().take(1000).collect::() + ); + + // Verify nginx is still running + let status = http_request("/health"); + assert!( + status.is_some() && status.unwrap() == "200", + "Nginx should still be running after TTL configuration test" + ); + + println!("✓ TTL configuration values test completed"); + } + + /// Test that x402_ttl configuration merging works correctly + /// + /// This test verifies that: + /// 1. TTL configuration merges correctly from parent to child locations + /// 2. Configuration merging doesn't cause segfaults + /// 3. Multiple nested locations work correctly + #[test] + #[ignore = "requires Docker"] + fn test_ttl_configuration_merging() { + if !ensure_container_running() { + eprintln!("Failed to start container. Skipping test."); + return; + } + + // Make requests to different endpoints that may have different TTL configurations + // This tests configuration merging from parent to child locations + let endpoints = vec!["/api/protected", "/api/", "/health"]; + + for endpoint in endpoints { + let _ = http_request(endpoint); + thread::sleep(Duration::from_millis(100)); + } + + // Wait a bit for any segfaults to occur + thread::sleep(Duration::from_millis(500)); + + // Check for segfault indicators + let logs = Command::new("docker") + .args(["logs", "--tail", "100", CONTAINER_NAME]) + .output() + .ok() + .and_then(|output| { + let stdout = String::from_utf8_lossy(&output.stdout).to_string(); + Some(stdout) + }) + .unwrap_or_default(); + + assert!( + !logs.contains("signal 11") && !logs.contains("core dumped"), + "Segfault detected during configuration merging! Logs: {}", + logs.chars().take(1000).collect::() + ); + + // Verify nginx is still running + let status = http_request("/health"); + assert!( + status.is_some() && status.unwrap() == "200", + "Nginx should still be running after configuration merging test" + ); + + println!("✓ TTL configuration merging test completed"); + } +} diff --git a/tests/nginx.test.conf b/tests/nginx.test.conf index d6ff645..afd4d81 100644 --- a/tests/nginx.test.conf +++ b/tests/nginx.test.conf @@ -48,6 +48,7 @@ http { x402_network base-sepolia; x402_facilitator_fallback error; x402_description "Test API access payment"; + x402_ttl 60; # TTL configuration for testing segfault issues } # Protected location with x402 payment AND proxy_pass