diff --git a/examples/upstream.rs b/examples/upstream.rs index 460726a..a4dee0b 100644 --- a/examples/upstream.rs +++ b/examples/upstream.rs @@ -13,19 +13,21 @@ use std::slice; use ngx::core::{Pool, Status}; use ngx::ffi::{ - nginx_version, ngx_atoi, ngx_command_t, ngx_conf_log_error, ngx_conf_t, ngx_connection_t, ngx_event_free_peer_pt, + nginx_version, ngx_atoi, ngx_command_t, ngx_conf_t, ngx_connection_t, ngx_event_free_peer_pt, ngx_event_get_peer_pt, ngx_http_module_t, ngx_http_upstream_init_peer_pt, ngx_http_upstream_init_pt, ngx_http_upstream_init_round_robin, ngx_http_upstream_module, ngx_http_upstream_srv_conf_t, ngx_http_upstream_t, ngx_int_t, ngx_module_t, ngx_peer_connection_t, ngx_str_t, ngx_uint_t, NGX_CONF_NOARGS, NGX_CONF_TAKE1, - NGX_CONF_UNSET, NGX_ERROR, NGX_HTTP_MODULE, NGX_HTTP_UPS_CONF, NGX_LOG_EMERG, NGX_RS_HTTP_SRV_CONF_OFFSET, + NGX_CONF_UNSET, NGX_ERROR, NGX_HTTP_MODULE, NGX_HTTP_UPS_CONF, NGX_RS_HTTP_SRV_CONF_OFFSET, NGX_RS_MODULE_SIGNATURE, }; use ngx::http::{ ngx_http_conf_get_module_srv_conf, ngx_http_conf_upstream_srv_conf_immutable, ngx_http_conf_upstream_srv_conf_mutable, HTTPModule, Merge, MergeConfigError, Request, }; -use ngx::log::DebugMask; -use ngx::{http_upstream_init_peer_pt, ngx_log_debug_http, ngx_log_debug_mask, ngx_null_command, ngx_string}; +use ngx::log::{DebugMask, Level}; +use ngx::{ + http_upstream_init_peer_pt, ngx_log_debug_http, ngx_log_debug_mask, ngx_log_error, ngx_null_command, ngx_string, +}; #[derive(Clone, Copy, Debug)] #[repr(C)] @@ -247,12 +249,7 @@ unsafe extern "C" fn ngx_http_upstream_init_custom( let maybe_conf: Option<*mut SrvConfig> = ngx_http_conf_upstream_srv_conf_mutable(us, &*addr_of!(ngx_http_upstream_custom_module)); if maybe_conf.is_none() { - ngx_conf_log_error( - NGX_LOG_EMERG as usize, - cf, - 0, - "CUSTOM UPSTREAM no upstream srv_conf".as_bytes().as_ptr() as *const c_char, - ); + ngx_log_error!(Level::Emerg, cf, "CUSTOM UPSTREAM no upstream srv_conf"); return isize::from(Status::NGX_ERROR); } let hccf = maybe_conf.unwrap(); @@ -263,12 +260,7 @@ unsafe extern "C" fn ngx_http_upstream_init_custom( let init_upstream_ptr = (*hccf).original_init_upstream.unwrap(); if init_upstream_ptr(cf, us) != Status::NGX_OK.into() { - ngx_conf_log_error( - NGX_LOG_EMERG as usize, - cf, - 0, - "CUSTOM UPSTREAM failed calling init_upstream".as_bytes().as_ptr() as *const c_char, - ); + ngx_log_error!(Level::Emerg, cf, "CUSTOM UPSTREAM failed calling init_upstream"); return isize::from(Status::NGX_ERROR); } @@ -296,13 +288,12 @@ unsafe extern "C" fn ngx_http_upstream_commands_set_custom( let value: &[ngx_str_t] = slice::from_raw_parts((*(*cf).args).elts as *const ngx_str_t, (*(*cf).args).nelts); let n = ngx_atoi(value[1].data, value[1].len); if n == (NGX_ERROR as isize) || n == 0 { - ngx_conf_log_error( - NGX_LOG_EMERG as usize, + ngx_log_error!( + Level::Emerg, cf, - 0, - "invalid value \"%V\" in \"%V\" directive".as_bytes().as_ptr() as *const c_char, + "invalid value \"{}\" in \"{}\" directive", value[1], - &(*cmd).name, + &(*cmd).name ); return usize::MAX as *mut c_char; } @@ -340,14 +331,7 @@ impl HTTPModule for Module { let mut pool = Pool::from_ngx_pool((*cf).pool); let conf = pool.alloc_type::(); if conf.is_null() { - ngx_conf_log_error( - NGX_LOG_EMERG as usize, - cf, - 0, - "CUSTOM UPSTREAM could not allocate memory for config" - .as_bytes() - .as_ptr() as *const c_char, - ); + ngx_log_error!(Level::Emerg, cf, "CUSTOM UPSTREAM could not allocate memory for config"); return std::ptr::null_mut(); } diff --git a/src/http/request.rs b/src/http/request.rs index d25d614..fbc6838 100644 --- a/src/http/request.rs +++ b/src/http/request.rs @@ -106,6 +106,32 @@ impl<'a> From<&'a mut Request> for *mut ngx_http_request_t { } } +impl crate::log::LogTarget for ngx_http_request_t { + #[inline] + fn debug_mask(&self) -> crate::log::DebugMask { + crate::log::DebugMask::Http + } + + #[inline] + fn get_log(&self) -> *const ngx_log_t { + // SAFETY: request must have a connecton and connection must have log + unsafe { (*self.connection).log } + } +} + +impl crate::log::LogTarget for Request { + #[inline] + fn debug_mask(&self) -> crate::log::DebugMask { + crate::log::DebugMask::Http + } + + #[inline] + fn get_log(&self) -> *const ngx_log_t { + // SAFETY: request must have a connecton and connection must have log + unsafe { (*self.connection()).log } + } +} + impl Request { /// Create a [`Request`] from an [`ngx_http_request_t`]. /// diff --git a/src/log.rs b/src/log.rs index a0f0397..9d75832 100644 --- a/src/log.rs +++ b/src/log.rs @@ -1,28 +1,173 @@ -/// Utility function to provide typed checking of the mask's field state. -#[inline(always)] -pub fn check_mask(mask: DebugMask, log_level: usize) -> bool { - let mask_bits: u32 = mask.into(); - if log_level & mask_bits as usize == 0 { +// `fmt::Arguments` in log macros stores references to temporaries and cannot be extracted to a variable, +// thus we are not able to move it out of the unsafe block. +#![allow(clippy::macro_metavars_in_unsafe)] + +use std::ffi::CStr; +use std::fmt; + +use crate::ffi::{self, ngx_conf_log_error, ngx_err_t, ngx_log_error_core, ngx_log_t, ngx_uint_t}; + +/// Checks if the message of the specified level should be logged with this logger. +/// +/// # Safety +/// +/// The function should be called with a valid target pointer. +#[inline] +pub unsafe fn should_log(target: *const T, level: Level) -> bool { + debug_assert!(!target.is_null()); + let log = (*target).get_log(); + if log.is_null() { + return false; + } + (*log).log_level >= level.into() +} + +/// Checks if the debug message with the specified mask should be logged with this logger. +/// +/// # Safety +/// +/// The function should be called with a valid target pointer. +#[inline] +pub unsafe fn should_debug(target: *const T, mask: Option) -> bool { + debug_assert!(!target.is_null()); + let log = (*target).get_log(); + if log.is_null() { return false; } - true + let mask: u32 = mask.unwrap_or((*target).debug_mask()).into(); + (*log).log_level & mask as usize != 0 } -/// Write to logger at a specified level. +/// Writes [std::fmt::Arguments] into the nginx logger. +/// +/// # Safety /// -/// See [Logging](https://nginx.org/en/docs/dev/development_guide.html#logging) -/// for available log levels. +/// The function should be called with a valid target pointer. +#[inline] +pub unsafe fn log_error(target: *const T, level: Level, err: ngx_err_t, args: fmt::Arguments<'_>) { + debug_assert!(!target.is_null()); + if let Some(str) = args.as_str() { + (*target).write_log(level, err, str.as_bytes()); + } else { + (*target).write_log(level, err, args.to_string().as_bytes()); + } +} + +/// Severity level +#[repr(usize)] +#[derive(PartialEq, Eq, PartialOrd, Ord, Debug)] +pub enum Level { + /// System is unusable + Emerg = ffi::NGX_LOG_EMERG as usize, + /// Action must be taken immediately + Alert = ffi::NGX_LOG_ALERT as usize, + /// Critical conditions + Crit = ffi::NGX_LOG_CRIT as usize, + /// Error conditions + Err = ffi::NGX_LOG_ERR as usize, + /// Warning conditions + Warn = ffi::NGX_LOG_WARN as usize, + /// Normal but significant condition + Notice = ffi::NGX_LOG_NOTICE as usize, + /// Informational messages + Info = ffi::NGX_LOG_INFO as usize, + /// Debug-level messages + Debug = ffi::NGX_LOG_DEBUG as usize, +} + +impl From for ngx_uint_t { + #[inline] + fn from(value: Level) -> Self { + value as ngx_uint_t + } +} + +/// Utility trait for nginx structures that contain logger objects +pub trait LogTarget { + /// Default debug mask for this target + #[inline] + fn debug_mask(&self) -> DebugMask { + DebugMask::Core + } + + /// Returns `ngx_log_t` owned by the target + fn get_log(&self) -> *const ngx_log_t; + + /// Low-level implementation for writing byte slice into the nginx logger + #[inline] + fn write_log(&self, level: Level, err: ngx_err_t, message: &[u8]) { + const FORMAT: &CStr = unsafe { CStr::from_bytes_with_nul_unchecked(b"%*s\0") }; + let log = self.get_log().cast_mut(); + unsafe { ngx_log_error_core(level.into(), log, err, FORMAT.as_ptr(), message.len(), message.as_ptr()) }; + } +} + +/// Implementations for the main types +impl LogTarget for ngx_log_t { + #[inline] + fn get_log(&self) -> *const ngx_log_t { + self + } +} + +impl LogTarget for ffi::ngx_cycle_t { + #[inline] + fn get_log(&self) -> *const ngx_log_t { + self.log + } +} + +impl LogTarget for ffi::ngx_conf_t { + #[inline] + fn get_log(&self) -> *const ngx_log_t { + self.log + } + + #[inline] + fn write_log(&self, level: Level, err: ngx_err_t, msg: &[u8]) { + const FORMAT: &CStr = unsafe { CStr::from_bytes_with_nul_unchecked(b"%*s\0") }; + if level < Level::Debug { + // ngx_conf_log_error will not mutate the cf argument. + // ngx_log_error_core may mutate the log argument, but cf does not own the log. + let cf = self as *const _ as *mut _; + unsafe { ngx_conf_log_error(level.into(), cf, err, FORMAT.as_ptr(), msg.len(), msg.as_ptr()) }; + } else { + // Debug messages don't need the configuration file context + // SAFETY: this should called after `should_log` or `should_debug`, when we already know + // that the log pointer is valid + unsafe { &*self.log }.write_log(level, err, msg); + } + } +} + +impl LogTarget for ffi::ngx_event_t { + #[inline] + fn debug_mask(&self) -> DebugMask { + DebugMask::Event + } + + #[inline] + fn get_log(&self) -> *const ngx_log_t { + self.log + } +} + +/// Write to logger at a specified [Level]. +#[macro_export] +macro_rules! ngx_log_error { + ( $level:expr, $log:expr, $($arg:tt)* ) => { + if unsafe { $crate::log::should_log($log, $level) } { + unsafe { $crate::log::log_error($log, $level, 0, format_args!($($arg)*)) }; + } + } +} + +/// Write to logger at debug level. #[macro_export] macro_rules! ngx_log_debug { ( $log:expr, $($arg:tt)* ) => { - let log_level = unsafe { (*$log).log_level }; - if log_level != 0 { - let level = $crate::ffi::NGX_LOG_DEBUG as $crate::ffi::ngx_uint_t; - let fmt = ::std::ffi::CString::new("%s").unwrap(); - let c_message = ::std::ffi::CString::new(format!($($arg)*)).unwrap(); - unsafe { - $crate::ffi::ngx_log_error_core(level, $log, 0, fmt.as_ptr(), c_message.as_ptr()); - } + if unsafe { $crate::log::should_debug($log, None) } { + unsafe { $crate::log::log_error($log, $crate::log::Level::Debug, 0, format_args!($($arg)*)) }; } } } @@ -33,8 +178,9 @@ macro_rules! ngx_log_debug { #[macro_export] macro_rules! ngx_log_debug_http { ( $request:expr, $($arg:tt)* ) => { - let log = unsafe { (*$request.connection()).log }; - $crate::ngx_log_debug!(log, $($arg)*); + if unsafe { $crate::log::should_debug($request, None) } { + unsafe { $crate::log::log_error($request, $crate::log::Level::Debug, 0, format_args!($($arg)*)) }; + } } } @@ -49,80 +195,38 @@ macro_rules! ngx_log_debug_http { #[macro_export] macro_rules! ngx_log_debug_mask { ( DebugMask::Core, $log:expr, $($arg:tt)* ) => ({ - let log_level = unsafe { (*$log).log_level }; - if $crate::log::check_mask(DebugMask::Core, log_level) { - let level = $crate::ffi::NGX_LOG_DEBUG as $crate::ffi::ngx_uint_t; - let fmt = ::std::ffi::CString::new("%s").unwrap(); - let c_message = ::std::ffi::CString::new(format!($($arg)*)).unwrap(); - unsafe { - $crate::ffi::ngx_log_error_core(level, $log, 0, fmt.as_ptr(), c_message.as_ptr()); - } + if unsafe { $crate::log::should_debug($log, Some(DebugMask::Core)) } { + unsafe { $crate::log::log_error($log, $crate::log::Level::Debug, 0, format_args!($($arg)*)) }; } }); ( DebugMask::Alloc, $log:expr, $($arg:tt)* ) => ({ - let log_level = unsafe { (*$log).log_level }; - if $crate::log::check_mask(DebugMask::Alloc, log_level) { - let level = $crate::ffi::NGX_LOG_DEBUG as $crate::ffi::ngx_uint_t; - let fmt = ::std::ffi::CString::new("%s").unwrap(); - let c_message = ::std::ffi::CString::new(format!($($arg)*)).unwrap(); - unsafe { - $crate::ffi::ngx_log_error_core(level, $log, 0, fmt.as_ptr(), c_message.as_ptr()); - } + if unsafe { $crate::log::should_debug($log, Some(DebugMask::Alloc)) } { + unsafe { $crate::log::log_error($log, $crate::log::Level::Debug, 0, format_args!($($arg)*)) }; } }); ( DebugMask::Mutex, $log:expr, $($arg:tt)* ) => ({ - let log_level = unsafe { (*$log).log_level }; - if $crate::log::check_mask(DebugMask::Mutex, log_level) { - let level = $crate::ffi::NGX_LOG_DEBUG as $crate::ffi::ngx_uint_t; - let fmt = ::std::ffi::CString::new("%s").unwrap(); - let c_message = ::std::ffi::CString::new(format!($($arg)*)).unwrap(); - unsafe { - $crate::ffi::ngx_log_error_core(level, $log, 0, fmt.as_ptr(), c_message.as_ptr()); - } + if unsafe { $crate::log::should_debug($log, Some(DebugMask::Mutex)) } { + unsafe { $crate::log::log_error($log, $crate::log::Level::Debug, 0, format_args!($($arg)*)) }; } }); ( DebugMask::Event, $log:expr, $($arg:tt)* ) => ({ - let log_level = unsafe { (*$log).log_level }; - if $crate::log::check_mask(DebugMask::Event, log_level) { - let level = $crate::ffi::NGX_LOG_DEBUG as $crate::ffi::ngx_uint_t; - let fmt = ::std::ffi::CString::new("%s").unwrap(); - let c_message = ::std::ffi::CString::new(format!($($arg)*)).unwrap(); - unsafe { - $crate::ffi::ngx_log_error_core(level, $log, 0, fmt.as_ptr(), c_message.as_ptr()); - } + if unsafe { $crate::log::should_debug($log, Some(DebugMask::Event)) } { + unsafe { $crate::log::log_error($log, $crate::log::Level::Debug, 0, format_args!($($arg)*)) }; } }); ( DebugMask::Http, $log:expr, $($arg:tt)* ) => ({ - let log_level = unsafe { (*$log).log_level }; - if $crate::log::check_mask(DebugMask::Http, log_level) { - let level = $crate::ffi::NGX_LOG_DEBUG as $crate::ffi::ngx_uint_t; - let fmt = ::std::ffi::CString::new("%s").unwrap(); - let c_message = ::std::ffi::CString::new(format!($($arg)*)).unwrap(); - unsafe { - $crate::ffi::ngx_log_error_core(level, $log, 0, fmt.as_ptr(), c_message.as_ptr()); - } + if unsafe { $crate::log::should_debug($log, Some(DebugMask::Http))} { + unsafe { $crate::log::log_error($log, $crate::log::Level::Debug, 0, format_args!($($arg)*)) }; } }); ( DebugMask::Mail, $log:expr, $($arg:tt)* ) => ({ - let log_level = unsafe { (*$log).log_level }; - if $crate::log::check_mask(DebugMask::Mail, log_level) { - let level = $crate::ffi::NGX_LOG_DEBUG as $crate::ffi::ngx_uint_t; - let fmt = ::std::ffi::CString::new("%s").unwrap(); - let c_message = ::std::ffi::CString::new(format!($($arg)*)).unwrap(); - unsafe { - $crate::ffi::ngx_log_error_core(level, $log, 0, fmt.as_ptr(), c_message.as_ptr()); - } + if unsafe { $crate::log::should_debug($log, Some(DebugMask::Mail)) } { + unsafe { $crate::log::log_error($log, $crate::log::Level::Debug, 0, format_args!($($arg)*)) }; } }); ( DebugMask::Stream, $log:expr, $($arg:tt)* ) => ({ - let log_level = unsafe { (*$log).log_level }; - if $crate::log::check_mask(DebugMask::Stream, log_level) { - let level = $crate::ffi::NGX_LOG_DEBUG as $crate::ffi::ngx_uint_t; - let fmt = ::std::ffi::CString::new("%s").unwrap(); - let c_message = ::std::ffi::CString::new(format!($($arg)*)).unwrap(); - unsafe { - $crate::ffi::ngx_log_error_core(level, $log, 0, fmt.as_ptr(), c_message.as_ptr()); - } + if unsafe { $crate::log::should_debug($log, Some(DebugMask::Stream)) } { + unsafe { $crate::log::log_error($log, $crate::log::Level::Debug, 0, format_args!($($arg)*)) }; } }); } @@ -183,6 +287,32 @@ mod tests { use super::*; + struct MockLog { + log: ngx_log_t, + buffer: std::cell::Cell>, + } + + impl MockLog { + fn new(level: u32) -> Self { + let mut inst = MockLog { + log: unsafe { std::mem::zeroed() }, + buffer: vec![].into(), + }; + inst.log.log_level = level as _; + inst + } + } + + impl LogTarget for MockLog { + fn get_log(&self) -> *const ngx_log_t { + &self.log + } + + fn write_log(&self, _level: Level, _err: ngx_err_t, message: &[u8]) { + self.buffer.set(message.to_vec()); + } + } + #[test] fn test_mask_lower_bound() { assert!(>::into(DebugMask::Core) == crate::ffi::NGX_LOG_DEBUG_FIRST); @@ -192,16 +322,44 @@ mod tests { assert!(>::into(DebugMask::Stream) == crate::ffi::NGX_LOG_DEBUG_LAST); } #[test] - fn test_check_mask() { - struct MockLog { - log_level: usize, - } - let mock = MockLog { log_level: 16 }; + fn test_mask() { + let log = MockLog::new(crate::ffi::NGX_LOG_DEBUG_CORE); + + let mut r = unsafe { should_debug(&log, None) }; + assert!(r); - let mut r = check_mask(DebugMask::Core, mock.log_level); + r = unsafe { should_debug(&log, Some(DebugMask::Core)) }; assert!(r); - r = check_mask(DebugMask::Alloc, mock.log_level); + r = unsafe { should_debug(&log, Some(DebugMask::Alloc)) }; assert!(!r); + + ngx_log_debug!(&log, "mask-core-default"); + assert_eq!(log.buffer.take(), b"mask-core-default"); + + ngx_log_debug_mask!(DebugMask::Core, &log, "mask-core"); + assert_eq!(log.buffer.take(), b"mask-core"); + + ngx_log_debug_mask!(DebugMask::Alloc, &log, "mask-alloc"); + assert_ne!(log.buffer.take(), b"mask-alloc"); + } + #[test] + fn test_level() { + let log = MockLog::new(crate::ffi::NGX_LOG_NOTICE); + + ngx_log_error!(Level::Warn, &log, "level-warn"); + assert_eq!(log.buffer.take(), b"level-warn"); + + ngx_log_error!(Level::Notice, &log, "level-notice"); + assert_eq!(log.buffer.take(), b"level-notice"); + + ngx_log_error!(Level::Info, &log, "level-info"); + assert_ne!(log.buffer.take(), b"level-info"); + + ngx_log_error!(Level::Debug, &log, "level-debug"); + assert_ne!(log.buffer.take(), b"level-debug"); + + ngx_log_error!(Level::Err, &log, "level-err"); + assert_eq!(log.buffer.take(), b"level-err"); } }