diff --git a/Cargo.lock b/Cargo.lock index bda7430..66375c1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -314,7 +314,7 @@ dependencies = [ [[package]] name = "nginx_module" -version = "0.1.5" +version = "0.2.0" dependencies = [ "anyhow", "bindgen", diff --git a/nginx_derive/src/lib.rs b/nginx_derive/src/lib.rs index 8ae2e13..05931bf 100644 --- a/nginx_derive/src/lib.rs +++ b/nginx_derive/src/lib.rs @@ -93,7 +93,11 @@ fn impl_derive_config(input: DeriveInput) -> proc_macro2::TokenStream { pub const COMMANDS_COUNT: usize = #field_count; pub unsafe extern "C" fn create(conf: *mut ::nginx_module::ngx_conf_t) -> *mut std::ffi::c_void { - let pool = ::nginx_module::Pool::from_raw((*conf).pool); + let Some(pool) = ::nginx_module::Pool::from_raw((*conf).pool) else { + let log = ::nginx_module::Log::new((*conf).log); + log.error("Null pool on create config".to_string()); + return std::ptr::null_mut() + }; let config = pool.alloc::<#struct_name>(); match config { Err(e) => { diff --git a/nginx_module/Cargo.toml b/nginx_module/Cargo.toml index 95ec386..7eabe64 100644 --- a/nginx_module/Cargo.toml +++ b/nginx_module/Cargo.toml @@ -8,7 +8,7 @@ name = "nginx_module" readme = "README.md" repository = "https://github.com/g-Core/nginx-rust" - version = "0.1.6" + version = "0.2.0" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html diff --git a/nginx_module/src/complex_value.rs b/nginx_module/src/complex_value.rs index da786f1..1442c39 100644 --- a/nginx_module/src/complex_value.rs +++ b/nginx_module/src/complex_value.rs @@ -7,12 +7,8 @@ use std::{marker::PhantomData, mem::MaybeUninit}; use super::{HttpRequest, NgxConfig, NgxStr}; use crate::{ bindings::{ - ngx_http_compile_complex_value, - ngx_http_compile_complex_value_t, - ngx_http_complex_value, - ngx_http_complex_value_t, - ngx_palloc, - NGX_OK, + ngx_http_compile_complex_value, ngx_http_compile_complex_value_t, ngx_http_complex_value, + ngx_http_complex_value_t, ngx_palloc, NGX_OK, }, ConfigValue, }; @@ -49,7 +45,13 @@ impl<'a> ConfigValue<'a> for ComplexValue<'a> { compiler.value = value.as_mut_ptr_unsafe(); let complex_value: *mut ngx_http_complex_value_t = ngx_palloc( - conf.pool().inner(), + conf.pool() + .ok_or_else(|| { + anyhow::anyhow!( + "ngx_conf_t.pool is null while compiling ngx_http_complex_value" + ) + })? + .inner(), std::mem::size_of::(), ) .cast(); diff --git a/nginx_module/src/http_request.rs b/nginx_module/src/http_request.rs index 3d54a02..973960d 100644 --- a/nginx_module/src/http_request.rs +++ b/nginx_module/src/http_request.rs @@ -50,7 +50,6 @@ pub struct HeadersIter<'a> { _phantom: PhantomData<&'a ()>, } - impl<'a, Ctx: Default> HttpRequestAndContext<'a, Ctx> { /// /// # Safety @@ -69,7 +68,11 @@ impl<'a, Ctx: Default> HttpRequestAndContext<'a, Ctx> { let req = &mut *(&mut self.0 as *mut ngx_http_request_t as *mut HttpRequest); let ptr = self.0.ctx.add(module.ctx_index); let ctx = if (*ptr).is_null() { - let pool = Pool::from_raw(self.0.pool); + let Some(pool) = Pool::from_raw(self.0.pool) else { + return Err(anyhow::anyhow!( + "Request pool is null when splitting request" + )); + }; let ctx = pool.alloc()?; *ptr = (ctx as *mut Ctx).cast(); @@ -164,8 +167,8 @@ impl<'a> HttpRequest<'a> { self.0.internal() != 0 } - pub fn pool(&self) -> &'a Pool { - unsafe { Pool::from_raw(self.0.pool) } + pub fn pool(&self) -> Option<&'a Pool> { + unsafe { Pool::from_raw_ref(self.0.pool) } } pub(crate) unsafe fn ptr_mut(&self) -> *mut ngx_http_request_t { @@ -258,7 +261,7 @@ impl<'a> HttpRequest<'a> { } } - pub fn set_path(&mut self, data: NgxStr<'a>) { + pub fn set_path(&mut self, data: NgxStr<'a>) { self.0.unparsed_uri = data.inner(); } @@ -560,7 +563,9 @@ impl<'a> HeaderList<'a> { pub fn set(&mut self, name: NgxStr<'a>, value: NgxStr<'a>) { let mut part_opt = Some(&mut self.0.part); while let Some(part) = part_opt { - let elems = unsafe { std::slice::from_raw_parts_mut(part.elts as *mut ngx_table_elt_t, part.nelts) }; + let elems = unsafe { + std::slice::from_raw_parts_mut(part.elts as *mut ngx_table_elt_t, part.nelts) + }; for elem in elems { if unsafe { NgxStr::from_raw(elem.key) } == name { elem.value = value.inner(); @@ -570,15 +575,18 @@ impl<'a> HeaderList<'a> { } } - pub fn add(&mut self, name: NgxStr<'a>, value: NgxStr<'a>) -> anyhow::Result<()> { - let h = unsafe { (ngx_list_push(&mut self.0) as *mut ngx_table_elt_t).as_mut().ok_or_else(|| anyhow::anyhow!("Cannot push to header list"))? }; + let h = unsafe { + (ngx_list_push(&mut self.0) as *mut ngx_table_elt_t) + .as_mut() + .ok_or_else(|| anyhow::anyhow!("Cannot push to header list"))? + }; h.hash = 1; h.key = name.inner(); h.value = value.inner(); h.lowcase_key = name.inner().data; h.next = std::ptr::null_mut(); - Ok(()) + Ok(()) } pub fn remove(&mut self, name: NgxStr<'a>) { @@ -622,7 +630,6 @@ impl<'a> HeadersOut<'a> { _phantom: PhantomData, } } - } impl<'a> Iterator for HeadersIter<'a> { @@ -655,5 +662,3 @@ impl<'a> Iterator for HeadersIter<'a> { } } } - - diff --git a/nginx_module/src/pool.rs b/nginx_module/src/pool.rs index 4456e0f..5f88634 100644 --- a/nginx_module/src/pool.rs +++ b/nginx_module/src/pool.rs @@ -9,14 +9,49 @@ use crate::bindings::{ngx_palloc, ngx_pool_cleanup_add, ngx_pool_t}; pub struct Pool(ngx_pool_t); impl Pool { + /// + /// Constructs a mutable reference to `Self` from a raw pointer to `ngx_pool_t`. /// /// # Safety - /// - /// `ptr` should be a valid ngx_pool_t pointer - /// Also unsafe as it can assign an arbitrary lifetime. + /// This function is `unsafe` because it performs a raw pointer cast and dereference, + /// which can lead to undefined behavior if the pointer is invalid, null, or not aligned correctly + /// for the type `Self`. + /// + /// # Parameters + /// - `ptr`: A raw mutable pointer to `ngx_pool_t` that is expected to point to memory + /// compatible with type `Self`. + /// + /// # Returns + /// - `Option<&'a mut Self>`: + /// - Returns `Some(&'a mut Self)` if `ptr` is not null and valid to dereference. + /// - Returns `None` if `ptr` is null. + /// + pub unsafe fn from_raw<'a>(ptr: *mut ngx_pool_t) -> Option<&'a mut Self> { + (ptr as *mut Self).as_mut() + } + + /// + /// Converts a raw mutable pointer of type `*mut ngx_pool_t` into an immutable reference of type `&Self`. + /// + /// # Safety + /// + /// This function is `unsafe` because it operates on a raw pointer and assumes that: + /// - The pointer points to a valid instance of the type `Self`. + /// - The lifetime `'a` must accurately represent the validity period of the referenced data. + /// + /// Failing to uphold these invariants could lead to undefined behavior. + /// + /// # Parameters + /// + /// - `ptr`: A raw mutable pointer of type `*mut ngx_pool_t` that will be cast to a pointer of type `*mut Self`. + /// + /// # Returns + /// + /// - `Some(&'a Self)`: If the pointer is not null, the function returns a reference to `Self`. + /// - `None`: If the pointer is null, the function returns `None`. /// - pub unsafe fn from_raw<'a>(ptr: *mut ngx_pool_t) -> &'a mut Self { - &mut *(ptr as *mut Self) + pub unsafe fn from_raw_ref<'a>(ptr: *mut ngx_pool_t) -> Option<&'a Self> { + (ptr as *mut Self).as_ref() } pub fn alloc(&self) -> anyhow::Result<&mut T> { diff --git a/nginx_module/src/var.rs b/nginx_module/src/var.rs index 9299725..2131b5c 100644 --- a/nginx_module/src/var.rs +++ b/nginx_module/src/var.rs @@ -60,8 +60,10 @@ impl NginxVar { var_value.set_len(value.as_bytes().len() as u32); var_value.data = value.as_bytes().as_ptr().cast_mut(); s(req.ptr_mut(), &mut var_value, self.0.data); - } else if let Ok(value) = NgxStr::with_pool(req.pool(), value.as_bytes()) { - req.set_indexed_var(IndexedVar(self.0.index as isize), value); + } else if let Some(pool) = req.pool() { + if let Ok(value) = NgxStr::with_pool(pool, value.as_bytes()) { + req.set_indexed_var(IndexedVar(self.0.index as isize), value); + } } } } @@ -140,7 +142,12 @@ unsafe extern "C" fn getter_fn<'a, Context: Default + 'a, T: VarAccess<'a, Conte return NGX_OK as isize; }; if let Some(v) = T::get(req) { - let Ok(data) = req.pool().alloc_bytes(v.inner().len) else { + let Some(pool) = req.pool() else { + req.log() + .error("Request pool is null when getting variable value".to_string()); + return NGX_ERROR as isize; + }; + let Ok(data) = pool.alloc_bytes(v.inner().len) else { req.log().error(format!( "Allocation failed for variable data, {} bytes", v.inner().len diff --git a/nginx_module/src/wrappers.rs b/nginx_module/src/wrappers.rs index b827c41..58e2a3a 100644 --- a/nginx_module/src/wrappers.rs +++ b/nginx_module/src/wrappers.rs @@ -66,8 +66,8 @@ impl<'pool> NgxConfig<'pool> { (&self.inner as *const ngx_conf_t).cast_mut() } - pub fn pool(&mut self) -> &mut Pool { - unsafe { Pool::from_raw(self.inner.pool) } + pub unsafe fn pool(&mut self) -> Option<&mut Pool> { + Pool::from_raw(self.inner.pool) } pub fn log(&self) -> &Log {