From dadcb3e1cac67480692d82afe1a442a6f91f8ffe Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 20 Aug 2025 11:42:11 -0700 Subject: [PATCH] refactor: changes to support edition = "2024" enforced by workspace level lints, now specified in each Cargo.toml, except nginx-sys, because bindgen can't be set to Edition2024 until MSRV hits 1.85 this change set is created automatically by `cargo fix --edition --all`, then with manual fixes afterwards: * Back out any use of unsafe(no_mangle) for plain no_mangle, since 1.81 wont accept it * remove uses of expr_2021 in macros, which is not available in the MSRV. * Manual fix to ngx_container_of! macro for safety rules in 2024, these weren't migrated automatically by `cargo fix` * Manual fixes to several other macros that created an &mut Request in an expression, Rust 2024 is stricter about taking &mut of temporaries, so instead the request is let-bound first. --- Cargo.toml | 7 ++ examples/Cargo.toml | 3 + examples/async.rs | 34 +++++---- examples/awssig.rs | 42 +++++++---- examples/curl.rs | 26 ++++--- examples/httporigdst.rs | 100 ++++++++++++++----------- examples/shared_dict.rs | 18 +++-- examples/upstream.rs | 150 +++++++++++++++++++++----------------- nginx-src/Cargo.toml | 3 + nginx-sys/Cargo.toml | 5 ++ nginx-sys/build/main.rs | 2 + nginx-sys/src/detail.rs | 22 +++--- nginx-sys/src/event.rs | 70 ++++++++++-------- nginx-sys/src/lib.rs | 70 ++++++++++-------- nginx-sys/src/queue.rs | 66 ++++++++++------- nginx-sys/src/rbtree.rs | 26 ++++--- nginx-sys/src/string.rs | 18 +++-- src/async_/sleep.rs | 2 +- src/async_/spawn.rs | 2 +- src/collections/queue.rs | 24 +++--- src/collections/rbtree.rs | 4 +- src/core/mod.rs | 8 +- src/core/pool.rs | 75 ++++++++++--------- src/core/slab.rs | 14 ++-- src/core/string.rs | 8 +- src/http/conf.rs | 74 +++++++++++-------- src/http/module.rs | 42 +++++++---- src/http/request.rs | 44 +++++------ src/http/upstream.rs | 6 +- 29 files changed, 554 insertions(+), 411 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index fd1021f8..d919066c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,6 +12,10 @@ homepage = "https://github.com/nginx/ngx-rust" repository = "https://github.com/nginx/ngx-rust" rust-version = "1.81.0" +[workspace.lints.rust] +rust-2024-compatibility = "warn" +edition-2024-expr-fragment-specifier = "allow" + [package] name = "ngx" version = "0.5.0-beta" @@ -70,3 +74,6 @@ maintenance = { status = "experimental" } [dev-dependencies] tempfile = { version = "3.20.0", default-features = false } + +[lints] +workspace = true diff --git a/examples/Cargo.toml b/examples/Cargo.toml index 9006aba7..afde14b9 100644 --- a/examples/Cargo.toml +++ b/examples/Cargo.toml @@ -65,3 +65,6 @@ default = ["export-modules", "ngx/vendored"] # See https://github.com/rust-lang/rust/issues/20267 export-modules = [] linux = [] + +[lints] +workspace = true diff --git a/examples/async.rs b/examples/async.rs index 47f5de8e..b0dbaf38 100644 --- a/examples/async.rs +++ b/examples/async.rs @@ -24,19 +24,21 @@ impl http::HttpModule for 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(); + unsafe { + // 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(async_access_handler); + core::Status::NGX_OK.into() } - // set an Access phase handler - *h = Some(async_access_handler); - core::Status::NGX_OK.into() } } @@ -98,16 +100,16 @@ impl http::Merge for ModuleConfig { unsafe extern "C" fn check_async_work_done(event: *mut ngx_event_t) { let ctx = ngx::ngx_container_of!(event, RequestCTX, event); - let c: *mut ngx_connection_t = (*event).data.cast(); + let c: *mut ngx_connection_t = unsafe { (*event).data.cast() }; - if (*ctx).done.load(Ordering::Relaxed) { + if unsafe { (*ctx).done.load(Ordering::Relaxed) } { // Triggering async_access_handler again - ngx_post_event((*c).write, addr_of_mut!(ngx_posted_events)); + unsafe { ngx_post_event((*c).write, addr_of_mut!(ngx_posted_events)) }; } else { // this doesn't have have good performance but works as a simple thread-safe example and // doesn't causes segfault. The best method that provides both thread-safety and // performance requires an nginx patch. - ngx_post_event(event, addr_of_mut!(ngx_posted_next_events)); + unsafe { ngx_post_event(event, addr_of_mut!(ngx_posted_next_events)) }; } } diff --git a/examples/awssig.rs b/examples/awssig.rs index cbbea102..d1fae242 100644 --- a/examples/awssig.rs +++ b/examples/awssig.rs @@ -1,3 +1,8 @@ +// Allow following allow to work in MSRV 1.81 +#![allow(unknown_lints)] +// Suppress spurrions lint for 2024 compatibility +#![allow(tail_expr_drop_order)] + use std::ffi::{c_char, c_void}; use http::HeaderMap; @@ -19,19 +24,21 @@ impl HttpModule for 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_PRECONTENT_PHASE as usize].handlers, - ) as *mut ngx_http_handler_pt; - if h.is_null() { - return core::Status::NGX_ERROR.into(); + unsafe { + // 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_PRECONTENT_PHASE as usize].handlers, + ) as *mut ngx_http_handler_pt; + if h.is_null() { + return core::Status::NGX_ERROR.into(); + } + // set an phase handler + *h = Some(awssigv4_header_handler); + core::Status::NGX_OK.into() } - // set an phase handler - *h = Some(awssigv4_header_handler); - core::Status::NGX_OK.into() } } @@ -298,10 +305,13 @@ http_request_handler!(awssigv4_header_handler, |request: &mut Request| { for (name, value) in request.headers_in_iterator() { if let Ok(name) = name.to_str() { if name.to_lowercase() == "host" { - if let Ok(value) = http::HeaderValue::from_bytes(value.as_bytes()) { - headers.insert(http::header::HOST, value); - } else { - return core::Status::NGX_DECLINED; + match http::HeaderValue::from_bytes(value.as_bytes()) { + Ok(value) => { + headers.insert(http::header::HOST, value); + } + _ => { + return core::Status::NGX_DECLINED; + } } } } else { diff --git a/examples/curl.rs b/examples/curl.rs index 3d07002f..db7a8513 100644 --- a/examples/curl.rs +++ b/examples/curl.rs @@ -18,19 +18,21 @@ impl http::HttpModule for 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(); + unsafe { + // 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() } - // set an Access phase handler - *h = Some(curl_access_handler); - core::Status::NGX_OK.into() } } diff --git a/examples/httporigdst.rs b/examples/httporigdst.rs index 75cc6527..04223d23 100644 --- a/examples/httporigdst.rs +++ b/examples/httporigdst.rs @@ -1,5 +1,5 @@ use std::ffi::{c_int, c_void}; -use std::ptr::addr_of; +use std::ptr::{addr_of, NonNull}; use ngx::core; use ngx::ffi::{ @@ -47,29 +47,33 @@ impl NgxHttpOrigDstCtx { } pub unsafe fn bind_addr(&self, v: *mut ngx_variable_value_t) { + let mut v = NonNull::new(v).unwrap(); + let v = unsafe { v.as_mut() }; 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) { + let mut v = NonNull::new(v).unwrap(); + let v = unsafe { v.as_mut() }; 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; } } @@ -129,19 +133,21 @@ unsafe fn ngx_get_origdst( ) -> Result<(String, in_port_t), core::Status> { let c = request.connection(); - if (*c).type_ != libc::SOCK_STREAM { + if unsafe { (*c).type_ } != libc::SOCK_STREAM { ngx_log_debug_http!(request, "httporigdst: connection is not type SOCK_STREAM"); return Err(core::Status::NGX_DECLINED); } - if ngx_connection_local_sockaddr(c, std::ptr::null_mut(), 0) != core::Status::NGX_OK.into() { + if unsafe { ngx_connection_local_sockaddr(c, std::ptr::null_mut(), 0) } + != core::Status::NGX_OK.into() + { ngx_log_debug_http!(request, "httporigdst: no local sockaddr from connection"); return Err(core::Status::NGX_ERROR); } let level: c_int; let optname: c_int; - match (*(*c).local_sockaddr).sa_family as i32 { + match unsafe { (*(*c).local_sockaddr).sa_family } as i32 { libc::AF_INET => { level = libc::SOL_IP; optname = libc::SO_ORIGINAL_DST; @@ -152,15 +158,17 @@ unsafe fn ngx_get_origdst( } } - let mut addr: sockaddr_storage = { std::mem::zeroed() }; + let mut addr: sockaddr_storage = unsafe { std::mem::zeroed() }; let mut addrlen: libc::socklen_t = std::mem::size_of_val(&addr) as libc::socklen_t; - let rc = libc::getsockopt( - (*c).fd, - level, - optname, - &mut addr as *mut _ as *mut _, - &mut addrlen as *mut u32, - ); + let rc = unsafe { + libc::getsockopt( + (*c).fd, + level, + optname, + &mut addr as *mut _ as *mut _, + &mut addrlen as *mut u32, + ) + }; if rc == -1 { ngx_log_debug_http!(request, "httporigdst: getsockopt failed"); return Err(core::Status::NGX_DECLINED); @@ -192,10 +200,11 @@ 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::(&*addr_of!(ngx_http_orig_dst_module)); + let ctx = request + .get_module_ctx::(unsafe { &*addr_of!(ngx_http_orig_dst_module) }); if let Some(obj) = ctx { ngx_log_debug_http!(request, "httporigdst: found context and binding variable",); - obj.bind_addr(v); + unsafe { obj.bind_addr(v) }; return core::Status::NGX_OK; } // lazy initialization: @@ -204,7 +213,7 @@ http_variable_get!( // set context // bind address ngx_log_debug_http!(request, "httporigdst: context not found, getting address"); - let r = ngx_get_origdst(request); + let r = unsafe { ngx_get_origdst(request) }; match r { Err(e) => { return e; @@ -226,10 +235,11 @@ http_variable_get!( ip, port, ); - (*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)); + unsafe { (*new_ctx).save(&ip, port, &request.pool()) }; + unsafe { (*new_ctx).bind_addr(v) }; + request.set_module_ctx(new_ctx as *mut c_void, unsafe { + &*addr_of!(ngx_http_orig_dst_module) + }); } } core::Status::NGX_OK @@ -239,10 +249,11 @@ http_variable_get!( 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::(&*addr_of!(ngx_http_orig_dst_module)); + let ctx = request + .get_module_ctx::(unsafe { &*addr_of!(ngx_http_orig_dst_module) }); if let Some(obj) = ctx { ngx_log_debug_http!(request, "httporigdst: found context and binding variable",); - obj.bind_port(v); + unsafe { obj.bind_port(v) }; return core::Status::NGX_OK; } // lazy initialization: @@ -251,7 +262,7 @@ http_variable_get!( // set context // bind port ngx_log_debug_http!(request, "httporigdst: context not found, getting address"); - let r = ngx_get_origdst(request); + let r = unsafe { ngx_get_origdst(request) }; match r { Err(e) => { return e; @@ -273,10 +284,11 @@ http_variable_get!( ip, port, ); - (*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)); + unsafe { (*new_ctx).save(&ip, port, &request.pool()) }; + unsafe { (*new_ctx).bind_port(v) }; + request.set_module_ctx(new_ctx as *mut c_void, unsafe { + &*addr_of!(ngx_http_orig_dst_module) + }); } } core::Status::NGX_OK @@ -292,13 +304,15 @@ impl HttpModule for Module { // static ngx_int_t ngx_http_orig_dst_add_variables(ngx_conf_t *cf) unsafe extern "C" fn preconfiguration(cf: *mut ngx_conf_t) -> ngx_int_t { - for mut v in NGX_HTTP_ORIG_DST_VARS { - let var = ngx_http_add_variable(cf, &mut v.name, v.flags); - if var.is_null() { + for mut v in unsafe { NGX_HTTP_ORIG_DST_VARS } { + let var = NonNull::new(unsafe { ngx_http_add_variable(cf, &mut v.name, v.flags) }); + if var.is_none() { return core::Status::NGX_ERROR.into(); } - (*var).get_handler = v.get_handler; - (*var).data = v.data; + let mut var = var.unwrap(); + let var = unsafe { var.as_mut() }; + var.get_handler = v.get_handler; + var.data = v.data; } core::Status::NGX_OK.into() } diff --git a/examples/shared_dict.rs b/examples/shared_dict.rs index 1981e1aa..feb15a04 100644 --- a/examples/shared_dict.rs +++ b/examples/shared_dict.rs @@ -23,16 +23,18 @@ impl HttpModule for HttpSharedDictModule { } unsafe extern "C" fn preconfiguration(cf: *mut ngx_conf_t) -> ngx_int_t { - for mut v in NGX_HTTP_SHARED_DICT_VARS { - let var = ngx_http_add_variable(cf, &mut v.name, v.flags); - if var.is_null() { - return Status::NGX_ERROR.into(); + unsafe { + for mut v in NGX_HTTP_SHARED_DICT_VARS { + let var = ngx_http_add_variable(cf, &mut v.name, v.flags); + if var.is_null() { + return Status::NGX_ERROR.into(); + } + (*var).get_handler = v.get_handler; + (*var).set_handler = v.set_handler; + (*var).data = v.data; } - (*var).get_handler = v.get_handler; - (*var).set_handler = v.set_handler; - (*var).data = v.data; + Status::NGX_OK.into() } - Status::NGX_OK.into() } } diff --git a/examples/upstream.rs b/examples/upstream.rs index 5ea2c81b..ad7f2bd9 100644 --- a/examples/upstream.rs +++ b/examples/upstream.rs @@ -168,25 +168,27 @@ unsafe extern "C" fn ngx_http_upstream_get_custom_peer( pc: *mut ngx_peer_connection_t, data: *mut c_void, ) -> ngx_int_t { - let hcpd: *mut UpstreamPeerData = unsafe { mem::transmute(data) }; + unsafe { + let hcpd: *mut UpstreamPeerData = mem::transmute(data); - ngx_log_debug_mask!( - DebugMask::Http, - (*pc).log, - "CUSTOM UPSTREAM get peer, try: {}, conn: {:p}", - (*pc).tries, - (*hcpd).client_connection.unwrap(), - ); + ngx_log_debug_mask!( + DebugMask::Http, + (*pc).log, + "CUSTOM UPSTREAM get peer, try: {}, conn: {:p}", + (*pc).tries, + (*hcpd).client_connection.unwrap(), + ); - let original_get_peer = (*hcpd).original_get_peer.unwrap(); - let rc = original_get_peer(pc, (*hcpd).data); + let original_get_peer = (*hcpd).original_get_peer.unwrap(); + let rc = original_get_peer(pc, (*hcpd).data); - if rc != Status::NGX_OK.into() { - return rc; - } + if rc != Status::NGX_OK.into() { + return rc; + } - ngx_log_debug_mask!(DebugMask::Http, (*pc).log, "CUSTOM UPSTREAM end get peer"); - Status::NGX_OK.into() + ngx_log_debug_mask!(DebugMask::Http, (*pc).log, "CUSTOM UPSTREAM end get peer"); + Status::NGX_OK.into() + } } // ngx_http_upstream_free_custom_peer @@ -197,15 +199,23 @@ unsafe extern "C" fn ngx_http_upstream_free_custom_peer( data: *mut c_void, state: ngx_uint_t, ) { - ngx_log_debug_mask!(DebugMask::Http, (*pc).log, "CUSTOM UPSTREAM free peer"); + ngx_log_debug_mask!( + DebugMask::Http, + unsafe { (*pc).log }, + "CUSTOM UPSTREAM free peer" + ); let hcpd: *mut UpstreamPeerData = unsafe { mem::transmute(data) }; - let original_free_peer = (*hcpd).original_free_peer.unwrap(); + let original_free_peer = unsafe { (*hcpd).original_free_peer.unwrap() }; - original_free_peer(pc, (*hcpd).data, state); + unsafe { original_free_peer(pc, (*hcpd).data, state) }; - ngx_log_debug_mask!(DebugMask::Http, (*pc).log, "CUSTOM UPSTREAM end free peer"); + ngx_log_debug_mask!( + DebugMask::Http, + unsafe { (*pc).log }, + "CUSTOM UPSTREAM end free peer" + ); } // ngx_http_upstream_init_custom @@ -217,7 +227,7 @@ unsafe extern "C" fn ngx_http_upstream_init_custom( ) -> ngx_int_t { ngx_log_debug_mask!( DebugMask::Http, - (*cf).log, + unsafe { (*cf).log }, "CUSTOM UPSTREAM peer init_upstream" ); @@ -235,7 +245,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() { + if unsafe { init_upstream_ptr(cf, us) } != Status::NGX_OK.into() { ngx_conf_log_error!( NGX_LOG_EMERG, cf, @@ -249,7 +259,7 @@ unsafe extern "C" fn ngx_http_upstream_init_custom( ngx_log_debug_mask!( DebugMask::Http, - (*cf).log, + unsafe { (*cf).log }, "CUSTOM UPSTREAM end peer init_upstream" ); isize::from(Status::NGX_OK) @@ -263,41 +273,43 @@ unsafe extern "C" fn ngx_http_upstream_commands_set_custom( cmd: *mut ngx_command_t, conf: *mut c_void, ) -> *mut c_char { - // SAFETY: this function is called with non-NULL cf always - let cf = &mut *cf; - ngx_log_debug_mask!(DebugMask::Http, cf.log, "CUSTOM UPSTREAM module init"); - let args: &[ngx_str_t] = (*cf.args).as_slice(); - - let ccf = &mut (*(conf as *mut SrvConfig)); - - if let Some(value) = args.get(1) { - let n = ngx_atoi(value.data, value.len); - if n == (NGX_ERROR as isize) || n == 0 { - ngx_conf_log_error!( - NGX_LOG_EMERG, - cf, - "invalid value \"{}\" in \"{}\" directive", - value, - &(*cmd).name - ); - return ngx::core::NGX_CONF_ERROR; + unsafe { + // SAFETY: this function is called with non-NULL cf always + let cf = &mut *cf; + ngx_log_debug_mask!(DebugMask::Http, cf.log, "CUSTOM UPSTREAM module init"); + let args: &[ngx_str_t] = (*cf.args).as_slice(); + + let ccf = &mut (*(conf as *mut SrvConfig)); + + if let Some(value) = args.get(1) { + let n = ngx_atoi(value.data, value.len); + if n == (NGX_ERROR as isize) || n == 0 { + ngx_conf_log_error!( + NGX_LOG_EMERG, + cf, + "invalid value \"{}\" in \"{}\" directive", + value, + &(*cmd).name + ); + return ngx::core::NGX_CONF_ERROR; + } + ccf.max = n as u32; } - ccf.max = n as u32; - } - let uscf = NgxHttpUpstreamModule::server_conf_mut(cf).expect("http upstream srv conf"); + let uscf = NgxHttpUpstreamModule::server_conf_mut(cf).expect("http upstream srv conf"); - ccf.original_init_upstream = if uscf.peer.init_upstream.is_some() { - uscf.peer.init_upstream - } else { - Some(ngx_http_upstream_init_round_robin) - }; + ccf.original_init_upstream = if uscf.peer.init_upstream.is_some() { + uscf.peer.init_upstream + } else { + Some(ngx_http_upstream_init_round_robin) + }; - uscf.peer.init_upstream = Some(ngx_http_upstream_init_custom); + uscf.peer.init_upstream = Some(ngx_http_upstream_init_custom); - ngx_log_debug_mask!(DebugMask::Http, cf.log, "CUSTOM UPSTREAM end module init"); + ngx_log_debug_mask!(DebugMask::Http, cf.log, "CUSTOM UPSTREAM end module init"); - ngx::core::NGX_CONF_OK + ngx::core::NGX_CONF_OK + } } // The upstream module. @@ -311,25 +323,27 @@ impl HttpModule for Module { } unsafe extern "C" fn create_srv_conf(cf: *mut ngx_conf_t) -> *mut c_void { - let pool = Pool::from_ngx_pool((*cf).pool); - let conf = pool.alloc_type::(); - if conf.is_null() { - ngx_conf_log_error!( - NGX_LOG_EMERG, - cf, - "CUSTOM UPSTREAM could not allocate memory for config" + unsafe { + let pool = Pool::from_ngx_pool((*cf).pool); + let conf = pool.alloc_type::(); + if conf.is_null() { + ngx_conf_log_error!( + NGX_LOG_EMERG, + cf, + "CUSTOM UPSTREAM could not allocate memory for config" + ); + return std::ptr::null_mut(); + } + + (*conf).max = NGX_CONF_UNSET as u32; + + ngx_log_debug_mask!( + DebugMask::Http, + (*cf).log, + "CUSTOM UPSTREAM end create_srv_conf" ); - return std::ptr::null_mut(); + conf as *mut c_void } - - (*conf).max = NGX_CONF_UNSET as u32; - - ngx_log_debug_mask!( - DebugMask::Http, - (*cf).log, - "CUSTOM UPSTREAM end create_srv_conf" - ); - conf as *mut c_void } } diff --git a/nginx-src/Cargo.toml b/nginx-src/Cargo.toml index 4ac31046..e9423bd6 100644 --- a/nginx-src/Cargo.toml +++ b/nginx-src/Cargo.toml @@ -17,3 +17,6 @@ duct = "1" flate2 = "1" tar = "0.4" ureq = "3.0.10" + +[lints] +workspace = true diff --git a/nginx-sys/Cargo.toml b/nginx-sys/Cargo.toml index 4c2ce3ed..66356cd8 100644 --- a/nginx-sys/Cargo.toml +++ b/nginx-sys/Cargo.toml @@ -34,3 +34,8 @@ shlex = "1.3" [features] vendored = ["dep:nginx-src"] + +[lints] +# MSRV prevents configuring bindgen to emit Edition2024 for the moment, and +# workspace lints require 2024. Do not inherit workspace lints until MSRV +# above 1.85.0 diff --git a/nginx-sys/build/main.rs b/nginx-sys/build/main.rs index 8f85bd0c..95504cda 100644 --- a/nginx-sys/build/main.rs +++ b/nginx-sys/build/main.rs @@ -236,6 +236,8 @@ fn generate_binding(nginx: &NginxSource) { .clang_args(clang_args) .layout_tests(false) .rust_target(rust_target) + .rust_edition(bindgen::RustEdition::Edition2021) + .wrap_unsafe_ops(true) .use_core() .generate() .expect("Unable to generate bindings"); diff --git a/nginx-sys/src/detail.rs b/nginx-sys/src/detail.rs index 081e9933..78c333e5 100644 --- a/nginx-sys/src/detail.rs +++ b/nginx-sys/src/detail.rs @@ -12,12 +12,14 @@ use crate::bindings::{ngx_pnalloc, ngx_pool_t, u_char}; /// /// The caller must provide a valid pointer to the memory pool. pub unsafe fn bytes_to_uchar(pool: *mut ngx_pool_t, data: &[u8]) -> Option<*mut u_char> { - let ptr: *mut u_char = ngx_pnalloc(pool, data.len()) as _; - if ptr.is_null() { - return None; + unsafe { + let ptr: *mut u_char = ngx_pnalloc(pool, data.len()) as _; + if ptr.is_null() { + return None; + } + copy_nonoverlapping(data.as_ptr(), ptr, data.len()); + Some(ptr) } - copy_nonoverlapping(data.as_ptr(), ptr, data.len()); - Some(ptr) } /// Convert a string slice (`&str`) to a raw pointer (`*mut u_char`) allocated in the given nginx @@ -42,10 +44,12 @@ pub unsafe fn bytes_to_uchar(pool: *mut ngx_pool_t, data: &[u8]) -> Option<*mut /// let ptr = str_to_uchar(pool, data); /// ``` pub unsafe fn str_to_uchar(pool: *mut ngx_pool_t, data: &str) -> *mut u_char { - let ptr: *mut u_char = ngx_pnalloc(pool, data.len()) as _; - debug_assert!(!ptr.is_null()); - copy_nonoverlapping(data.as_ptr(), ptr, data.len()); - ptr + unsafe { + let ptr: *mut u_char = ngx_pnalloc(pool, data.len()) as _; + debug_assert!(!ptr.is_null()); + copy_nonoverlapping(data.as_ptr(), ptr, data.len()); + ptr + } } #[inline] diff --git a/nginx-sys/src/event.rs b/nginx-sys/src/event.rs index bd08d37c..3f48bea8 100644 --- a/nginx-sys/src/event.rs +++ b/nginx-sys/src/event.rs @@ -12,29 +12,31 @@ use crate::{ ///`ev` must be a valid pointer to an `ngx_event_t`. #[inline] pub unsafe fn ngx_add_timer(ev: *mut ngx_event_t, timer: ngx_msec_t) { - let key: ngx_msec_t = ngx_current_msec.wrapping_add(timer); + unsafe { + let key: ngx_msec_t = ngx_current_msec.wrapping_add(timer); - if (*ev).timer_set() != 0 { - /* - * Use a previous timer value if difference between it and a new - * value is less than NGX_TIMER_LAZY_DELAY milliseconds: this allows - * to minimize the rbtree operations for fast connections. - */ - if key.abs_diff((*ev).timer.key) < NGX_TIMER_LAZY_DELAY as _ { - return; - } + if (*ev).timer_set() != 0 { + /* + * Use a previous timer value if difference between it and a new + * value is less than NGX_TIMER_LAZY_DELAY milliseconds: this allows + * to minimize the rbtree operations for fast connections. + */ + if key.abs_diff((*ev).timer.key) < NGX_TIMER_LAZY_DELAY as _ { + return; + } - ngx_del_timer(ev); - } + ngx_del_timer(ev); + } - (*ev).timer.key = key; + (*ev).timer.key = key; - ngx_rbtree_insert( - ptr::addr_of_mut!(ngx_event_timer_rbtree), - ptr::addr_of_mut!((*ev).timer), - ); + ngx_rbtree_insert( + ptr::addr_of_mut!(ngx_event_timer_rbtree), + ptr::addr_of_mut!((*ev).timer), + ); - (*ev).set_timer_set(1); + (*ev).set_timer_set(1); + } } /// Deletes a previously set timeout. @@ -44,16 +46,18 @@ pub unsafe fn ngx_add_timer(ev: *mut ngx_event_t, timer: ngx_msec_t) { /// `ev` must be a valid pointer to an `ngx_event_t`, previously armed with [ngx_add_timer]. #[inline] pub unsafe fn ngx_del_timer(ev: *mut ngx_event_t) { - ngx_rbtree_delete( - ptr::addr_of_mut!(ngx_event_timer_rbtree), - ptr::addr_of_mut!((*ev).timer), - ); + unsafe { + ngx_rbtree_delete( + ptr::addr_of_mut!(ngx_event_timer_rbtree), + ptr::addr_of_mut!((*ev).timer), + ); - (*ev).timer.left = ptr::null_mut(); - (*ev).timer.right = ptr::null_mut(); - (*ev).timer.parent = ptr::null_mut(); + (*ev).timer.left = ptr::null_mut(); + (*ev).timer.right = ptr::null_mut(); + (*ev).timer.parent = ptr::null_mut(); - (*ev).set_timer_set(0); + (*ev).set_timer_set(0); + } } /// Post the event `ev` to the post queue `q`. @@ -64,9 +68,11 @@ pub unsafe fn ngx_del_timer(ev: *mut ngx_event_t) { /// `q` is a valid pointer to a queue head. #[inline] pub unsafe fn ngx_post_event(ev: *mut ngx_event_t, q: *mut ngx_queue_t) { - if (*ev).posted() == 0 { - (*ev).set_posted(1); - ngx_queue_insert_before(q, ptr::addr_of_mut!((*ev).queue)); + unsafe { + if (*ev).posted() == 0 { + (*ev).set_posted(1); + ngx_queue_insert_before(q, ptr::addr_of_mut!((*ev).queue)); + } } } @@ -78,6 +84,8 @@ pub unsafe fn ngx_post_event(ev: *mut ngx_event_t, q: *mut ngx_queue_t) { /// `ev.queue` is initialized with `ngx_queue_init`. #[inline] pub unsafe fn ngx_delete_posted_event(ev: *mut ngx_event_t) { - (*ev).set_posted(0); - ngx_queue_remove(ptr::addr_of_mut!((*ev).queue)); + unsafe { + (*ev).set_posted(0); + ngx_queue_remove(ptr::addr_of_mut!((*ev).queue)); + } } diff --git a/nginx-sys/src/lib.rs b/nginx-sys/src/lib.rs index 4d40e2b7..b741d3d7 100644 --- a/nginx-sys/src/lib.rs +++ b/nginx-sys/src/lib.rs @@ -54,17 +54,19 @@ impl ngx_array_t { /// The array must be a valid, initialized array containing elements of type T or compatible in /// layout with T (e.g. `#[repr(transparent)]` wrappers). pub unsafe fn as_slice(&self) -> &[T] { - debug_assert_eq!( - core::mem::size_of::(), - self.size, - "ngx_array_t::as_slice(): element size mismatch" - ); - if self.nelts == 0 { - &[] - } else { - // SAFETY: in a valid array, `elts` is a valid well-aligned pointer to at least `nelts` - // elements of size `size` - core::slice::from_raw_parts(self.elts.cast(), self.nelts) + unsafe { + debug_assert_eq!( + core::mem::size_of::(), + self.size, + "ngx_array_t::as_slice(): element size mismatch" + ); + if self.nelts == 0 { + &[] + } else { + // SAFETY: in a valid array, `elts` is a valid well-aligned pointer to at least `nelts` + // elements of size `size` + core::slice::from_raw_parts(self.elts.cast(), self.nelts) + } } } @@ -75,17 +77,19 @@ impl ngx_array_t { /// The array must be a valid, initialized array containing elements of type T or compatible in /// layout with T (e.g. `#[repr(transparent)]` wrappers). pub unsafe fn as_slice_mut(&mut self) -> &mut [T] { - debug_assert_eq!( - core::mem::size_of::(), - self.size, - "ngx_array_t::as_slice_mut(): element size mismatch" - ); - if self.nelts == 0 { - &mut [] - } else { - // SAFETY: in a valid array, `elts` is a valid well-aligned pointer to at least `nelts` - // elements of size `size` - core::slice::from_raw_parts_mut(self.elts.cast(), self.nelts) + unsafe { + debug_assert_eq!( + core::mem::size_of::(), + self.size, + "ngx_array_t::as_slice_mut(): element size mismatch" + ); + if self.nelts == 0 { + &mut [] + } else { + // SAFETY: in a valid array, `elts` is a valid well-aligned pointer to at least `nelts` + // elements of size `size` + core::slice::from_raw_parts_mut(self.elts.cast(), self.nelts) + } } } } @@ -326,16 +330,18 @@ pub unsafe fn add_to_ngx_table( key: impl AsRef<[u8]>, value: impl AsRef<[u8]>, ) -> Option<()> { - if let Some(table) = table.as_mut() { - let key = key.as_ref(); - table.key = ngx_str_t::from_bytes(pool, key)?; - table.value = ngx_str_t::from_bytes(pool, value.as_ref())?; - table.lowcase_key = ngx_pnalloc(pool, table.key.len).cast(); - if table.lowcase_key.is_null() { - return None; + unsafe { + if let Some(table) = table.as_mut() { + let key = key.as_ref(); + table.key = ngx_str_t::from_bytes(pool, key)?; + table.value = ngx_str_t::from_bytes(pool, value.as_ref())?; + table.lowcase_key = ngx_pnalloc(pool, table.key.len).cast(); + if table.lowcase_key.is_null() { + return None; + } + table.hash = ngx_hash_strlow(table.lowcase_key, table.key.data, table.key.len); + return Some(()); } - table.hash = ngx_hash_strlow(table.lowcase_key, table.key.data, table.key.len); - return Some(()); + None } - None } diff --git a/nginx-sys/src/queue.rs b/nginx-sys/src/queue.rs index a4b7804d..b493451e 100644 --- a/nginx-sys/src/queue.rs +++ b/nginx-sys/src/queue.rs @@ -23,8 +23,10 @@ macro_rules! ngx_queue_data { /// `q` must be a valid pointer to [ngx_queue_t]. #[inline] pub unsafe fn ngx_queue_init(q: *mut ngx_queue_t) { - (*q).prev = q; - (*q).next = q; + unsafe { + (*q).prev = q; + (*q).next = q; + } } /// Returns `true` if the queue contains no elements. @@ -34,7 +36,7 @@ pub unsafe fn ngx_queue_init(q: *mut ngx_queue_t) { /// `q` must be a valid pointer to [ngx_queue_t], initialized with [ngx_queue_init]. #[inline] pub unsafe fn ngx_queue_empty(q: *const ngx_queue_t) -> bool { - ptr::eq(q, (*q).prev) + unsafe { ptr::eq(q, (*q).prev) } } /// Inserts a new node after the current. @@ -44,10 +46,12 @@ pub unsafe fn ngx_queue_empty(q: *const ngx_queue_t) -> bool { /// Both `q` and `x` must be valid pointers to [ngx_queue_t] #[inline] pub unsafe fn ngx_queue_insert_after(q: *mut ngx_queue_t, x: *mut ngx_queue_t) { - (*x).next = (*q).next; - (*(*x).next).prev = x; - (*x).prev = q; - (*q).next = x; + unsafe { + (*x).next = (*q).next; + (*(*x).next).prev = x; + (*x).prev = q; + (*q).next = x; + } } /// Inserts a new node before the current. @@ -57,10 +61,12 @@ pub unsafe fn ngx_queue_insert_after(q: *mut ngx_queue_t, x: *mut ngx_queue_t) { /// Both `q` and `x` must be valid pointers to [ngx_queue_t]. #[inline] pub unsafe fn ngx_queue_insert_before(q: *mut ngx_queue_t, x: *mut ngx_queue_t) { - (*x).prev = (*q).prev; - (*(*x).prev).next = x; - (*x).next = q; - (*q).prev = x; + unsafe { + (*x).prev = (*q).prev; + (*(*x).prev).next = x; + (*x).next = q; + (*q).prev = x; + } } /// Removes a node from the queue. @@ -70,10 +76,12 @@ pub unsafe fn ngx_queue_insert_before(q: *mut ngx_queue_t, x: *mut ngx_queue_t) /// `q` must be a valid pointer to an [ngx_queue_t] node. #[inline] pub unsafe fn ngx_queue_remove(q: *mut ngx_queue_t) { - (*(*q).next).prev = (*q).prev; - (*(*q).prev).next = (*q).next; - (*q).prev = ptr::null_mut(); - (*q).next = ptr::null_mut(); + unsafe { + (*(*q).next).prev = (*q).prev; + (*(*q).prev).next = (*q).next; + (*q).prev = ptr::null_mut(); + (*q).next = ptr::null_mut(); + } } /// Splits a queue at a node, returning the queue tail in a separate queue. @@ -85,12 +93,14 @@ pub unsafe fn ngx_queue_remove(q: *mut ngx_queue_t) { /// `n` must be a valid pointer to [ngx_queue_t]. #[inline] pub unsafe fn ngx_queue_split(h: *mut ngx_queue_t, q: *mut ngx_queue_t, n: *mut ngx_queue_t) { - (*n).prev = (*h).prev; - (*(*n).prev).next = n; - (*n).next = q; - (*h).prev = (*q).prev; - (*(*h).prev).next = h; - (*q).prev = n; + unsafe { + (*n).prev = (*h).prev; + (*(*n).prev).next = n; + (*n).next = q; + (*h).prev = (*q).prev; + (*(*h).prev).next = h; + (*q).prev = n; + } } /// Adds a second queue to the first queue. @@ -101,10 +111,12 @@ pub unsafe fn ngx_queue_split(h: *mut ngx_queue_t, q: *mut ngx_queue_t, n: *mut /// `n` will be left in invalid state, pointing to the subrange of `h` without back references. #[inline] pub unsafe fn ngx_queue_add(h: *mut ngx_queue_t, n: *mut ngx_queue_t) { - (*(*h).prev).next = (*n).next; - (*(*n).next).prev = (*h).prev; - (*h).prev = (*n).prev; - (*(*h).prev).next = h; + unsafe { + (*(*h).prev).next = (*n).next; + (*(*n).next).prev = (*h).prev; + (*h).prev = (*n).prev; + (*(*h).prev).next = h; + } } impl ngx_queue_t { @@ -148,7 +160,9 @@ mod tests { } pub unsafe fn free(x: *mut Self) { - let _ = Box::from_raw(x); + unsafe { + let _ = Box::from_raw(x); + } } } diff --git a/nginx-sys/src/rbtree.rs b/nginx-sys/src/rbtree.rs index 88b94274..799e6fe4 100644 --- a/nginx-sys/src/rbtree.rs +++ b/nginx-sys/src/rbtree.rs @@ -29,10 +29,12 @@ pub unsafe fn ngx_rbtree_init( sentinel: *mut ngx_rbtree_node_t, insert: ngx_rbtree_insert_pt, ) { - ngx_rbtree_sentinel_init(sentinel); - (*tree).root = sentinel; - (*tree).sentinel = sentinel; - (*tree).insert = insert; + unsafe { + ngx_rbtree_sentinel_init(sentinel); + (*tree).root = sentinel; + (*tree).sentinel = sentinel; + (*tree).insert = insert; + } } /// Marks the tree node as red. @@ -42,7 +44,7 @@ pub unsafe fn ngx_rbtree_init( /// `node` must be a valid pointer to a [ngx_rbtree_node_t]. #[inline] pub unsafe fn ngx_rbt_red(node: *mut ngx_rbtree_node_t) { - (*node).color = 1 + unsafe { (*node).color = 1 } } /// Marks the tree node as black. @@ -52,7 +54,7 @@ pub unsafe fn ngx_rbt_red(node: *mut ngx_rbtree_node_t) { /// `node` must be a valid pointer to a [ngx_rbtree_node_t]. #[inline] pub unsafe fn ngx_rbt_black(node: *mut ngx_rbtree_node_t) { - (*node).color = 0 + unsafe { (*node).color = 0 } } /// Initializes the sentinel node. @@ -62,7 +64,7 @@ pub unsafe fn ngx_rbt_black(node: *mut ngx_rbtree_node_t) { /// `node` must be a valid pointer to a [ngx_rbtree_node_t]. #[inline] pub unsafe fn ngx_rbtree_sentinel_init(node: *mut ngx_rbtree_node_t) { - ngx_rbt_black(node) + unsafe { ngx_rbt_black(node) } } /// Returns the least (leftmost) node of the tree. @@ -76,9 +78,11 @@ pub unsafe fn ngx_rbtree_min( mut node: *mut ngx_rbtree_node_t, sentinel: *mut ngx_rbtree_node_t, ) -> *mut ngx_rbtree_node_t { - while !ptr::addr_eq((*node).left, sentinel) { - node = (*node).left; - } + unsafe { + while !ptr::addr_eq((*node).left, sentinel) { + node = (*node).left; + } - node + node + } } diff --git a/nginx-sys/src/string.rs b/nginx-sys/src/string.rs index d0076672..2bffddb1 100644 --- a/nginx-sys/src/string.rs +++ b/nginx-sys/src/string.rs @@ -66,10 +66,12 @@ impl ngx_str_t { /// /// The caller must provide a valid pointer to a memory pool. pub unsafe fn from_bytes(pool: *mut ngx_pool_t, src: &[u8]) -> Option { - detail::bytes_to_uchar(pool, src).map(|data| Self { - data, - len: src.len(), - }) + unsafe { + detail::bytes_to_uchar(pool, src).map(|data| Self { + data, + len: src.len(), + }) + } } /// Create an `ngx_str_t` instance from a string slice (`&str`). @@ -87,9 +89,11 @@ impl ngx_str_t { /// # Returns /// An `ngx_str_t` instance representing the given string slice. pub unsafe fn from_str(pool: *mut ngx_pool_t, data: &str) -> Self { - ngx_str_t { - data: detail::str_to_uchar(pool, data), - len: data.len(), + unsafe { + ngx_str_t { + data: detail::str_to_uchar(pool, data), + len: data.len(), + } } } diff --git a/src/async_/sleep.rs b/src/async_/sleep.rs index 58954905..2f32c4fc 100644 --- a/src/async_/sleep.rs +++ b/src/async_/sleep.rs @@ -123,7 +123,7 @@ impl TimerEvent { unsafe extern "C" fn timer_handler(ev: *mut ngx_event_t) { let timer = ngx_container_of!(ev, Self, event); - if let Some(waker) = (*timer).waker.take() { + if let Some(waker) = unsafe { (*timer).waker.take() } { waker.wake(); } } diff --git a/src/async_/spawn.rs b/src/async_/spawn.rs index dbab031f..c1626b5f 100644 --- a/src/async_/spawn.rs +++ b/src/async_/spawn.rs @@ -85,7 +85,7 @@ impl SchedulerInner { // - the access is unique due to being single-threaded // - the reference is dropped before we start processing queued runnables. let cell: NonNull> = - unsafe { ngx_container_of!(NonNull::new_unchecked(ev), Self, event).cast() }; + ngx_container_of!(unsafe { NonNull::new_unchecked(ev) }, Self, event).cast(); let this = unsafe { &mut *UnsafeCell::raw_get(cell.as_ptr()) }; ngx_log_debug!( diff --git a/src/collections/queue.rs b/src/collections/queue.rs index 757d6d09..b4001d19 100644 --- a/src/collections/queue.rs +++ b/src/collections/queue.rs @@ -95,7 +95,7 @@ where /// `head` is a valid pointer to a list head, and `T::from_queue` on the list entries results in /// valid pointers to `T`. pub unsafe fn from_ptr<'a>(head: *const ngx_queue_t) -> &'a Self { - &*head.cast() + unsafe { &*head.cast() } } /// Creates a mutable queue reference from a pointer to [ngx_queue_t]. @@ -105,7 +105,7 @@ where /// `head` is a valid pointer to a list head, and `T::from_queue` on the list entries results in /// valid pointers to `T`. pub unsafe fn from_ptr_mut<'a>(head: *mut ngx_queue_t) -> &'a mut Self { - &mut *head.cast() + unsafe { &mut *head.cast() } } /// Returns `true` if the queue contains no elements. @@ -351,15 +351,17 @@ impl Queue { /// /// `node` must be an element of this list. unsafe fn remove(&mut self, node: NonNull) -> T { - ngx_queue_remove(node.as_ptr()); - self.len -= 1; - - let entry = QueueEntry::::from_queue(node); - let copy = entry.read(); - // Skip drop as QueueEntry is already copied to `x`. - self.allocator() - .deallocate(entry.cast(), Layout::for_value(entry.as_ref())); - copy.item + unsafe { + ngx_queue_remove(node.as_ptr()); + self.len -= 1; + + let entry = QueueEntry::::from_queue(node); + let copy = entry.read(); + // Skip drop as QueueEntry is already copied to `x`. + self.allocator() + .deallocate(entry.cast(), Layout::for_value(entry.as_ref())); + copy.item + } } } diff --git a/src/collections/rbtree.rs b/src/collections/rbtree.rs index 29462a60..f964009e 100644 --- a/src/collections/rbtree.rs +++ b/src/collections/rbtree.rs @@ -70,7 +70,7 @@ where /// `tree` is a valid pointer to [ngx_rbtree_t], and `T::from_rbtree_node` on the tree nodes /// results in valid pointers to `T`. pub unsafe fn from_ptr<'a>(tree: *const ngx_rbtree_t) -> &'a Self { - &*tree.cast() + unsafe { &*tree.cast() } } /// Creates a mutable tree reference from a pointer to [ngx_rbtree_t]. @@ -80,7 +80,7 @@ where /// `tree` is a valid pointer to [ngx_rbtree_t], and `T::from_rbtree_node` on the tree nodes /// results in valid pointers to `T`. pub unsafe fn from_ptr_mut<'a>(tree: *mut ngx_rbtree_t) -> &'a mut Self { - &mut *tree.cast() + unsafe { &mut *tree.cast() } } /// Returns `true` if the tree contains no elements. diff --git a/src/core/mod.rs b/src/core/mod.rs index ad8264c4..b8dcde7f 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -18,8 +18,8 @@ pub use string::*; /// `$ptr` must be a valid pointer to the field `$field` of `$type`. #[macro_export] macro_rules! ngx_container_of { - ($ptr:expr, $type:path, $field:ident) => { - $ptr.byte_sub(::core::mem::offset_of!($type, $field)) - .cast::<$type>() - }; + ($ptr:expr, $type:path, $field:ident) => {{ + let ptr = $ptr; + unsafe { ptr.byte_sub(::core::mem::offset_of!($type, $field)) }.cast::<$type>() + }}; } diff --git a/src/core/pool.rs b/src/core/pool.rs index 59a41bd0..ae4adc57 100644 --- a/src/core/pool.rs +++ b/src/core/pool.rs @@ -55,16 +55,18 @@ unsafe impl Allocator for Pool { } unsafe fn deallocate(&self, ptr: NonNull, layout: Layout) { - // ngx_pfree is noop for small allocations unless NGX_DEBUG_PALLOC is set. - // - // Note: there should be no cleanup handlers for the allocations made using this API. - // Violating that could result in the following issues: - // - use-after-free on large allocation - // - multiple cleanup handlers attached to a dangling ptr (these are not unique) - if layout.size() > 0 // 0 is dangling ptr + unsafe { + // ngx_pfree is noop for small allocations unless NGX_DEBUG_PALLOC is set. + // + // Note: there should be no cleanup handlers for the allocations made using this API. + // Violating that could result in the following issues: + // - use-after-free on large allocation + // - multiple cleanup handlers attached to a dangling ptr (these are not unique) + if layout.size() > 0 // 0 is dangling ptr && (layout.size() > self.as_ref().max || layout.align() > NGX_ALIGNMENT) - { - ngx_pfree(self.0.as_ptr(), ptr.as_ptr().cast()); + { + ngx_pfree(self.0.as_ptr(), ptr.as_ptr().cast()); + } } } @@ -78,7 +80,7 @@ unsafe impl Allocator for Pool { new_layout.size() >= old_layout.size(), "`new_layout.size()` must be greater than or equal to `old_layout.size()`" ); - self.resize(ptr, old_layout, new_layout) + unsafe { self.resize(ptr, old_layout, new_layout) } } unsafe fn grow_zeroed( @@ -91,16 +93,16 @@ unsafe impl Allocator for Pool { new_layout.size() >= old_layout.size(), "`new_layout.size()` must be greater than or equal to `old_layout.size()`" ); - #[allow(clippy::manual_inspect)] - self.resize(ptr, old_layout, new_layout).map(|new_ptr| { - unsafe { + unsafe { + #[allow(clippy::manual_inspect)] + self.resize(ptr, old_layout, new_layout).map(|new_ptr| { new_ptr .cast::() .byte_add(old_layout.size()) - .write_bytes(0, new_layout.size() - old_layout.size()) - }; - new_ptr - }) + .write_bytes(0, new_layout.size() - old_layout.size()); + new_ptr + }) + } } unsafe fn shrink( @@ -113,7 +115,7 @@ unsafe impl Allocator for Pool { new_layout.size() <= old_layout.size(), "`new_layout.size()` must be smaller than or equal to `old_layout.size()`" ); - self.resize(ptr, old_layout, new_layout) + unsafe { self.resize(ptr, old_layout, new_layout) } } } @@ -140,9 +142,11 @@ impl Pool { /// The caller must ensure that a valid `ngx_pool_t` pointer is provided, pointing to valid /// memory and non-null. A null argument will cause an assertion failure and panic. pub unsafe fn from_ngx_pool(pool: *mut ngx_pool_t) -> Pool { - debug_assert!(!pool.is_null()); - debug_assert!(pool.is_aligned()); - Pool(NonNull::new_unchecked(pool)) + unsafe { + debug_assert!(!pool.is_null()); + debug_assert!(pool.is_aligned()); + Pool(NonNull::new_unchecked(pool)) + } } /// Creates a buffer of the specified size in the memory pool. @@ -205,14 +209,16 @@ impl Pool { /// # Safety /// This function is marked as unsafe because it involves raw pointer manipulation. unsafe fn add_cleanup_for_value(&self, value: *mut T) -> Result<(), ()> { - let cln = ngx_pool_cleanup_add(self.0.as_ptr(), 0); - if cln.is_null() { - return Err(()); - } - (*cln).handler = Some(cleanup_type::); - (*cln).data = value as *mut c_void; + unsafe { + let cln = ngx_pool_cleanup_add(self.0.as_ptr(), 0); + if cln.is_null() { + return Err(()); + } + (*cln).handler = Some(cleanup_type::); + (*cln).data = value as *mut c_void; - Ok(()) + Ok(()) + } } /// Allocates memory from the pool of the specified size. @@ -293,10 +299,11 @@ impl Pool { old_layout: Layout, new_layout: Layout, ) -> Result, AllocError> { - if ptr.byte_add(old_layout.size()).as_ptr() == self.as_ref().d.last - && ptr.byte_add(new_layout.size()).as_ptr() <= self.as_ref().d.end - && ptr.align_offset(new_layout.align()) == 0 - { + if unsafe { + ptr.byte_add(old_layout.size()).as_ptr() == self.as_ref().d.last + && ptr.byte_add(new_layout.size()).as_ptr() <= self.as_ref().d.end + && ptr.align_offset(new_layout.align()) == 0 + } { let pool = self.0.as_ptr(); unsafe { (*pool).d.last = ptr.byte_add(new_layout.size()).as_ptr() }; Ok(NonNull::slice_from_raw_parts(ptr, new_layout.size())) @@ -324,5 +331,7 @@ impl Pool { /// /// * `data` - A raw pointer to the value of type `T` to be cleaned up. unsafe extern "C" fn cleanup_type(data: *mut c_void) { - ptr::drop_in_place(data as *mut T); + unsafe { + ptr::drop_in_place(data as *mut T); + } } diff --git a/src/core/slab.rs b/src/core/slab.rs index 3dadf0d8..cd707b45 100644 --- a/src/core/slab.rs +++ b/src/core/slab.rs @@ -30,7 +30,7 @@ unsafe impl Allocator for SlabPool { #[inline] unsafe fn deallocate(&self, ptr: NonNull, layout: Layout) { - self.lock().deallocate(ptr, layout) + unsafe { self.lock().deallocate(ptr, layout) } } #[inline] @@ -45,7 +45,7 @@ unsafe impl Allocator for SlabPool { old_layout: Layout, new_layout: Layout, ) -> Result, AllocError> { - self.lock().grow(ptr, old_layout, new_layout) + unsafe { self.lock().grow(ptr, old_layout, new_layout) } } #[inline] @@ -55,7 +55,7 @@ unsafe impl Allocator for SlabPool { old_layout: Layout, new_layout: Layout, ) -> Result, AllocError> { - self.lock().grow_zeroed(ptr, old_layout, new_layout) + unsafe { self.lock().grow_zeroed(ptr, old_layout, new_layout) } } #[inline] @@ -65,7 +65,7 @@ unsafe impl Allocator for SlabPool { old_layout: Layout, new_layout: Layout, ) -> Result, AllocError> { - self.lock().shrink(ptr, old_layout, new_layout) + unsafe { self.lock().shrink(ptr, old_layout, new_layout) } } } @@ -140,8 +140,10 @@ unsafe impl Allocator for LockedSlabPool { } unsafe fn deallocate(&self, ptr: NonNull, layout: Layout) { - if layout.size() != 0 { - ngx_slab_free_locked(self.0.as_ptr(), ptr.as_ptr().cast()) + unsafe { + if layout.size() != 0 { + ngx_slab_free_locked(self.0.as_ptr(), ptr.as_ptr().cast()) + } } } } diff --git a/src/core/string.rs b/src/core/string.rs index fb88a435..f0609840 100644 --- a/src/core/string.rs +++ b/src/core/string.rs @@ -43,8 +43,10 @@ impl NgxStr { /// to range of bytes of at least `len` bytes, whose content remains valid and doesn't /// change for the lifetime of the returned `NgxStr`. pub unsafe fn from_ngx_str<'a>(str: ngx_str_t) -> &'a NgxStr { - let bytes: &[u8] = str.as_bytes(); - &*(bytes as *const [u8] as *const NgxStr) + unsafe { + let bytes: &[u8] = str.as_bytes(); + &*(bytes as *const [u8] as *const NgxStr) + } } /// Create an [NgxStr] from a borrowed byte slice. @@ -351,7 +353,7 @@ mod _alloc { capacity: usize, alloc: A, ) -> Self { - Self(Vec::from_raw_parts_in(ptr, length, capacity, alloc)) + unsafe { Self(Vec::from_raw_parts_in(ptr, length, capacity, alloc)) } } /// Splits the NgxString into its raw components. diff --git a/src/http/conf.rs b/src/http/conf.rs index 977d22c3..ca18d4fb 100644 --- a/src/http/conf.rs +++ b/src/http/conf.rs @@ -41,84 +41,100 @@ pub trait HttpModuleConfExt { impl HttpModuleConfExt for crate::ffi::ngx_cycle_t { #[inline] unsafe fn http_main_conf_unchecked(&self, module: &ngx_module_t) -> Option> { - let http_conf = self - .conf_ctx - .add(nginx_sys::ngx_http_module.index) - .as_ref()?; - let conf_ctx = (*http_conf).cast::(); - let conf_ctx = conf_ctx.as_ref()?; - NonNull::new((*conf_ctx.main_conf.add(module.ctx_index)).cast()) + unsafe { + let http_conf = self + .conf_ctx + .add(nginx_sys::ngx_http_module.index) + .as_ref()?; + let conf_ctx = (*http_conf).cast::(); + let conf_ctx = conf_ctx.as_ref()?; + NonNull::new((*conf_ctx.main_conf.add(module.ctx_index)).cast()) + } } } impl HttpModuleConfExt for crate::ffi::ngx_conf_t { #[inline] unsafe fn http_main_conf_unchecked(&self, module: &ngx_module_t) -> Option> { - let conf_ctx = self.ctx.cast::(); - let conf_ctx = conf_ctx.as_ref()?; - NonNull::new((*conf_ctx.main_conf.add(module.ctx_index)).cast()) + unsafe { + let conf_ctx = self.ctx.cast::(); + let conf_ctx = conf_ctx.as_ref()?; + NonNull::new((*conf_ctx.main_conf.add(module.ctx_index)).cast()) + } } #[inline] unsafe fn http_server_conf_unchecked(&self, module: &ngx_module_t) -> Option> { - let conf_ctx = self.ctx.cast::(); - let conf_ctx = conf_ctx.as_ref()?; - NonNull::new((*conf_ctx.srv_conf.add(module.ctx_index)).cast()) + unsafe { + let conf_ctx = self.ctx.cast::(); + let conf_ctx = conf_ctx.as_ref()?; + NonNull::new((*conf_ctx.srv_conf.add(module.ctx_index)).cast()) + } } #[inline] unsafe fn http_location_conf_unchecked(&self, module: &ngx_module_t) -> Option> { - let conf_ctx = self.ctx.cast::(); - let conf_ctx = conf_ctx.as_ref()?; - NonNull::new((*conf_ctx.loc_conf.add(module.ctx_index)).cast()) + unsafe { + let conf_ctx = self.ctx.cast::(); + let conf_ctx = conf_ctx.as_ref()?; + NonNull::new((*conf_ctx.loc_conf.add(module.ctx_index)).cast()) + } } } impl HttpModuleConfExt for ngx_http_core_srv_conf_t { #[inline] unsafe fn http_main_conf_unchecked(&self, module: &ngx_module_t) -> Option> { - let conf_ctx = self.ctx.as_ref()?; - NonNull::new((*conf_ctx.main_conf.add(module.ctx_index)).cast()) + unsafe { + let conf_ctx = self.ctx.as_ref()?; + NonNull::new((*conf_ctx.main_conf.add(module.ctx_index)).cast()) + } } #[inline] unsafe fn http_server_conf_unchecked(&self, module: &ngx_module_t) -> Option> { - let conf_ctx = self.ctx.as_ref()?; - NonNull::new((*conf_ctx.srv_conf.add(module.ctx_index)).cast()) + unsafe { + let conf_ctx = self.ctx.as_ref()?; + NonNull::new((*conf_ctx.srv_conf.add(module.ctx_index)).cast()) + } } #[inline] unsafe fn http_location_conf_unchecked(&self, module: &ngx_module_t) -> Option> { - let conf_ctx = self.ctx.as_ref()?; - NonNull::new((*conf_ctx.loc_conf.add(module.ctx_index)).cast()) + unsafe { + let conf_ctx = self.ctx.as_ref()?; + NonNull::new((*conf_ctx.loc_conf.add(module.ctx_index)).cast()) + } } } impl HttpModuleConfExt for ngx_http_request_t { #[inline] unsafe fn http_main_conf_unchecked(&self, module: &ngx_module_t) -> Option> { - NonNull::new((*self.main_conf.add(module.ctx_index)).cast()) + unsafe { NonNull::new((*self.main_conf.add(module.ctx_index)).cast()) } } #[inline] unsafe fn http_server_conf_unchecked(&self, module: &ngx_module_t) -> Option> { - NonNull::new((*self.srv_conf.add(module.ctx_index)).cast()) + unsafe { NonNull::new((*self.srv_conf.add(module.ctx_index)).cast()) } } #[inline] unsafe fn http_location_conf_unchecked(&self, module: &ngx_module_t) -> Option> { - NonNull::new((*self.loc_conf.add(module.ctx_index)).cast()) + unsafe { NonNull::new((*self.loc_conf.add(module.ctx_index)).cast()) } } } impl HttpModuleConfExt for ngx_http_upstream_srv_conf_t { #[inline] unsafe fn http_server_conf_unchecked(&self, module: &ngx_module_t) -> Option> { - let conf = self.srv_conf; - if conf.is_null() { - return None; + unsafe { + let conf = self.srv_conf; + if conf.is_null() { + return None; + } + NonNull::new((*conf.add(module.ctx_index)).cast()) } - NonNull::new((*conf.add(module.ctx_index)).cast()) } } diff --git a/src/http/module.rs b/src/http/module.rs index 14e019e2..7a9292cc 100644 --- a/src/http/module.rs +++ b/src/http/module.rs @@ -77,8 +77,10 @@ pub trait HttpModule { Self: super::HttpModuleMainConf, Self::MainConf: Default, { - let pool = Pool::from_ngx_pool((*cf).pool); - pool.allocate::(Default::default()) as *mut c_void + unsafe { + let pool = Pool::from_ngx_pool((*cf).pool); + pool.allocate::(Default::default()) as *mut c_void + } } /// # Safety @@ -102,8 +104,10 @@ pub trait HttpModule { Self: super::HttpModuleServerConf, Self::ServerConf: Default, { - let pool = Pool::from_ngx_pool((*cf).pool); - pool.allocate::(Default::default()) as *mut c_void + unsafe { + let pool = Pool::from_ngx_pool((*cf).pool); + pool.allocate::(Default::default()) as *mut c_void + } } /// # Safety @@ -119,11 +123,13 @@ pub trait HttpModule { Self: super::HttpModuleServerConf, Self::ServerConf: Merge, { - let prev = &mut *(prev as *mut Self::ServerConf); - let conf = &mut *(conf as *mut Self::ServerConf); - match conf.merge(prev) { - Ok(_) => ptr::null_mut(), - Err(_) => NGX_CONF_ERROR as _, + unsafe { + let prev = &mut *(prev as *mut Self::ServerConf); + let conf = &mut *(conf as *mut Self::ServerConf); + match conf.merge(prev) { + Ok(_) => ptr::null_mut(), + Err(_) => NGX_CONF_ERROR as _, + } } } @@ -136,8 +142,10 @@ pub trait HttpModule { Self: super::HttpModuleLocationConf, Self::LocationConf: Default, { - let pool = Pool::from_ngx_pool((*cf).pool); - pool.allocate::(Default::default()) as *mut c_void + unsafe { + let pool = Pool::from_ngx_pool((*cf).pool); + pool.allocate::(Default::default()) as *mut c_void + } } /// # Safety @@ -153,11 +161,13 @@ pub trait HttpModule { Self: super::HttpModuleLocationConf, Self::LocationConf: Merge, { - let prev = &mut *(prev as *mut Self::LocationConf); - let conf = &mut *(conf as *mut Self::LocationConf); - match conf.merge(prev) { - Ok(_) => ptr::null_mut(), - Err(_) => NGX_CONF_ERROR as _, + unsafe { + let prev = &mut *(prev as *mut Self::LocationConf); + let conf = &mut *(conf as *mut Self::LocationConf); + match conf.merge(prev) { + Ok(_) => ptr::null_mut(), + Err(_) => NGX_CONF_ERROR as _, + } } } } diff --git a/src/http/request.rs b/src/http/request.rs index 3671717c..a1998ead 100644 --- a/src/http/request.rs +++ b/src/http/request.rs @@ -16,8 +16,8 @@ use crate::http::status::*; macro_rules! http_request_handler { ( $name: ident, $handler: expr ) => { extern "C" fn $name(r: *mut $crate::ffi::ngx_http_request_t) -> $crate::ffi::ngx_int_t { - let status: $crate::core::Status = - $handler(unsafe { &mut $crate::http::Request::from_ngx_http_request(r) }); + let mut request = unsafe { $crate::http::Request::from_ngx_http_request(r) }; + let status: $crate::core::Status = $handler(&mut request); status.0 } }; @@ -52,11 +52,8 @@ macro_rules! http_variable_set { v: *mut $crate::ffi::ngx_variable_value_t, data: usize, ) { - $handler( - unsafe { &mut $crate::http::Request::from_ngx_http_request(r) }, - v, - data, - ); + let mut request = unsafe { $crate::http::Request::from_ngx_http_request(r) }; + $handler(&mut request, v, data); } }; } @@ -75,11 +72,8 @@ macro_rules! http_variable_get { v: *mut $crate::ffi::ngx_variable_value_t, data: usize, ) -> $crate::ffi::ngx_int_t { - let status: $crate::core::Status = $handler( - unsafe { &mut $crate::http::Request::from_ngx_http_request(r) }, - v, - data, - ); + let mut request = unsafe { $crate::http::Request::from_ngx_http_request(r) }; + let status: $crate::core::Status = $handler(&mut request, v, data); status.0 } }; @@ -124,7 +118,7 @@ impl Request { /// The caller has provided a valid non-null pointer to a valid `ngx_http_request_t` /// which shares the same representation as `Request`. pub unsafe fn from_ngx_http_request<'a>(r: *mut ngx_http_request_t) -> &'a mut Request { - &mut *r.cast::() + unsafe { &mut *r.cast::() } } /// Is this the main request (as opposed to a subrequest)? @@ -390,23 +384,29 @@ impl Request { impl crate::http::HttpModuleConfExt for Request { #[inline] unsafe fn http_main_conf_unchecked(&self, module: &ngx_module_t) -> Option> { - // SAFETY: main_conf[module.ctx_index] is either NULL or allocated with ngx_p(c)alloc and - // explicitly initialized by the module - NonNull::new((*self.0.main_conf.add(module.ctx_index)).cast()) + unsafe { + // SAFETY: main_conf[module.ctx_index] is either NULL or allocated with ngx_p(c)alloc and + // explicitly initialized by the module + NonNull::new((*self.0.main_conf.add(module.ctx_index)).cast()) + } } #[inline] unsafe fn http_server_conf_unchecked(&self, module: &ngx_module_t) -> Option> { - // SAFETY: srv_conf[module.ctx_index] is either NULL or allocated with ngx_p(c)alloc and - // explicitly initialized by the module - NonNull::new((*self.0.srv_conf.add(module.ctx_index)).cast()) + unsafe { + // SAFETY: srv_conf[module.ctx_index] is either NULL or allocated with ngx_p(c)alloc and + // explicitly initialized by the module + NonNull::new((*self.0.srv_conf.add(module.ctx_index)).cast()) + } } #[inline] unsafe fn http_location_conf_unchecked(&self, module: &ngx_module_t) -> Option> { - // SAFETY: loc_conf[module.ctx_index] is either NULL or allocated with ngx_p(c)alloc and - // explicitly initialized by the module - NonNull::new((*self.0.loc_conf.add(module.ctx_index)).cast()) + unsafe { + // SAFETY: loc_conf[module.ctx_index] is either NULL or allocated with ngx_p(c)alloc and + // explicitly initialized by the module + NonNull::new((*self.0.loc_conf.add(module.ctx_index)).cast()) + } } } diff --git a/src/http/upstream.rs b/src/http/upstream.rs index 22a0e7ed..9b659c16 100644 --- a/src/http/upstream.rs +++ b/src/http/upstream.rs @@ -16,10 +16,8 @@ macro_rules! http_upstream_init_peer_pt { r: *mut $crate::ffi::ngx_http_request_t, us: *mut $crate::ffi::ngx_http_upstream_srv_conf_t, ) -> $crate::ffi::ngx_int_t { - let status: $crate::core::Status = $handler( - unsafe { &mut $crate::http::Request::from_ngx_http_request(r) }, - us, - ); + let mut request = unsafe { $crate::http::Request::from_ngx_http_request(r) }; + let status: $crate::core::Status = $handler(&mut request, us); status.0 } };