Middleware perf tuning (#167)

* middleware performance tuning + security & safety docs

* zero-alloc state machine for next fn

* updated CHANGELOG.md

Author: RomanEmreis

CHANGELOG.md

@@ -5,11 +5,18 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/).
 
-## Unreleased
+## 0.8.6
+
+### Added
+* fuzz tests for router and OpenAPI (#166)
 
 ## Changed
-* Refactored directory listing HTML generation.
-* Removed dependencies on `handlebars` and `chrono`
+* Added security notes for tap_req middleware (#167)
+* Added safety notes for wrap middleware (#167)
+* Improved performance of the entire middleware pipeline, reducing heap allocations (#167)
+* Unused Next/NextFn are now zero-alloc (#167)
+* Refactored directory listing HTML generation. (#165)
+* Removed dependencies on `handlebars` and `chrono` (#165)
 
 ## 0.8.5
 

volga/src/app/pipeline.rs

@@ -130,9 +130,7 @@ impl Pipeline {
 
     #[cfg(feature = "middleware")]
     pub(crate) async fn execute(&self, ctx: HttpContext) -> HttpResult {
-        let next = &self.start;
-        if let Some(next) = next {
-            let next: NextFn = next.clone();
+        if let Some(next) = &self.start {
             next(ctx).await
         } else {
             ctx.execute().await

volga/src/middleware.rs

@@ -100,9 +100,9 @@ impl Middlewares {
         let last = iter.next()?;
         let mut next: NextFn = {
             let handler = last.clone();
-            // Call the last middleware, ignoring its `next` argument with an empty placeholder
-            Arc::new(move |ctx| 
-                handler(ctx, Arc::new(|_| Box::pin(async { not_found!() }))))
+            // Allocate the placeholder once at compose time, not per-request
+            let dummy: NextFn = Arc::new(|_| Box::pin(async { not_found!() }));
+            Arc::new(move |ctx| handler(ctx, dummy.clone()))
         };
 
         for mw in iter {
@@ -133,6 +133,31 @@ impl App {
     ///# app.run().await
     ///# }
     /// ```
+    ///
+    /// # Timeouts
+    ///
+    /// The pipeline does not enforce per-request timeouts. If your middleware
+    /// performs a long-running or potentially unbounded operation, check the
+    /// [`CancellationToken`](crate::CancellationToken) injected into each
+    /// request's extensions to avoid holding connections open indefinitely:
+    ///
+    /// ```no_run
+    /// use volga::{App, CancellationToken, error::Error};
+    ///
+    ///# #[tokio::main]
+    ///# async fn main() -> std::io::Result<()> {
+    /// let mut app = App::new();
+    ///
+    /// app.wrap(|ctx, next| async move {
+    ///     let token = ctx.extract::<CancellationToken>()?;
+    ///     tokio::select! {
+    ///         res = next(ctx) => res,
+    ///         _ = token.cancelled() => Err(Error::server_error("request cancelled")),
+    ///     }
+    /// });
+    ///# app.run().await
+    ///# }
+    /// ```
     pub fn wrap<F, Fut>(&mut self, middleware: F) -> &mut Self
     where
         F: Fn(HttpContext, NextFn) -> Fut + Send + Sync + 'static,
@@ -213,6 +238,14 @@ impl App {
     ///# app.run().await
     ///# }
     /// ```
+    ///
+    /// # Security
+    ///
+    /// `tap_req` grants full mutable ownership of the incoming request, including
+    /// all headers. Security-critical values such as `Authorization` can be
+    /// stripped or overwritten before downstream middleware and handlers observe
+    /// them. Only register trusted closures and be mindful that registration order
+    /// determines which code sees the original request.
     #[cfg(feature = "di")]
     pub fn tap_req<F, Args, R>(&mut self, map: F) -> &mut Self
     where
@@ -259,6 +292,14 @@ impl App {
     ///# app.run().await
     ///# }
     /// ```
+    ///
+    /// # Security
+    ///
+    /// `tap_req` grants full mutable ownership of the incoming request, including
+    /// all headers. Security-critical values such as `Authorization` can be
+    /// stripped or overwritten before downstream middleware and handlers observe
+    /// them. Only register trusted closures and be mindful that registration order
+    /// determines which code sees the original request.
     #[cfg(not(feature = "di"))]
     pub fn tap_req<F, R>(&mut self, map: F) -> &mut Self
     where
@@ -469,6 +510,14 @@ impl<'a> Route<'a> {
     ///# app.run().await
     ///# }
     /// ```
+    ///
+    /// # Security
+    ///
+    /// `tap_req` grants full mutable ownership of the incoming request, including
+    /// all headers. Security-critical values such as `Authorization` can be
+    /// stripped or overwritten before downstream middleware and handlers observe
+    /// them. Only register trusted closures and be mindful that registration order
+    /// determines which code sees the original request.
     #[cfg(feature = "di")]
     pub fn tap_req<F, Args, R>(self, map: F) -> Self
     where
@@ -513,6 +562,14 @@ impl<'a> Route<'a> {
     ///# app.run().await
     ///# }
     /// ```
+    ///
+    /// # Security
+    ///
+    /// `tap_req` grants full mutable ownership of the incoming request, including
+    /// all headers. Security-critical values such as `Authorization` can be
+    /// stripped or overwritten before downstream middleware and handlers observe
+    /// them. Only register trusted closures and be mindful that registration order
+    /// determines which code sees the original request.
     #[cfg(not(feature = "di"))]
     pub fn tap_req<F, R>(self, map: F) -> Self
     where
@@ -729,15 +786,23 @@ impl<'a> RouteGroup<'a> {
     ///# app.run().await
     ///# }
     /// ```
+    ///
+    /// # Security
+    ///
+    /// `tap_req` grants full mutable ownership of the incoming request, including
+    /// all headers. Security-critical values such as `Authorization` can be
+    /// stripped or overwritten before downstream middleware and handlers observe
+    /// them. Only register trusted closures and be mindful that registration order
+    /// determines which code sees the original request.
     #[cfg(feature = "di")]
     pub fn tap_req<F, Args, R>(&mut self, map: F) -> &mut Self
     where
         F: TapReqHandler<Args, Output = R>,
         R: IntoTapResult,
         Args: FromContainer + Send + 'static,
     {
-        let map_err_fn = make_tap_req_fn(map);
-        self.middleware.push(map_err_fn);
+        let tap_req_fn = make_tap_req_fn(map);
+        self.middleware.push(tap_req_fn);
         self
     }
 
@@ -776,14 +841,22 @@ impl<'a> RouteGroup<'a> {
     ///# app.run().await
     ///# }
     /// ```
+    ///
+    /// # Security
+    ///
+    /// `tap_req` grants full mutable ownership of the incoming request, including
+    /// all headers. Security-critical values such as `Authorization` can be
+    /// stripped or overwritten before downstream middleware and handlers observe
+    /// them. Only register trusted closures and be mindful that registration order
+    /// determines which code sees the original request.
     #[cfg(not(feature = "di"))]
     pub fn tap_req<F, R>(&mut self, map: F) -> &mut Self
     where
         F: TapReqHandler<Output = R>,
         R: IntoTapResult,
     {
-        let map_err_fn = make_tap_req_fn(map);
-        self.middleware.push(map_err_fn);
+        let tap_req_fn = make_tap_req_fn(map);
+        self.middleware.push(tap_req_fn);
         self
     }
 

volga/src/middleware/handler.rs

@@ -1,5 +1,6 @@
 //! Extractors for middleware functions
 
+use futures_util::future::BoxFuture;
 use std::future::Future;
 use std::pin::Pin;
 use std::task::{Context, Poll};
@@ -8,9 +9,23 @@ use crate::error::Error;
 use crate::{HttpRequestMut, HttpResponse, HttpResult};
 use super::{HttpContext, NextFn};
 
+/// Internal state machine for [`Next`]
+///
+/// `Pending` is intentionally large: `HttpContext` lives here until the first
+/// poll, avoiding the heap allocation that would be required to box it.
+/// Both variants reside inside the already heap-allocated [`Next`] future,
+/// so this does not create stack pressure.
+#[allow(clippy::large_enum_variant)]
+enum NextState {
+    /// Not yet polled; the inner future is created on demand
+    Pending(HttpContext, NextFn),
+    /// Polled at least once; holds the running future
+    Running(BoxFuture<'static, HttpResult>),
+}
+
 /// Represents the [`Future`] that wraps the next middleware in the chain,
 /// that will be called by `await` with the current [`HttpContext`]
-/// 
+///
 /// # Example
 /// ```no_run
 /// # use volga::middleware::Next;
@@ -21,7 +36,7 @@ use super::{HttpContext, NextFn};
 /// });
 /// ```
 pub struct Next {
-    inner: Option<Pin<Box<dyn Future<Output = HttpResult> + Send>>>
+    state: Option<NextState>
 }
 
 impl std::fmt::Debug for Next {
@@ -37,24 +52,35 @@ impl Future for Next {
     #[inline]
     fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
         let this = self.get_mut();
-        let fut = this
-            .inner
-            .as_mut()
-            .ok_or_else(|| Error::server_error("Next polled after completion"))?;
-
-        let poll = fut.as_mut().poll(cx);
-        if matches!(poll, Poll::Ready(_)) {
-            this.inner = None;
+        match this.state.take() {
+            None => Poll::Ready(Err(Error::server_error("Next polled after completion"))),
+            Some(NextState::Pending(ctx, next)) => {
+                let mut fut = next(ctx);
+                let poll = fut.as_mut().poll(cx);
+                if poll.is_pending() {
+                    this.state = Some(NextState::Running(fut));
+                }
+                poll
+            }
+            Some(NextState::Running(mut fut)) => {
+                let poll = fut.as_mut().poll(cx);
+                if poll.is_pending() {
+                    this.state = Some(NextState::Running(fut));
+                }
+                poll
+            }
         }
-        poll
     }
 }
 
 impl Next {
-    /// Creates a new [`Next`]
+    /// Creates a new [`Next`].
+    ///
+    /// The inner future is created lazily: `next(ctx)` is not called until
+    /// this future is first polled. This avoids a heap allocation when the
+    /// middleware exits early without awaiting `next`.
     pub fn new(ctx: HttpContext, next: NextFn) -> Self {
-        Self { inner: Some(Box::pin(next(ctx))) }
-        //Self { ctx: Some(ctx), next }
+        Self { state: Some(NextState::Pending(ctx, next)) }
     }
 }
 
@@ -166,12 +192,12 @@ mod tests {
     use futures_util::task::noop_waker_ref;
     use crate::{HttpBody, HttpResponse, status};
     use crate::error::Error;
-    use super::{MapOkHandler, MiddlewareHandler, Next};
+    use super::{MapOkHandler, MiddlewareHandler, Next, NextState};
 
     #[test]
     fn next_returns_error_when_polled_after_completion() {
         let mut next = Next {
-            inner: Some(Box::pin(async { status!(204) })),
+            state: Some(NextState::Running(Box::pin(async { status!(204) }))),
         };
 
         let waker = noop_waker_ref();
@@ -194,7 +220,7 @@ mod tests {
     #[tokio::test]
     async fn middleware_handler_invokes_function_with_next() {
         let next = Next {
-            inner: Some(Box::pin(async { status!(204) })),
+            state: Some(NextState::Running(Box::pin(async { status!(204) }))),
         };
 
         let handler = |value: u8, next: Next| async move {