refactor: use NgxResult in handlers

ensh63 · ensh63 · commit e95e2b85492a · 2025-10-01T14:32:56.000-07:00
diff --git a/examples/async.rs b/examples/async.rs
@@ -1,5 +1,5 @@
 use std::ffi::{c_char, c_void};
-use std::ptr::{addr_of, addr_of_mut};
+use std::ptr::addr_of_mut;
 use std::sync::atomic::{AtomicBool, AtomicPtr, Ordering};
 use std::sync::{Arc, OnceLock};
 use std::time::Instant;
@@ -145,24 +145,22 @@ http_request_handler!(async_access_handler, |request: &mut http::Request| {
     ngx_log_debug_http!(request, "async module enabled: {}", co.enable);
 
     if !co.enable {
-        return core::Status::NGX_DECLINED;
+        return core::Status::NGX_DECLINED.into();
     }
 
-    if let Some(ctx) =
-        unsafe { request.get_module_ctx::<RequestCTX>(&*addr_of!(ngx_http_async_module)) }
-    {
+    if let Some(ctx) = request.get_module_ctx::<RequestCTX>(Module::module()) {
         if !ctx.done.load(Ordering::Relaxed) {
-            return core::Status::NGX_AGAIN;
+            return core::Status::NGX_AGAIN.into();
         }
 
-        return core::Status::NGX_OK;
+        return core::Status::NGX_OK.into();
     }
 
-    let ctx = request.pool().allocate(RequestCTX::default());
+    let ctx = request.pool().alloc_with_cleanup(RequestCTX::default());
     if ctx.is_null() {
-        return core::Status::NGX_ERROR;
+        return core::Status::NGX_ERROR.into();
     }
-    request.set_module_ctx(ctx.cast(), unsafe { &*addr_of!(ngx_http_async_module) });
+    request.set_module_ctx(ctx.cast(), Module::module());
 
     let ctx = unsafe { &mut *ctx };
     ctx.event.handler = Some(check_async_work_done);
@@ -194,7 +192,7 @@ http_request_handler!(async_access_handler, |request: &mut http::Request| {
         // and use the same trick as the thread pool)
     }));
 
-    core::Status::NGX_AGAIN
+    core::Status::NGX_AGAIN.into()
 });
 
 extern "C" fn ngx_http_async_commands_set_enable(
diff --git a/examples/awssig.rs b/examples/awssig.rs
@@ -272,7 +272,7 @@ http_request_handler!(awssigv4_header_handler, |request: &mut Request| {
         }
     });
     if !conf.enable {
-        return core::Status::NGX_DECLINED;
+        return core::Status::NGX_DECLINED.into();
     }
 
     // TODO: build url properly from the original URL from client
@@ -284,7 +284,7 @@ http_request_handler!(awssigv4_header_handler, |request: &mut Request| {
     let datetime = chrono::Utc::now();
     let uri = match request.unparsed_uri().to_str() {
         Ok(v) => format!("https://{}.{}{}", conf.s3_bucket, conf.s3_endpoint, v),
-        Err(_) => return core::Status::NGX_DECLINED,
+        Err(_) => return core::Status::NGX_DECLINED.into(),
     };
 
     let datetime_now = datetime.format("%Y%m%dT%H%M%SZ");
@@ -301,11 +301,11 @@ http_request_handler!(awssigv4_header_handler, |request: &mut Request| {
                     if let Ok(value) = http::HeaderValue::from_bytes(value.as_bytes()) {
                         headers.insert(http::header::HOST, value);
                     } else {
-                        return core::Status::NGX_DECLINED;
+                        return core::Status::NGX_DECLINED.into();
                     }
                 }
             } else {
-                return core::Status::NGX_DECLINED;
+                return core::Status::NGX_DECLINED.into();
             }
         }
         headers.insert("X-Amz-Date", datetime_now.parse().unwrap());
@@ -338,5 +338,5 @@ http_request_handler!(awssigv4_header_handler, |request: &mut Request| {
         ngx_log_debug_http!(request, "headers_in  {name}: {value}",);
     }
 
-    core::Status::NGX_OK
+    core::Status::NGX_OK.into()
 });
