From 06c3513bc0da212d87c6fae5332e845146991027 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Wed, 21 Dec 2022 20:28:45 +0000 Subject: [PATCH] add Allow header to resource's default 405 handler (#2949) --- actix-web/CHANGES.md | 4 +++ actix-web/src/guard/mod.rs | 16 ++++++++++- actix-web/src/resource.rs | 59 +++++++++++++++++++++++++++++++------- 3 files changed, 68 insertions(+), 11 deletions(-) diff --git a/actix-web/CHANGES.md b/actix-web/CHANGES.md index 6440ad693..8ea60266e 100644 --- a/actix-web/CHANGES.md +++ b/actix-web/CHANGES.md @@ -8,11 +8,15 @@ - Add rudimentary redirection service at `web::redirect()` / `web::Redirect`. [#1961] - Add `guard::Acceptable` for matching against `Accept` header mime types. [#2265] +### Fixed +- Add `Allow` header to `Resource`'s default responses when no routes are matched. [#2949] + [#1961]: https://github.com/actix/actix-web/pull/1961 [#2265]: https://github.com/actix/actix-web/pull/2265 [#2631]: https://github.com/actix/actix-web/pull/2631 [#2784]: https://github.com/actix/actix-web/pull/2784 [#2867]: https://github.com/actix/actix-web/pull/2867 +[#2949]: https://github.com/actix/actix-web/pull/2949 ## 4.2.1 - 2022-09-12 diff --git a/actix-web/src/guard/mod.rs b/actix-web/src/guard/mod.rs index 7cd846b66..e086f8648 100644 --- a/actix-web/src/guard/mod.rs +++ b/actix-web/src/guard/mod.rs @@ -286,11 +286,25 @@ pub fn Method(method: HttpMethod) -> impl Guard { MethodGuard(method) } +#[derive(Debug, Clone)] +pub(crate) struct RegisteredMethods(pub(crate) Vec); + /// HTTP method guard. -struct MethodGuard(HttpMethod); +#[derive(Debug)] +pub(crate) struct MethodGuard(HttpMethod); impl Guard for MethodGuard { fn check(&self, ctx: &GuardContext<'_>) -> bool { + let registered = ctx.req_data_mut().remove::(); + + if let Some(mut methods) = registered { + methods.0.push(self.0.clone()); + ctx.req_data_mut().insert(methods); + } else { + ctx.req_data_mut() + .insert(RegisteredMethods(vec![self.0.clone()])); + } + ctx.head().method == self.0 } } diff --git a/actix-web/src/resource.rs b/actix-web/src/resource.rs index c5c6701e6..997036751 100644 --- a/actix-web/src/resource.rs +++ b/actix-web/src/resource.rs @@ -13,8 +13,9 @@ use crate::{ body::MessageBody, data::Data, dev::{ensure_leading_slash, AppService, ResourceDef}, - guard::Guard, + guard::{self, Guard}, handler::Handler, + http::header, route::{Route, RouteService}, service::{ BoxedHttpService, BoxedHttpServiceFactory, HttpServiceFactory, ServiceRequest, @@ -40,8 +41,11 @@ use crate::{ /// .route(web::get().to(|| HttpResponse::Ok()))); /// ``` /// -/// If no matching route could be found, *405* response code get returned. Default behavior could be -/// overridden with `default_resource()` method. +/// If no matching route is found, [a 405 response is returned with an appropriate Allow header][RFC +/// 9110 §15.5.6]. This default behavior can be overridden using +/// [`default_service()`](Self::default_service). +/// +/// [RFC 9110 §15.5.6]: https://www.rfc-editor.org/rfc/rfc9110.html#section-15.5.6 pub struct Resource { endpoint: T, rdef: Patterns, @@ -66,7 +70,19 @@ impl Resource { guards: Vec::new(), app_data: None, default: boxed::factory(fn_service(|req: ServiceRequest| async { - Ok(req.into_response(HttpResponse::MethodNotAllowed())) + use crate::HttpMessage as _; + + let allowed = req.extensions().get::().cloned(); + + if let Some(methods) = allowed { + Ok(req.into_response( + HttpResponse::MethodNotAllowed() + .insert_header(header::Allow(methods.0)) + .finish(), + )) + } else { + Ok(req.into_response(HttpResponse::MethodNotAllowed())) + } })), } } @@ -309,13 +325,28 @@ where } } - /// Default service to be used if no matching route could be found. + /// Sets the default service to be used if no matching route is found. /// - /// You can use a [`Route`] as default service. + /// Unlike [`Scope`]s, a `Resource` does _not_ inherit its parent's default service. You can + /// use a [`Route`] as default service. /// - /// If a default service is not registered, an empty `405 Method Not Allowed` response will be - /// sent to the client instead. Unlike [`Scope`](crate::Scope)s, a [`Resource`] does **not** - /// inherit its parent's default service. + /// If a custom default service is not registered, an empty `405 Method Not Allowed` response + /// with an appropriate Allow header will be sent instead. + /// + /// # Examples + /// ``` + /// use actix_web::{App, HttpResponse, web}; + /// + /// let resource = web::resource("/test") + /// .route(web::get().to(HttpResponse::Ok)) + /// .default_service(web::to(|| { + /// HttpResponse::BadRequest() + /// })); + /// + /// App::new().service(resource); + /// ``` + /// + /// [`Scope`]: crate::Scope pub fn default_service(mut self, f: F) -> Self where F: IntoServiceFactory, @@ -606,7 +637,11 @@ mod tests { async fn test_default_resource() { let srv = init_service( App::new() - .service(web::resource("/test").route(web::get().to(HttpResponse::Ok))) + .service( + web::resource("/test") + .route(web::get().to(HttpResponse::Ok)) + .route(web::delete().to(HttpResponse::Ok)), + ) .default_service(|r: ServiceRequest| { ok(r.into_response(HttpResponse::BadRequest())) }), @@ -621,6 +656,10 @@ mod tests { .to_request(); let resp = call_service(&srv, req).await; assert_eq!(resp.status(), StatusCode::METHOD_NOT_ALLOWED); + assert_eq!( + resp.headers().get(header::ALLOW).unwrap().as_bytes(), + b"GET, DELETE" + ); let srv = init_service( App::new().service(