Fix/segfault after ttl config

src/ngx_module/config.rs

@@ -8,6 +8,15 @@ use std::str::FromStr;
 use std::time::Duration;
 
 /// Module configuration (raw strings from Nginx config)
+///
+/// # Memory Layout
+///
+/// This struct must use C-compatible memory layout (`#[repr(C)]`) because:
+/// 1. It's allocated by nginx using `ngx_pcalloc` (C memory allocator)
+/// 2. It's accessed via raw pointers from nginx's configuration system
+/// 3. Field order and padding must match what nginx expects
+/// 4. Adding new fields (like `ttl_str`) must not break existing memory layout
+#[repr(C)]
 #[derive(Clone, Default)]
 pub struct X402Config {
     pub enabled: ngx::ffi::ngx_flag_t,

src/ngx_module/mod.rs

@@ -101,18 +101,21 @@ pub unsafe extern "C" fn x402_metrics_handler(
 /// # Safety
 ///
 /// The caller must ensure that `r` is a valid pointer to a `ngx_http_request_t`.
-unsafe fn clear_x402_content_handler(r: *mut ngx::ffi::ngx_http_request_t, reason: &str) {
-    use ngx::ffi::ngx_http_request_t;
-    let r_raw = r.cast::<ngx_http_request_t>();
-    if r_raw.is_null() {
-        return;
-    }
+/// Clear x402 content handler if it's set
+///
+/// Uses ngx-rust's safe `Request` API instead of raw pointers.
+fn clear_x402_content_handler(req: &mut ngx::http::Request, reason: &str) {
+    use crate::ngx_module::logging::log_debug;
 
     extern "C" {
         fn x402_ngx_handler(r: *mut ngx::ffi::ngx_http_request_t) -> ngx::ffi::ngx_int_t;
     }
+
+    // Use Request's as_mut() to access the underlying structure
+    // Safe: Request is a zero-cost wrapper, as_mut() returns a valid mutable reference
+    let r_raw = req.as_mut();
     let x402_handler_fn: ngx::ffi::ngx_http_handler_pt = Some(x402_ngx_handler);
-    let current_handler = (*r_raw).content_handler;
+    let current_handler = r_raw.content_handler;
 
     // Check if content handler is x402_ngx_handler
     let is_x402_handler = if let (Some(current), Some(x402)) = (current_handler, x402_handler_fn) {
@@ -123,8 +126,7 @@ unsafe fn clear_x402_content_handler(r: *mut ngx::ffi::ngx_http_request_t, reaso
 
     if is_x402_handler {
         // Clear content handler to prevent payment verification in CONTENT_PHASE
-        (*r_raw).content_handler = None;
-        use crate::ngx_module::logging::log_debug;
+        r_raw.content_handler = None;
         log_debug(
             None,
             &format!(
@@ -167,13 +169,12 @@ pub unsafe extern "C" fn x402_phase_handler(
         || {
             // 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
+            // Request implements DerefMut, so we can pass req_mut directly to functions expecting &mut Request
             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,
-            };
+            use crate::ngx_module::request::is_websocket_request;
 
             // Skip payment verification for special request types
             // These requests should bypass payment verification:
@@ -190,9 +191,9 @@ pub unsafe extern "C" fn x402_phase_handler(
 
             // 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) };
+            // Use safe Request API instead of raw pointer access
+            let method_id = crate::ngx_module::request::get_http_method_id(req_mut);
+            let detected_method = crate::ngx_module::request::get_http_method(req_mut);
 
             log_debug(
                 Some(req_mut),
@@ -202,7 +203,7 @@ pub unsafe extern "C" fn x402_phase_handler(
                 ),
             );
 
-            if unsafe { should_skip_payment_for_method(r) } {
+            if crate::ngx_module::request::should_skip_payment_for_method(req_mut) {
                 let method = detected_method.unwrap_or("UNKNOWN");
                 log_debug(
                     Some(req_mut),
@@ -214,12 +215,10 @@ pub unsafe extern "C" fn x402_phase_handler(
                 // 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),
-                    );
-                }
+                clear_x402_content_handler(
+                    req_mut,
+                    &format!("for {} request to prevent payment verification", method),
+                );
                 return ngx::ffi::NGX_DECLINED as ngx::ffi::ngx_int_t;
             }
 
@@ -230,63 +229,57 @@ pub unsafe extern "C" fn x402_phase_handler(
                     "[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",
-                    );
-                }
+                clear_x402_content_handler(
+                    req_mut,
+                    "for WebSocket request to prevent payment verification",
+                );
                 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() {
-                        log_debug(
-                        Some(req_mut),
-                        "[x402] Phase handler: Subrequest detected (parent != NULL), skipping payment verification",
-                    );
-                        return ngx::ffi::NGX_DECLINED as ngx::ffi::ngx_int_t;
+            // Use panic protection to catch any invalid memory access
+            use crate::ngx_module::panic_handler::catch_panic;
+            let is_subrequest = catch_panic(
+                || {
+                    unsafe {
+                        let r_raw = r.cast_const();
+                        if r_raw.is_null() {
+                            return false;
+                        }
+                        // Access parent field - this may cause segfault if memory is invalid
+                        let parent = (*r_raw).parent;
+                        !parent.is_null()
                     }
-                }
+                },
+                "check subrequest (parent field)",
+            )
+            .unwrap_or(false);
+
+            if is_subrequest {
+                log_debug(
+                    Some(req_mut),
+                    "[x402] Phase handler: Subrequest detected (parent != NULL), skipping 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(
-                                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 for internal redirect using safe Request API
+            // Internal redirects in nginx are typically named locations (starting with @)
+            // We can detect this by checking the request path using Request::path()
+            // This is safer than accessing raw C structure fields
+            if let Ok(path) = req_mut.path().to_str() {
+                if path.starts_with('@') {
+                    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 for this location
+            #[allow(clippy::question_mark)] // Need to return NGX_DECLINED on error, can't use ?
             let conf = match get_module_config(req_mut) {
                 Ok(c) => c,
                 Err(_) => {
@@ -310,12 +303,10 @@ pub unsafe extern "C" fn x402_phase_handler(
                     // 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",
-                        );
-                    }
+                    clear_x402_content_handler(
+                        req_mut,
+                        "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
                 }