diff --git a/examples/curl.rs b/examples/curl.rs
@@ -103,10 +103,10 @@ http_request_handler!(curl_access_handler, |request: &mut http::Request| {
             {
                 http::HTTPStatus::FORBIDDEN.into()
             } else {
-                core::Status::NGX_DECLINED
+                core::Status::NGX_DECLINED.into()
             }
         }
-        false => core::Status::NGX_DECLINED,
+        false => core::Status::NGX_DECLINED.into(),
     }
 });
 
diff --git a/examples/httporigdst.rs b/examples/httporigdst.rs
@@ -196,7 +196,7 @@ http_variable_get!(
         if let Some(obj) = ctx {
             ngx_log_debug_http!(request, "httporigdst: found context and binding variable",);
             obj.bind_addr(v);
-            return core::Status::NGX_OK;
+            return core::Status::NGX_OK.into();
         }
         // lazy initialization:
         //   get original dest information
@@ -207,17 +207,17 @@ http_variable_get!(
         let r = ngx_get_origdst(request);
         match r {
             Err(e) => {
-                return e;
+                return e.into();
             }
             Ok((ip, port)) => {
                 // create context,
                 // set context
                 let new_ctx = request
                     .pool()
-                    .allocate::<NgxHttpOrigDstCtx>(Default::default());
+                    .alloc_with_cleanup::<NgxHttpOrigDstCtx>(Default::default());
 
                 if new_ctx.is_null() {
-                    return core::Status::NGX_ERROR;
+                    return core::Status::NGX_ERROR.into();
                 }
 
                 ngx_log_debug_http!(
@@ -232,7 +232,7 @@ http_variable_get!(
                     .set_module_ctx(new_ctx as *mut c_void, &*addr_of!(ngx_http_orig_dst_module));
             }
         }
-        core::Status::NGX_OK
+        core::Status::NGX_OK.into()
     }
 );
 
@@ -243,7 +243,7 @@ http_variable_get!(
         if let Some(obj) = ctx {
             ngx_log_debug_http!(request, "httporigdst: found context and binding variable",);
             obj.bind_port(v);
-            return core::Status::NGX_OK;
+            return core::Status::NGX_OK.into();
         }
         // lazy initialization:
         //   get original dest information
@@ -254,17 +254,17 @@ http_variable_get!(
         let r = ngx_get_origdst(request);
         match r {
             Err(e) => {
-                return e;
+                return e.into();
             }
             Ok((ip, port)) => {
                 // create context,
                 // set context
                 let new_ctx = request
                     .pool()
-                    .allocate::<NgxHttpOrigDstCtx>(Default::default());
+                    .alloc_with_cleanup::<NgxHttpOrigDstCtx>(Default::default());
 
                 if new_ctx.is_null() {
-                    return core::Status::NGX_ERROR;
+                    return core::Status::NGX_ERROR.into();
                 }
 
                 ngx_log_debug_http!(
@@ -279,7 +279,7 @@ http_variable_get!(
                     .set_module_ctx(new_ctx as *mut c_void, &*addr_of!(ngx_http_orig_dst_module));
             }
         }
-        core::Status::NGX_OK
+        core::Status::NGX_OK.into()
     }
 );
 
diff --git a/src/core/pool.rs b/src/core/pool.rs
@@ -272,7 +272,7 @@ impl Pool {
     ///
     /// Returns a typed pointer to the allocated memory if successful, or a null pointer if
     /// allocation or cleanup handler addition fails.
-    pub fn allocate<T>(&self, value: T) -> *mut T {
+    pub fn alloc_with_cleanup<T>(&self, value: T) -> *mut T {
         unsafe {
             let p = self.alloc(mem::size_of::<T>()) as *mut T;
             ptr::write(p, value);
@@ -308,7 +308,7 @@ impl Pool {
             Ok(NonNull::slice_from_raw_parts(ptr, new_layout.size()))
         } else {
             let size = core::cmp::min(old_layout.size(), new_layout.size());
-            let new_ptr = <Self as Allocator>::allocate(self, new_layout)?;
+            let new_ptr = self.allocate(new_layout)?;
             unsafe {
                 ptr.copy_to_nonoverlapping(new_ptr.cast(), size);
                 self.deallocate(ptr, old_layout);
diff --git a/src/core/status.rs b/src/core/status.rs
@@ -1,7 +1,10 @@
+use core::error::Error;
 use core::ffi::c_char;
 use core::fmt;
 use core::ptr;
 
+use allocator_api2::alloc::AllocError;
+
 use crate::ffi::*;
 
 /// Status
@@ -68,3 +71,34 @@ ngx_codes! {
 pub const NGX_CONF_ERROR: *mut c_char = ptr::null_mut::<c_char>().wrapping_offset(-1);
 /// Configuration handler succeeded.
 pub const NGX_CONF_OK: *mut c_char = ptr::null_mut();
+
+/// Error type encapsulating normal NGINX return codes
+#[derive(Debug)]
+pub struct NgxError {}
+
+impl fmt::Display for NgxError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "NGINX Error")
+    }
+}
+
+impl Error for NgxError {}
+
+impl From<AllocError> for NgxError {
+    fn from(_err: AllocError) -> Self {
+        NgxError {}
+    }
+}
+
+/// Result type for NGINX status codes.
+pub type NgxResult<T = nginx_sys::ngx_int_t> = core::result::Result<T, NgxError>;
+
+impl From<Status> for NgxResult {
+    fn from(val: Status) -> Self {
+        if val == Status::NGX_ERROR {
+            Err(NgxError {})
+        } else {
+            Ok(val.0)
+        }
+    }
+}
diff --git a/src/http/module.rs b/src/http/module.rs
@@ -78,7 +78,7 @@ pub trait HttpModule {
         Self::MainConf: Default,
     {
         let pool = Pool::from_ngx_pool((*cf).pool);
-        pool.allocate::<Self::MainConf>(Default::default()) as *mut c_void
+        pool.alloc_with_cleanup::<Self::MainConf>(Default::default()) as *mut c_void
     }
 
     /// # Safety
@@ -103,7 +103,7 @@ pub trait HttpModule {
         Self::ServerConf: Default,
     {
         let pool = Pool::from_ngx_pool((*cf).pool);
-        pool.allocate::<Self::ServerConf>(Default::default()) as *mut c_void
+        pool.alloc_with_cleanup::<Self::ServerConf>(Default::default()) as *mut c_void
     }
 
     /// # Safety
@@ -137,7 +137,7 @@ pub trait HttpModule {
         Self::LocationConf: Default,
     {
         let pool = Pool::from_ngx_pool((*cf).pool);
-        pool.allocate::<Self::LocationConf>(Default::default()) as *mut c_void
+        pool.alloc_with_cleanup::<Self::LocationConf>(Default::default()) as *mut c_void
     }
 
     /// # Safety
diff --git a/src/http/request.rs b/src/http/request.rs
diff --git a/src/http/status.rs b/src/http/status.rs

Original file line number	Diff line number	Diff line change
`@@ -272,7 +272,7 @@ http_request_handler!(awssigv4_header_handler, \|request: &mut Request\| {`
`272`	`272`	`}`
`273`	`273`	`});`
`274`	`274`	`if !conf.enable {`
`275`		`- return core::Status::NGX_DECLINED;`
	`275`	`+ return core::Status::NGX_DECLINED.into();`
`276`	`276`	`}`
`277`	`277`
`278`	`278`	`// TODO: build url properly from the original URL from client`
`@@ -284,7 +284,7 @@ http_request_handler!(awssigv4_header_handler, \|request: &mut Request\| {`
`284`	`284`	`let datetime = chrono::Utc::now();`
`285`	`285`	`let uri = match request.unparsed_uri().to_str() {`
`286`	`286`	`Ok(v) => format!("https://{}.{}{}", conf.s3_bucket, conf.s3_endpoint, v),`
`287`		`- Err(_) => return core::Status::NGX_DECLINED,`
	`287`	`+ Err(_) => return core::Status::NGX_DECLINED.into(),`
`288`	`288`	`};`
`289`	`289`
`290`	`290`	`let datetime_now = datetime.format("%Y%m%dT%H%M%SZ");`
`@@ -301,11 +301,11 @@ http_request_handler!(awssigv4_header_handler, \|request: &mut Request\| {`
`301`	`301`	`if let Ok(value) = http::HeaderValue::from_bytes(value.as_bytes()) {`
`302`	`302`	`headers.insert(http::header::HOST, value);`
`303`	`303`	`} else {`
`304`		`- return core::Status::NGX_DECLINED;`
	`304`	`+ return core::Status::NGX_DECLINED.into();`
`305`	`305`	`}`
`306`	`306`	`}`
`307`	`307`	`} else {`
`308`		`- return core::Status::NGX_DECLINED;`
	`308`	`+ return core::Status::NGX_DECLINED.into();`
`309`	`309`	`}`
`310`	`310`	`}`
`311`	`311`	`headers.insert("X-Amz-Date", datetime_now.parse().unwrap());`
`@@ -338,5 +338,5 @@ http_request_handler!(awssigv4_header_handler, \|request: &mut Request\| {`
`338`	`338`	`ngx_log_debug_http!(request, "headers_in {name}: {value}",);`
`339`	`339`	`}`
`340`	`340`
`341`		`- core::Status::NGX_OK`
	`341`	`+ core::Status::NGX_OK.into()`
`342`	`342`	`});`
Original file line number	Diff line number	Diff line change
`@@ -103,10 +103,10 @@ http_request_handler!(curl_access_handler, \|request: &mut http::Request\| {`
`103`	`103`	`{`
`104`	`104`	`http::HTTPStatus::FORBIDDEN.into()`
`105`	`105`	`} else {`
`106`		`- core::Status::NGX_DECLINED`
	`106`	`+ core::Status::NGX_DECLINED.into()`
`107`	`107`	`}`
`108`	`108`	`}`
`109`		`- false => core::Status::NGX_DECLINED,`
	`109`	`+ false => core::Status::NGX_DECLINED.into(),`
`110`	`110`	`}`
`111`	`111`	`});`
`112`	`112`
Original file line number	Diff line number	Diff line change
`@@ -78,7 +78,7 @@ pub trait HttpModule {`
`78`	`78`	`Self::MainConf: Default,`
`79`	`79`	`{`
`80`	`80`	`let pool = Pool::from_ngx_pool((*cf).pool);`
`81`		`- pool.allocate::<Self::MainConf>(Default::default()) as *mut c_void`
	`81`	`+ pool.alloc_with_cleanup::<Self::MainConf>(Default::default()) as *mut c_void`
`82`	`82`	`}`
`83`	`83`
`84`	`84`	`/// # Safety`
`@@ -103,7 +103,7 @@ pub trait HttpModule {`
`103`	`103`	`Self::ServerConf: Default,`
`104`	`104`	`{`
`105`	`105`	`let pool = Pool::from_ngx_pool((*cf).pool);`
`106`		`- pool.allocate::<Self::ServerConf>(Default::default()) as *mut c_void`
	`106`	`+ pool.alloc_with_cleanup::<Self::ServerConf>(Default::default()) as *mut c_void`
`107`	`107`	`}`
`108`	`108`
`109`	`109`	`/// # Safety`
`@@ -137,7 +137,7 @@ pub trait HttpModule {`
`137`	`137`	`Self::LocationConf: Default,`
`138`	`138`	`{`
`139`	`139`	`let pool = Pool::from_ngx_pool((*cf).pool);`
`140`		`- pool.allocate::<Self::LocationConf>(Default::default()) as *mut c_void`
	`140`	`+ pool.alloc_with_cleanup::<Self::LocationConf>(Default::default()) as *mut c_void`
`141`	`141`	`}`
`142`	`142`
`143`	`143`	`/// # Safety`