Skip to content

WIP: Simple NgxResult type and related refactoring in Pool, Request, and variable handlers. #215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

WIP: Simple NgxResult type and related refactoring in Pool, Request, and variable handlers. #215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e95e2b8
refactor: use NgxResult in handlers
ensh63 Sep 16, 2025
7f6e4e6
refactor: overlook pool methods
ensh63 Sep 18, 2025
28c1fa0
refactor: use Result for handler return type in variable getters and …
ensh63 Sep 22, 2025
8fe93db
feat: request handler trait
ensh63 Sep 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 9 additions & 11 deletions examples/async.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(
Expand Down
10 changes: 5 additions & 5 deletions examples/awssig.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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");
Expand All @@ -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());
Expand Down Expand Up @@ -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()
});
60 changes: 27 additions & 33 deletions examples/curl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@ use std::ffi::{c_char, c_void};

use ngx::core;
use ngx::ffi::{
ngx_array_push, ngx_command_t, ngx_conf_t, ngx_http_handler_pt, ngx_http_module_t,
ngx_http_phases_NGX_HTTP_ACCESS_PHASE, ngx_int_t, ngx_module_t, ngx_str_t, ngx_uint_t,
ngx_command_t, ngx_conf_t, ngx_http_module_t, ngx_module_t, ngx_str_t, ngx_uint_t,
NGX_CONF_TAKE1, NGX_HTTP_LOC_CONF, NGX_HTTP_LOC_CONF_OFFSET, NGX_HTTP_MODULE, NGX_LOG_EMERG,
};
use ngx::http::{self, HttpModule, MergeConfigError};
use ngx::http::{HttpModuleLocationConf, HttpModuleMainConf, NgxHttpCoreModule};
use ngx::{http_request_handler, ngx_conf_log_error, ngx_log_debug_http, ngx_string};
use ngx::http::HttpModuleLocationConf;
use ngx::http::{self, HttpModule, MergeConfigError, RequestHandler};
use ngx::{ngx_conf_log_error, ngx_log_debug_http, ngx_string};

struct Module;

Expand All @@ -17,20 +16,8 @@ impl http::HttpModule for Module {
unsafe { &*::core::ptr::addr_of!(ngx_http_curl_module) }
}

unsafe extern "C" fn postconfiguration(cf: *mut ngx_conf_t) -> ngx_int_t {
// SAFETY: this function is called with non-NULL cf always
let cf = &mut *cf;
let cmcf = NgxHttpCoreModule::main_conf_mut(cf).expect("http core main conf");

let h = ngx_array_push(
&mut cmcf.phases[ngx_http_phases_NGX_HTTP_ACCESS_PHASE as usize].handlers,
) as *mut ngx_http_handler_pt;
if h.is_null() {
return core::Status::NGX_ERROR.into();
}
// set an Access phase handler
*h = Some(curl_access_handler);
core::Status::NGX_OK.into()
fn request_handlers() -> impl Iterator<Item = impl RequestHandler<Module = Self>> {
::core::iter::once(CurlRequestHandler)
}
}

Expand Down Expand Up @@ -90,25 +77,32 @@ impl http::Merge for ModuleConfig {
}
}

