Fix/ttl segfault issue

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<ngx_str_t> {
+///
+/// # 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<ngx_str_t> {
     if src.len == 0 {
         return Some(ngx_str_t {
             len: 0,

src/ngx_module/commands/mod.rs

@@ -24,7 +24,7 @@
 
 mod asset;
 mod basic;
-mod common;
+pub mod common;
 mod network;
 mod other;
 

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");

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::<X402Config>();
     let conf = conf.cast::<X402Config>();
 
@@ -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

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::<String>() {
+        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, T>(f: F, context: &str) -> Option<T>
+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::<String>() {
+                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, T>(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, T>(f: F, context: &str, additional_info: &str) -> Option<T>
+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::<String>() {
+                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
+        }
+    }
+}

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;