From 8f5998f48f364efb35910fa53c7f364316b987f2 Mon Sep 17 00:00:00 2001 From: Aleksei Bavshin Date: Thu, 5 Jun 2025 18:34:41 -0700 Subject: [PATCH 1/4] fix: correct return type for nginx configuration handlers Change ngx::core::NGX_CONF_ERROR to *mut c_char and ensure that the pointer has no provenance in a way that is compatible with the current MSRV (1.79.0). Add ngx::core::NGX_CONF_OK constant for completeness. --- examples/async.rs | 2 +- examples/awssig.rs | 13 +++++++------ examples/curl.rs | 2 +- examples/upstream.rs | 6 +++--- src/core/status.rs | 9 ++++++--- 5 files changed, 18 insertions(+), 14 deletions(-) diff --git a/examples/async.rs b/examples/async.rs index 43ce3db5..f46f20ed 100644 --- a/examples/async.rs +++ b/examples/async.rs @@ -218,7 +218,7 @@ extern "C" fn ngx_http_async_commands_set_enable( } }; - std::ptr::null_mut() + ngx::core::NGX_CONF_OK } fn ngx_http_async_runtime() -> &'static Runtime { diff --git a/examples/awssig.rs b/examples/awssig.rs index 384f98d2..7a93e10c 100644 --- a/examples/awssig.rs +++ b/examples/awssig.rs @@ -188,7 +188,7 @@ extern "C" fn ngx_http_awssigv4_commands_set_enable( } }; - std::ptr::null_mut() + ngx::core::NGX_CONF_OK } extern "C" fn ngx_http_awssigv4_commands_set_access_key( @@ -202,7 +202,7 @@ extern "C" fn ngx_http_awssigv4_commands_set_access_key( conf.access_key = (*args.add(1)).to_string(); }; - std::ptr::null_mut() + ngx::core::NGX_CONF_OK } extern "C" fn ngx_http_awssigv4_commands_set_secret_key( @@ -216,7 +216,7 @@ extern "C" fn ngx_http_awssigv4_commands_set_secret_key( conf.secret_key = (*args.add(1)).to_string(); }; - std::ptr::null_mut() + ngx::core::NGX_CONF_OK } extern "C" fn ngx_http_awssigv4_commands_set_s3_bucket( @@ -230,10 +230,11 @@ extern "C" fn ngx_http_awssigv4_commands_set_s3_bucket( conf.s3_bucket = (*args.add(1)).to_string(); if conf.s3_bucket.len() == 1 { println!("Validation failed"); - return ngx::core::NGX_CONF_ERROR as _; + return ngx::core::NGX_CONF_ERROR; } }; - std::ptr::null_mut() + + ngx::core::NGX_CONF_OK } extern "C" fn ngx_http_awssigv4_commands_set_s3_endpoint( @@ -247,7 +248,7 @@ extern "C" fn ngx_http_awssigv4_commands_set_s3_endpoint( conf.s3_endpoint = (*args.add(1)).to_string(); }; - std::ptr::null_mut() + ngx::core::NGX_CONF_OK } http_request_handler!(awssigv4_header_handler, |request: &mut Request| { diff --git a/examples/curl.rs b/examples/curl.rs index c8d9d2c2..da7c9dad 100644 --- a/examples/curl.rs +++ b/examples/curl.rs @@ -131,5 +131,5 @@ extern "C" fn ngx_http_curl_commands_set_enable( } }; - std::ptr::null_mut() + ngx::core::NGX_CONF_OK } diff --git a/examples/upstream.rs b/examples/upstream.rs index b9286726..7517ece2 100644 --- a/examples/upstream.rs +++ b/examples/upstream.rs @@ -282,7 +282,7 @@ unsafe extern "C" fn ngx_http_upstream_commands_set_custom( value[1], &(*cmd).name ); - return usize::MAX as *mut c_char; + return ngx::core::NGX_CONF_ERROR; } ccf.max = n as u32; } @@ -298,8 +298,8 @@ unsafe extern "C" fn ngx_http_upstream_commands_set_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_CONF_OK - std::ptr::null_mut() + + ngx::core::NGX_CONF_OK } // The upstream module. diff --git a/src/core/status.rs b/src/core/status.rs index 46bb79f7..4cab8844 100644 --- a/src/core/status.rs +++ b/src/core/status.rs @@ -1,4 +1,6 @@ +use core::ffi::c_char; use core::fmt; +use core::ptr; use crate::ffi::*; @@ -62,6 +64,7 @@ ngx_codes! { (NGX_ABORT); } -/// NGX_CONF_ERROR - An error occurred while parsing and validating configuration. -pub const NGX_CONF_ERROR: *const () = -1isize as *const (); -// pub const CONF_OK: Status = Status(NGX_CONF_OK as ngx_int_t); +/// An error occurred while parsing and validating configuration. +pub const NGX_CONF_ERROR: *mut c_char = ptr::null_mut::().wrapping_offset(-1); +/// Configuration handler succeeded. +pub const NGX_CONF_OK: *mut c_char = ptr::null_mut(); From 0b9a2ba353f831d8fb0b091031394718bed9367f Mon Sep 17 00:00:00 2001 From: Aleksei Bavshin Date: Sat, 11 Jan 2025 22:59:51 -0800 Subject: [PATCH 2/4] feat: enforce minimal required alignment for pool allocations Refuse to compile if the NGX_ALIGNMENT value detected or specified while building nginx is insufficient. --- nginx-sys/build/main.rs | 2 ++ nginx-sys/build/wrapper.h | 4 ++++ nginx-sys/src/lib.rs | 8 ++++++++ 3 files changed, 14 insertions(+) diff --git a/nginx-sys/build/main.rs b/nginx-sys/build/main.rs index 22208dd7..a657276d 100644 --- a/nginx-sys/build/main.rs +++ b/nginx-sys/build/main.rs @@ -216,6 +216,8 @@ fn generate_binding(nginx: &NginxSource) { // Bindings will not compile on Linux without block listing this item // It is worth investigating why this is .blocklist_item("IPPORT_RESERVED") + // will be restored later in build.rs + .blocklist_item("NGX_ALIGNMENT") .generate_cstr(true) // The input header we would like to generate bindings for. .header("build/wrapper.h") diff --git a/nginx-sys/build/wrapper.h b/nginx-sys/build/wrapper.h index c333744f..552a793b 100644 --- a/nginx-sys/build/wrapper.h +++ b/nginx-sys/build/wrapper.h @@ -20,6 +20,10 @@ const char *NGX_RS_MODULE_SIGNATURE = NGX_MODULE_SIGNATURE; +// NGX_ALIGNMENT could be defined as a constant or an expression, with the +// latter being unsupported by bindgen. +const size_t NGX_RS_ALIGNMENT = NGX_ALIGNMENT; + // `--prefix=` results in not emitting the declaration #ifndef NGX_PREFIX #define NGX_PREFIX "" diff --git a/nginx-sys/src/lib.rs b/nginx-sys/src/lib.rs index 83a458c4..ffb7eddf 100644 --- a/nginx-sys/src/lib.rs +++ b/nginx-sys/src/lib.rs @@ -36,6 +36,14 @@ pub use queue::*; #[cfg(ngx_feature = "stream")] pub use stream::*; +/// Default alignment for pool allocations. +pub const NGX_ALIGNMENT: usize = NGX_RS_ALIGNMENT; + +// Check if the allocations made with ngx_palloc are properly aligned. +// If the check fails, objects allocated from `ngx_pool` can violate Rust pointer alignment +// requirements. +const _: () = assert!(core::mem::align_of::() <= NGX_ALIGNMENT); + impl ngx_command_t { /// Creates a new empty [`ngx_command_t`] instance. /// From afada25f5ac010f8e1c44dbe6245f4961e6ce71f Mon Sep 17 00:00:00 2001 From: Aleksei Bavshin Date: Tue, 10 Jun 2025 14:40:47 -0700 Subject: [PATCH 3/4] chore: address compiler warnings - mismatched_lifetime_syntaxes - clippy::uninlined_format_args --- nginx-sys/build/main.rs | 13 +++++-------- src/core/string.rs | 2 +- src/http/request.rs | 6 +++--- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/nginx-sys/build/main.rs b/nginx-sys/build/main.rs index a657276d..f2dc1afe 100644 --- a/nginx-sys/build/main.rs +++ b/nginx-sys/build/main.rs @@ -341,22 +341,19 @@ pub fn print_cargo_metadata>(includes: &[T]) -> Result<(), Box Cow { + pub fn to_string_lossy(&self) -> Cow<'_, str> { String::from_utf8_lossy(self.as_bytes()) } diff --git a/src/http/request.rs b/src/http/request.rs index c3a21556..d17d294c 100644 --- a/src/http/request.rs +++ b/src/http/request.rs @@ -375,13 +375,13 @@ impl Request { /// Iterate over headers_in /// each header item is (&str, &str) (borrowed) - pub fn headers_in_iterator(&self) -> NgxListIterator { + pub fn headers_in_iterator(&self) -> NgxListIterator<'_> { unsafe { list_iterator(&self.0.headers_in.headers) } } /// Iterate over headers_out /// each header item is (&str, &str) (borrowed) - pub fn headers_out_iterator(&self) -> NgxListIterator { + pub fn headers_out_iterator(&self) -> NgxListIterator<'_> { unsafe { list_iterator(&self.0.headers_out.headers) } } } @@ -448,7 +448,7 @@ impl<'a> From<&'a ngx_list_part_t> for ListPart<'a> { /// # Safety /// /// The caller has provided a valid [`ngx_str_t`] which can be dereferenced validly. -pub unsafe fn list_iterator(list: &ngx_list_t) -> NgxListIterator { +pub unsafe fn list_iterator(list: &ngx_list_t) -> NgxListIterator<'_> { NgxListIterator { part: Some((&list.part).into()), i: 0, From 77f5b0aa5a3cd879c39bba109a5a2ed405cc4b48 Mon Sep 17 00:00:00 2001 From: Aleksei Bavshin Date: Thu, 5 Jun 2025 16:37:09 -0700 Subject: [PATCH 4/4] ci: disable fail-fast for Rust/Test (Linux) jobs This allows to complete the linting and format checks on MSRV failure. --- .github/workflows/ci.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index cb15d608..5cb4130e 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -27,6 +27,7 @@ jobs: needs: rust-version runs-on: ubuntu-latest strategy: + fail-fast: false matrix: rust-version: - ${{ needs.rust-version.outputs.version }}