From d3bf929040129241d9141dfce9826c40acec549f Mon Sep 17 00:00:00 2001 From: WaterWhisperer Date: Sat, 21 Feb 2026 17:21:43 +0800 Subject: [PATCH] fix: panic when `.to()/.service()` called after `.wrap()` on Route (#3944) * fix: panic when `.to()/.service()` called after `.wrap()` on Route * tweak --------- Co-authored-by: Yuki Okushi --- actix-web/CHANGES.md | 4 +++ actix-web/src/route.rs | 64 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/actix-web/CHANGES.md b/actix-web/CHANGES.md index 3a36f57b9..b0bc84fe0 100644 --- a/actix-web/CHANGES.md +++ b/actix-web/CHANGES.md @@ -2,6 +2,10 @@ ## Unreleased +- Panic when calling `Route::to()` or `Route::service()` after `Route::wrap()` to prevent silently dropping route middleware. [#3944] + +[#3944]: https://github.com/actix/actix-web/pull/3944 + ## 4.13.0 - Minimum supported Rust version (MSRV) is now 1.88. diff --git a/actix-web/src/route.rs b/actix-web/src/route.rs index 65d7dcef0..feb94338e 100644 --- a/actix-web/src/route.rs +++ b/actix-web/src/route.rs @@ -23,6 +23,7 @@ use crate::{ pub struct Route { service: BoxedHttpServiceFactory, guards: Rc>>, + wrapped: bool, } impl Route { @@ -34,6 +35,7 @@ impl Route { Ok(req.into_response(HttpResponse::NotFound())) })), guards: Rc::new(Vec::new()), + wrapped: false, } } @@ -42,6 +44,17 @@ impl Route { /// `mw` is a middleware component (type), that can modify the requests and responses handled by /// this `Route`. /// + /// This middleware wraps the currently configured route service. Call this method after + /// [`Route::to`] or [`Route::service`] so the middleware is applied to the final handler. + /// + /// # Examples + /// ``` + /// # use actix_web::{web, HttpResponse, middleware}; + /// web::get() + /// .to(|| async { HttpResponse::Ok() }) + /// .wrap(middleware::Logger::default()); + /// ``` + /// /// See [`App::wrap`](crate::App::wrap) for more details. #[doc(alias = "middleware")] #[doc(alias = "use")] // nodejs terminology @@ -59,12 +72,24 @@ impl Route { Route { service: boxed::factory(apply(Compat::new(mw), self.service)), guards: self.guards, + wrapped: true, } } pub(crate) fn take_guards(&mut self) -> Vec> { mem::take(Rc::get_mut(&mut self.guards).unwrap()) } + + #[cold] + #[inline(never)] + #[track_caller] + fn panic_after_wrap(replaced: &str, example: &str) -> ! { + panic!( + "Route middleware was already registered with `.wrap()`. \ + Calling `.{replaced}()` now would replace the wrapped service and silently drop middleware. \ + Call `.{replaced}()` before `.wrap()` (for example: `{example}`)." + ); + } } impl ServiceFactory for Route { @@ -212,12 +237,21 @@ impl Route { /// .route(web::get().to(index)) /// ); /// ``` + /// + /// # Panics + /// Panics if called after [`Route::wrap`], since this would replace the wrapped service and + /// silently discard middleware. + #[track_caller] pub fn to(mut self, handler: F) -> Self where F: Handler, Args: FromRequest + 'static, F::Output: Responder + 'static, { + if self.wrapped { + Self::panic_after_wrap("to", "web::get().to(handler).wrap(mw)"); + } + self.service = handler_service(handler); self } @@ -254,6 +288,11 @@ impl Route { /// web::get().service(fn_factory(|| async { Ok(HelloWorld) })), /// ); /// ``` + /// + /// # Panics + /// Panics if called after [`Route::wrap`], since this would replace the wrapped service and + /// silently discard middleware. + #[track_caller] pub fn service(mut self, service_factory: S) -> Self where S: ServiceFactory< @@ -265,6 +304,10 @@ impl Route { > + 'static, E: Into + 'static, { + if self.wrapped { + Self::panic_after_wrap("service", "web::get().service(factory).wrap(mw)"); + } + self.service = boxed::factory(service_factory.map_err(Into::into)); self } @@ -459,4 +502,25 @@ mod tests { Bytes::from_static(b"Goodbye, and thanks for all the fish!") ); } + + #[test] + #[should_panic(expected = "Route middleware was already registered with `.wrap()`")] + fn wrap_before_to_panics() { + web::get() + .wrap(DefaultHeaders::new().add(("x-test", "x-value"))) + .to(HttpResponse::Ok); + } + + #[test] + #[should_panic(expected = "Route middleware was already registered with `.wrap()`")] + fn wrap_before_service_panics() { + web::get() + .wrap(DefaultHeaders::new().add(("x-test", "x-value"))) + .service(fn_factory(|| async { + Ok::<_, ()>(fn_service(|req: ServiceRequest| async { + let (req, _) = req.into_parts(); + Ok::<_, Infallible>(ServiceResponse::new(req, HttpResponse::Ok().finish())) + })) + })); + } }