From 0cec0b9133cfe64272c7d0740a66f01cf8ba2e44 Mon Sep 17 00:00:00 2001 From: "Y.Horie" Date: Mon, 25 May 2026 08:15:29 +0900 Subject: [PATCH] feat(http): expose upstream_states and request timing/byte accessors Add safe accessors for request struct fields that previously required raw-pointer deref: - `Request::upstream_states() -> &[UpstreamState]` walks each upstream attempt (one per peer for `proxy_next_upstream` retries) via a `#[repr(transparent)]` wrapper exposing `peer`, `status`, `response_time`, `connect_time`, `header_time`, `queue_time`, `bytes_sent`, `bytes_received`, `response_length`. - `Request::start_sec()`, `start_msec()`, `request_length()`, `bytes_sent()` for the request-level timing/byte fields. `Request::upstream()` (the raw-pointer accessor) is left untouched to preserve compatibility. Unit tests follow the `MaybeUninit::zeroed` pattern from #272. Signed-off-by: Y.Horie --- src/http/mod.rs | 1 + src/http/request.rs | 157 +++++++++++++++++++++++++++++++++++++++ src/http/upstream.rs | 171 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 329 insertions(+) diff --git a/src/http/mod.rs b/src/http/mod.rs index 00c329a8..024ad1f6 100644 --- a/src/http/mod.rs +++ b/src/http/mod.rs @@ -8,3 +8,4 @@ pub use conf::*; pub use module::*; pub use request::*; pub use status::*; +pub use upstream::*; diff --git a/src/http/request.rs b/src/http/request.rs index 64d7e6e8..b01cd857 100644 --- a/src/http/request.rs +++ b/src/http/request.rs @@ -9,6 +9,7 @@ use crate::core::*; use crate::ffi::*; use crate::http::HttpPhase; use crate::http::status::*; +use crate::http::upstream::UpstreamState; /// Define a static request handler. /// @@ -221,6 +222,65 @@ impl Request { Some(self.0.upstream) } + /// Per-attempt upstream states recorded for this request. Empty + /// when no upstream was contacted (no `proxy_pass` / + /// `fastcgi_pass` / etc., or a cache hit served directly from + /// disk). + /// + /// One entry per attempt in chronological order. A request that + /// exercises `proxy_next_upstream` yields multiple entries (e.g. + /// the failed peer, then the successful one); a request that + /// succeeds on the first try yields exactly one entry — which is + /// the same `ngx_http_upstream_state_t` that `u->state` points + /// at. + pub fn upstream_states(&self) -> &[UpstreamState] { + let arr = self.0.upstream_states; + if arr.is_null() { + return &[]; + } + // SAFETY: `upstream_states`, when non-null, points to an + // `ngx_array_t` allocated in the request pool, valid for the + // lifetime of the request. + let raw = unsafe { &*arr }; + if raw.nelts == 0 { + return &[]; + } + // SAFETY: `UpstreamState` is `#[repr(transparent)]` over + // `ngx_http_upstream_state_t`, so the layouts are identical + // and the slice cast is sound. + unsafe { slice::from_raw_parts(raw.elts.cast::(), raw.nelts) } + } + + /// Wall-clock seconds (since the epoch) at which nginx began + /// processing this request. Pair with [`Request::start_msec`] + /// to compute request latency. + pub fn start_sec(&self) -> time_t { + self.0.start_sec + } + + /// Milliseconds-since-second component of the request start + /// time, complementing [`Request::start_sec`]. + pub fn start_msec(&self) -> ngx_msec_t { + self.0.start_msec as ngx_msec_t + } + + /// Bytes received from the client for this request, including + /// the request line and headers. Useful as the source label + /// for `ingress` byte counters. + pub fn request_length(&self) -> off_t { + self.0.request_length + } + + /// Bytes sent to the client on this request's connection. + /// Reads `r->connection->sent`, the byte total nginx itself uses + /// to render `$bytes_sent` in `log_format`. + pub fn bytes_sent(&self) -> off_t { + // SAFETY: `connection` is non-null for the lifetime of a + // valid `Request` (set by nginx core in + // `ngx_http_create_request`). + unsafe { (*self.0.connection).sent } + } + /// Pointer to a [`ngx_connection_t`] client connection object. /// /// [`ngx_connection_t`]: https://nginx.org/en/docs/dev/development_guide.html#connection @@ -800,3 +860,100 @@ enum MethodInner { Trace, Connect, } + +#[cfg(test)] +mod tests { + use core::mem::MaybeUninit; + + extern crate alloc; + use alloc::vec::Vec; + + use super::*; + use crate::ffi::{ngx_array_t, ngx_connection_t, ngx_http_upstream_state_t}; + + fn zeroed_request() -> ngx_http_request_t { + // SAFETY: `ngx_http_request_t` is a `#[repr(C)]` aggregate of + // pointers and scalars for which the all-zero bit pattern is + // a defined (if mostly meaningless) value. Tests only call + // accessors that read fields populated below. + unsafe { MaybeUninit::zeroed().assume_init() } + } + + fn request_from(r: &mut ngx_http_request_t) -> &mut Request { + // SAFETY: `Request` is `#[repr(transparent)]` over + // `ngx_http_request_t`, so the reference is sound. + unsafe { Request::from_ngx_http_request(r) } + } + + #[test] + fn upstream_states_empty_when_array_null() { + let mut r = zeroed_request(); + // upstream_states is already null after zero-init. + let req = request_from(&mut r); + assert!(req.upstream_states().is_empty()); + } + + #[test] + fn upstream_states_empty_when_nelts_zero() { + let mut arr: ngx_array_t = unsafe { MaybeUninit::zeroed().assume_init() }; + // `nelts == 0` should be reported as empty even if `elts` + // happens to be non-null (nginx leaves it pointing at the + // initial allocation). + arr.elts = (&raw mut arr).cast(); + arr.nelts = 0; + + let mut r = zeroed_request(); + r.upstream_states = &raw mut arr; + let req = request_from(&mut r); + assert!(req.upstream_states().is_empty()); + } + + #[test] + fn upstream_states_returns_each_attempt_in_order() { + // Two upstream attempts: a 502 followed by a 200, mirroring a + // `proxy_next_upstream` retry. + let mut states: [ngx_http_upstream_state_t; 2] = + unsafe { MaybeUninit::zeroed().assume_init() }; + states[0].status = 502; + states[0].response_time = 3; + states[1].status = 200; + states[1].response_time = 12; + + let mut arr: ngx_array_t = unsafe { MaybeUninit::zeroed().assume_init() }; + arr.elts = states.as_mut_ptr().cast(); + arr.nelts = states.len(); + + let mut r = zeroed_request(); + r.upstream_states = &raw mut arr; + let req = request_from(&mut r); + + let collected: Vec<(u16, u64)> = + req.upstream_states().iter().map(|s| (s.status(), s.response_time() as u64)).collect(); + let expected: Vec<(u16, u64)> = [(502u16, 3u64), (200, 12)].into_iter().collect(); + assert_eq!(collected, expected); + } + + #[test] + fn timing_and_length_accessors_read_underlying_fields() { + let mut r = zeroed_request(); + r.start_sec = 1_700_000_000; + r.start_msec = 250; + r.request_length = 4096; + + let req = request_from(&mut r); + assert_eq!(req.start_sec(), 1_700_000_000); + assert_eq!(req.start_msec(), 250); + assert_eq!(req.request_length(), 4096); + } + + #[test] + fn bytes_sent_reads_through_connection() { + let mut conn: ngx_connection_t = unsafe { MaybeUninit::zeroed().assume_init() }; + conn.sent = 8192; + let mut r = zeroed_request(); + r.connection = &raw mut conn; + + let req = request_from(&mut r); + assert_eq!(req.bytes_sent(), 8192); + } +} diff --git a/src/http/upstream.rs b/src/http/upstream.rs index cfd39864..20b6d526 100644 --- a/src/http/upstream.rs +++ b/src/http/upstream.rs @@ -1,3 +1,91 @@ +use crate::core::NgxStr; +use crate::ffi::{ngx_http_upstream_state_t, ngx_msec_t, off_t}; + +/// Per-attempt state nginx records for each upstream contact a +/// request makes. A request that succeeds on the first try has +/// exactly one entry; a request that exercises `proxy_next_upstream` +/// has one entry per attempt (e.g. failed peer, then successful +/// peer) in chronological order. +/// +/// Yielded by [`crate::http::Request::upstream_states`]. Each +/// instance borrows from the underlying `ngx_http_request_t`'s pool +/// and is valid for the lifetime of the request. +/// +/// See +/// for the per-attempt fields nginx tracks. +#[repr(transparent)] +pub struct UpstreamState(ngx_http_upstream_state_t); + +impl UpstreamState { + /// Peer address contacted for this attempt (typically `host:port` + /// or `unix:/path`), or `None` when no peer was ever selected. + /// + /// `None` is the cache-HIT path where `r->upstream` exists (nginx + /// uses the upstream framework to consult the cache) but no + /// backend was contacted, plus init-time slots before peer + /// selection in failure cases. + pub fn peer(&self) -> Option<&NgxStr> { + if self.0.peer.is_null() { + return None; + } + // SAFETY: `peer` points to an `ngx_str_t` owned by the request + // pool when non-null; it lives at least as long as `&self`. + let s = unsafe { *self.0.peer }; + if s.len == 0 { + return None; + } + Some(unsafe { NgxStr::from_ngx_str(s) }) + } + + /// HTTP status returned by the peer. Zero when the attempt + /// failed before any response was received (e.g. connect error, + /// timeout). + pub fn status(&self) -> u16 { + self.0.status as u16 + } + + /// Total time (in milliseconds) spent on this attempt, from + /// connect through the last byte of the response. + pub fn response_time(&self) -> ngx_msec_t { + self.0.response_time + } + + /// Time (in milliseconds) the connect() syscall took. Useful + /// for distinguishing connection setup latency from full + /// response latency. + pub fn connect_time(&self) -> ngx_msec_t { + self.0.connect_time + } + + /// Time (in milliseconds) waiting for the peer to start sending + /// the response header. + pub fn header_time(&self) -> ngx_msec_t { + self.0.header_time + } + + /// Time (in milliseconds) the request spent queued before nginx + /// dispatched it to this peer. + pub fn queue_time(&self) -> ngx_msec_t { + self.0.queue_time + } + + /// Bytes sent to the peer (the proxied request body). + pub fn bytes_sent(&self) -> off_t { + self.0.bytes_sent + } + + /// Bytes received from the peer (the response, including + /// headers). + pub fn bytes_received(&self) -> off_t { + self.0.bytes_received + } + + /// Length of the response body advertised by the peer. + pub fn response_length(&self) -> off_t { + self.0.response_length + } +} + /// Define a static upstream peer initializer /// /// Initializes the upstream 'get', 'free', and 'session' callbacks and gives the module writer an @@ -22,3 +110,86 @@ macro_rules! http_upstream_init_peer_pt { } }; } + +#[cfg(test)] +mod tests { + use core::mem::MaybeUninit; + + use super::*; + use crate::ffi::ngx_str_t; + + /// Build a zero-initialised `UpstreamState` for tests. Real + /// nginx allocates these from the request pool; here we just + /// need the bytes to be zero so each accessor sees a defined + /// starting point. + fn zeroed_state() -> UpstreamState { + // SAFETY: `ngx_http_upstream_state_t` is `#[repr(C)]` and + // every field is a scalar or raw pointer for which all-zero + // bytes is a valid (and defined) value. + unsafe { MaybeUninit::zeroed().assume_init() } + } + + #[test] + fn peer_none_when_pointer_null() { + let state = zeroed_state(); + assert!(state.peer().is_none()); + } + + #[test] + fn peer_none_when_str_empty() { + // The pointer is non-null but the `ngx_str_t` it targets + // is the zero-length sentinel cache lookups leave behind. + let empty: ngx_str_t = unsafe { MaybeUninit::zeroed().assume_init() }; + let mut state = zeroed_state(); + state.0.peer = (&raw const empty).cast_mut(); + assert!(state.peer().is_none()); + } + + #[test] + fn peer_some_when_populated() { + let bytes = b"10.0.0.1:8080"; + let mut peer = ngx_str_t { len: bytes.len(), data: bytes.as_ptr().cast_mut() }; + let mut state = zeroed_state(); + state.0.peer = &raw mut peer; + + let got = state.peer().expect("peer should resolve"); + assert_eq!(got.as_bytes(), bytes); + } + + #[test] + fn accessors_read_underlying_fields() { + let mut state = zeroed_state(); + state.0.status = 502; + state.0.response_time = 75; + state.0.connect_time = 5; + state.0.header_time = 10; + state.0.queue_time = 1; + state.0.bytes_sent = 1024; + state.0.bytes_received = 4096; + state.0.response_length = 2048; + + assert_eq!(state.status(), 502); + assert_eq!(state.response_time(), 75); + assert_eq!(state.connect_time(), 5); + assert_eq!(state.header_time(), 10); + assert_eq!(state.queue_time(), 1); + assert_eq!(state.bytes_sent(), 1024); + assert_eq!(state.bytes_received(), 4096); + assert_eq!(state.response_length(), 2048); + } + + #[test] + fn upstream_state_size_matches_underlying_struct() { + // Guards the slice cast in `Request::upstream_states`: the + // `#[repr(transparent)]` newtype must have the same layout + // as the raw nginx struct so the slice elements line up. + assert_eq!( + core::mem::size_of::(), + core::mem::size_of::(), + ); + assert_eq!( + core::mem::align_of::(), + core::mem::align_of::(), + ); + } +}