Code review notes

- New log macro that accepts NGX_LOG_DEBUG bitmask filters
- Add logging to events
- Updated internal_redirect to call request methods:
  - ngx_named_location, ngx_internal_redirect which accepts args
- Subrequest flags for masking and state
- Subrequest accepts query parameters as the arg flag
- Quelling lint errors and warnings
- Quelling fmt workflow errors

Cargo.toml

@@ -6,7 +6,7 @@ members = [
 
 [package]
 name = "ngx"
-version = "0.3.0-beta"
+version = "0.4.0-beta"
 edition = "2021"
 autoexamples = false
 categories = ["api-bindings", "network-programming"]

nginx-sys/src/lib.rs

@@ -70,11 +70,9 @@ pub use bindings::*;
 /// let data: &str = "example"; // The string to convert
 /// let ptr = str_to_uchar(pool, data);
 /// ```
-pub fn str_to_uchar(pool: *mut ngx_pool_t, data: &str) -> *mut u_char {
+pub unsafe fn str_to_uchar(pool: *mut ngx_pool_t, data: &str) -> *mut u_char {
     let ptr: *mut u_char = unsafe { ngx_palloc(pool, data.len() as _) as _ };
-    unsafe {
-        copy_nonoverlapping(data.as_ptr(), ptr, data.len());
-    }
+    copy_nonoverlapping(data.as_ptr(), ptr, data.len());
     ptr
 }
 
@@ -94,24 +92,20 @@ impl ngx_str_t {
         }
     }
 
-    /// Convert the nginx string to a `String` by copying its contents.
-    ///
-    /// # Returns
-    /// A new `String` containing the contents of the nginx string.
-    pub fn to_string(&self) -> String {
-        return String::from(self.to_str());
-    }
-
     /// Create an `ngx_str_t` instance from a `String`.
     ///
     /// # Arguments
     ///
     /// * `pool` - A pointer to the nginx memory pool (`ngx_pool_t`).
     /// * `data` - The `String` from which to create the nginx string.
     ///
+    /// # Safety
+    /// This function is marked as unsafe because it passes a raw pointer (pool) to another unsafe
+    /// function which allocates a buffer from the pool.
+    ///
     /// # Returns
     /// An `ngx_str_t` instance representing the given `String`.
-    pub fn from_string(pool: *mut ngx_pool_t, data: String) -> Self {
+    pub unsafe fn from_string(pool: *mut ngx_pool_t, data: String) -> Self {
         ngx_str_t {
             data: str_to_uchar(pool, data.as_str()),
             len: data.len() as _,
@@ -125,9 +119,13 @@ impl ngx_str_t {
     /// * `pool` - A pointer to the nginx memory pool (`ngx_pool_t`).
     /// * `data` - The string slice from which to create the nginx string.
     ///
+    /// # Safety
+    /// This function is marked as unsafe because it passes a raw pointer (pool) to another unsafe
+    /// function which allocates a buffer from the pool.
+    ///
     /// # Returns
     /// An `ngx_str_t` instance representing the given string slice.
-    pub fn from_str(pool: *mut ngx_pool_t, data: &str) -> Self {
+    pub unsafe fn from_str(pool: *mut ngx_pool_t, data: &str) -> Self {
         ngx_str_t {
             data: str_to_uchar(pool, data),
             len: data.len() as _,
@@ -190,11 +188,16 @@ impl TryFrom<ngx_str_t> for &str {
 /// let value: &str = "value"; // The value to add
 /// let result = add_to_ngx_table(table, pool, key, value);
 /// ```
-pub fn add_to_ngx_table(table: *mut ngx_table_elt_t, pool: *mut ngx_pool_t, key: &str, value: &str) -> Option<()> {
+pub unsafe fn add_to_ngx_table(
+    table: *mut ngx_table_elt_t,
+    pool: *mut ngx_pool_t,
+    key: &str,
+    value: &str,
+) -> Option<()> {
     if table.is_null() {
         return None;
     }
-    unsafe { table.as_mut() }.map(|table| {
+    table.as_mut().map(|table| {
         table.hash = 1;
         table.key.len = key.len() as _;
         table.key.data = str_to_uchar(pool, key);

src/core/event.rs

@@ -1,11 +1,20 @@
 use crate::ffi::*;
+use crate::ngx_log_debug_mask;
 
+/// Wrapper struct for an `ngx_event_t` pointer, provides an interface into timer methods.
 #[repr(transparent)]
 pub struct Event(pub ngx_event_t);
 
 impl Event {
-    pub fn add_timer(&mut self, timer: ngx_msec_t) {
-        let key: ngx_msec_int_t = unsafe {ngx_current_msec as isize + timer as isize};
+    #[inline]
+    fn ident(&self) -> i32 {
+        let conn = self.0.data as *const ngx_connection_t;
+        unsafe { (*conn).fd }
+    }
+
+    /// Adds a timer to this event. Argument `timer` is in milliseconds.
+    pub fn add_timer(&mut self, timer_msec: ngx_msec_t) {
+        let key: ngx_msec_int_t = unsafe { ngx_current_msec as isize + timer_msec as isize };
         if self.0.timer_set() != 0 {
             /* FROM NGX:
              * Use a previous timer value if difference between it and a new
@@ -14,63 +23,94 @@ impl Event {
              */
             let diff = key - self.0.timer.key as ngx_msec_int_t;
             if diff.abs() < NGX_TIMER_LAZY_DELAY as isize {
-                // TODO add debugging macro
+                ngx_log_debug_mask!(
+                    NGX_LOG_DEBUG_EVENT,
+                    self.0.log,
+                    "event time: {}, old: {:?}, new: {:?}",
+                    self.ident(),
+                    self.0.timer.key,
+                    key
+                );
                 return;
             }
 
             self.del_timer();
         }
 
         self.0.timer.key = key as ngx_msec_t;
-        // TODO add debugging macro
+        ngx_log_debug_mask!(
+            NGX_LOG_DEBUG_EVENT,
+            self.0.log,
+            "event time: {}, old: {:?}, new: {:?}",
+            self.ident(),
+            self.0.timer.key,
+            key
+        );
         unsafe {
-            ngx_rbtree_insert(
-                &mut ngx_event_timer_rbtree as *mut _,
-                &mut self.0.timer as *mut _,
-            );
+            ngx_rbtree_insert(&mut ngx_event_timer_rbtree as *mut _, &mut self.0.timer as *mut _);
         }
 
         self.0.set_timer_set(1);
     }
 
+    /// Deletes an associated timer from this event.
     pub fn del_timer(&mut self) {
+        ngx_log_debug_mask!(
+            NGX_LOG_DEBUG_EVENT,
+            self.0.log,
+            "event timer del: {}:{:?}",
+            self.ident(),
+            self.0.timer.key
+        );
         unsafe {
-            ngx_rbtree_delete(
-                &mut ngx_event_timer_rbtree as *mut _,
-                &mut self.0.timer as *mut _,
-            );
+            ngx_rbtree_delete(&mut ngx_event_timer_rbtree as *mut _, &mut self.0.timer as *mut _);
         }
 
         self.0.set_timer_set(0);
     }
 
-    /// translated from ngx_post_event macro
-    pub fn post_to_queue(&mut self, queue: *mut ngx_queue_t) {
-        if self.0.posted() == 0{
+    /// Add event to processing queue. Translated from ngx_post_event macro.
+    ///
+    /// # Safety
+    /// This function is marked unsafe because it dereferences a raw pointer. The pointer (queue)
+    /// MUST NOT be null to satisfy its contract, will panic with null input.
+    ///
+    /// # Panics
+    /// Panics if the given queue is null.
+    pub unsafe fn post_to_queue(&mut self, queue: *mut ngx_queue_t) {
+        assert!(!queue.is_null(), "queue is empty");
+        if self.0.posted() == 0 {
             self.0.set_posted(1);
-            unsafe {
-                // translated from ngx_queue_insert_tail macro
-                self.0.queue.prev = (*queue).prev;
-                (*self.0.queue.prev).next = &self.0.queue as *const _ as *mut _;
-                self.0.queue.next = queue;
-                (*queue).prev = &self.0.queue as *const _ as *mut _;
-            }
+            // translated from ngx_queue_insert_tail macro
+            self.0.queue.prev = (*queue).prev;
+            (*self.0.queue.prev).next = &self.0.queue as *const _ as *mut _;
+            self.0.queue.next = queue;
+            (*queue).prev = &self.0.queue as *const _ as *mut _;
         }
     }
 
-    pub unsafe fn new_for_request(req: &crate::http::Request) -> &mut Event {
-        &mut *(req.pool().alloc(std::mem::size_of::<ngx_event_t>()) as *mut Event)
+    /// new_for_request creates an new Event (ngx_event_t) from the Request pool.
+    ///
+    /// # Safety
+    /// This function is marked as unsafe because it involves dereferencing a raw pointer memory
+    /// allocation from the underlying Nginx pool allocator.
+    ///
+    /// # Returns
+    /// An `Option<&mut Event>` representing the result of the allocation. `Some(&mut Event)`
+    /// indicates successful allocation, while `None` indicates a null Event.
+    pub unsafe fn new_for_request(req: &mut crate::http::Request) -> Option<&mut Event> {
+        Some(&mut *(req.pool().alloc(std::mem::size_of::<ngx_event_t>()) as *mut Event))
     }
 }
 
 impl From<*mut ngx_event_t> for &mut Event {
     fn from(evt: *mut ngx_event_t) -> Self {
-        unsafe {&mut *evt.cast::<Event>()}
+        unsafe { &mut *evt.cast::<Event>() }
     }
 }
 
-impl Into<*mut ngx_event_t> for &mut Event {
-    fn into(self) -> *mut ngx_event_t {
-        &mut self.0 as *mut ngx_event_t
+impl From<&mut Event> for *mut ngx_event_t {
+    fn from(val: &mut Event) -> Self {
+        &mut val.0 as *mut ngx_event_t
     }
 }

src/core/mod.rs

@@ -1,14 +1,14 @@
 mod buffer;
+mod event;
 mod pool;
 mod status;
 mod string;
-mod event;
 
 pub use buffer::*;
+pub use event::*;
 pub use pool::*;
 pub use status::*;
 pub use string::*;
-pub use event::*;
 
 /// Static empty configuration directive initializer for [`ngx_command_t`].
 ///