http_request_handler!(curl_access_handler, |request: &mut http::Request| {
let co = Module::location_conf(request).expect("module config is none");
struct CurlRequestHandler;

impl RequestHandler for CurlRequestHandler {
const PHASE: nginx_sys::NgxHttpPhases = nginx_sys::NgxHttpPhases::Access;
type Module = Module;

ngx_log_debug_http!(request, "curl module enabled: {}", co.enable);
fn handler(request: &mut http::Request) -> ngx::core::NgxResult {
let co = Module::location_conf(request).expect("module config is none");

match co.enable {
true => {
if request
.user_agent()
.is_some_and(|ua| ua.as_bytes().starts_with(b"curl"))
{
http::HTTPStatus::FORBIDDEN.into()
} else {
core::Status::NGX_DECLINED
ngx_log_debug_http!(request, "curl module enabled: {}", co.enable);

match co.enable {
true => {
if request
.user_agent()
.is_some_and(|ua| ua.as_bytes().starts_with(b"curl"))
{
http::HTTPStatus::FORBIDDEN.into()
} else {
core::Status::NGX_DECLINED.into()
}
}
false => core::Status::NGX_DECLINED.into(),
}
false => core::Status::NGX_DECLINED,
}
});
}

extern "C" fn ngx_http_curl_commands_set_enable(
cf: *mut ngx_conf_t,
Expand Down
95 changes: 37 additions & 58 deletions examples/httporigdst.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use std::ffi::{c_int, c_void};
use std::ptr::addr_of;
use std::ffi::c_int;

use ngx::core;
use ngx::ffi::{
Expand All @@ -19,57 +18,39 @@ struct NgxHttpOrigDstCtx {
}

impl NgxHttpOrigDstCtx {
pub fn save(&mut self, addr: &str, port: in_port_t, pool: &core::Pool) -> core::Status {
let addr_data = pool.alloc_unaligned(addr.len());
if addr_data.is_null() {
return core::Status::NGX_ERROR;
}
unsafe { libc::memcpy(addr_data, addr.as_ptr() as *const c_void, addr.len()) };
self.orig_dst_addr.len = addr.len();
self.orig_dst_addr.data = addr_data as *mut u8;
pub fn save(&mut self, addr: &str, port: in_port_t, pool: &core::Pool) -> core::NgxResult {
self.orig_dst_addr = unsafe { ngx_str_t::from_str(pool.as_ptr(), addr) };

let port_str = port.to_string();
let port_data = pool.alloc_unaligned(port_str.len());
if port_data.is_null() {
return core::Status::NGX_ERROR;
}
unsafe {
libc::memcpy(
port_data,
port_str.as_bytes().as_ptr() as *const c_void,
port_str.len(),
)
};
self.orig_dst_port.len = port_str.len();
self.orig_dst_port.data = port_data as *mut u8;
self.orig_dst_port = unsafe { ngx_str_t::from_str(pool.as_ptr(), &port_str) };
Comment on lines +22 to +25
Copy link
Preview

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pool.as_ptr() method call suggests accessing the raw pointer from a Pool, but this may not be the intended API usage. Consider using the Pool's proper allocation methods instead of accessing the raw pointer directly.

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This correction makes sense, but this is a possible future improvement for string module.


core::Status::NGX_OK
Ok(core::Status::NGX_OK.into())
}

pub unsafe fn bind_addr(&self, v: *mut ngx_variable_value_t) {
pub unsafe fn bind_addr(&self, v: &mut ngx_variable_value_t) {
if self.orig_dst_addr.len == 0 {
(*v).set_not_found(1);
v.set_not_found(1);
return;
}

(*v).set_valid(1);
(*v).set_no_cacheable(0);
(*v).set_not_found(0);
(*v).set_len(self.orig_dst_addr.len as u32);
(*v).data = self.orig_dst_addr.data;
v.set_valid(1);
v.set_no_cacheable(0);
v.set_not_found(0);
v.set_len(self.orig_dst_addr.len as u32);
v.data = self.orig_dst_addr.data;
}

pub unsafe fn bind_port(&self, v: *mut ngx_variable_value_t) {
pub unsafe fn bind_port(&self, v: &mut ngx_variable_value_t) {
if self.orig_dst_port.len == 0 {
(*v).set_not_found(1);
v.set_not_found(1);
return;
}

(*v).set_valid(1);
(*v).set_no_cacheable(0);
(*v).set_not_found(0);
(*v).set_len(self.orig_dst_port.len as u32);
(*v).data = self.orig_dst_port.data;
v.set_valid(1);
v.set_no_cacheable(0);
v.set_not_found(0);
v.set_len(self.orig_dst_port.len as u32);
v.data = self.orig_dst_port.data;
}
}

Expand Down Expand Up @@ -191,12 +172,12 @@ unsafe fn ngx_get_origdst(

http_variable_get!(
ngx_http_orig_dst_addr_variable,
|request: &mut http::Request, v: *mut ngx_variable_value_t, _: usize| {
let ctx = request.get_module_ctx::<NgxHttpOrigDstCtx>(&*addr_of!(ngx_http_orig_dst_module));
|request: &mut http::Request, v: &mut ngx_variable_value_t, _: usize| {
let ctx = request.get_module_ctx::<NgxHttpOrigDstCtx>(Module::module());
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
Expand All @@ -207,17 +188,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!(
Expand All @@ -226,24 +207,23 @@ http_variable_get!(
ip,
port,
);
(*new_ctx).save(&ip, port, &request.pool());
(*new_ctx).save(&ip, port, &request.pool())?;
(*new_ctx).bind_addr(v);
request
.set_module_ctx(new_ctx as *mut c_void, &*addr_of!(ngx_http_orig_dst_module));
request.set_module_ctx(new_ctx as _, Module::module());
}
}
core::Status::NGX_OK
core::Status::NGX_OK.into()
}
);

http_variable_get!(
ngx_http_orig_dst_port_variable,
|request: &mut http::Request, v: *mut ngx_variable_value_t, _: usize| {
let ctx = request.get_module_ctx::<NgxHttpOrigDstCtx>(&*addr_of!(ngx_http_orig_dst_module));
|request: &mut http::Request, v: &mut ngx_variable_value_t, _: usize| {
let ctx = request.get_module_ctx::<NgxHttpOrigDstCtx>(Module::module());
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
Expand All @@ -254,17 +234,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!(
Expand All @@ -273,13 +253,12 @@ http_variable_get!(
ip,
port,
);
(*new_ctx).save(&ip, port, &request.pool());
(*new_ctx).save(&ip, port, &request.pool())?;
(*new_ctx).bind_port(v);
request
.set_module_ctx(new_ctx as *mut c_void, &*addr_of!(ngx_http_orig_dst_module));
request.set_module_ctx(new_ctx as _, Module::module());
}
}
core::Status::NGX_OK
core::Status::NGX_OK.into()
}
);

Expand Down
Loading
Loading