add Allow header to resource's default 405 handler (#2949)

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

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<HttpMethod>);
+
 /// 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::<RegisteredMethods>();
+
+        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
     }
 }

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
+/// If no matching route is found, [a 405 response is returned with an appropriate Allow header][RFC
-/// overridden with `default_resource()` method.
+/// 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<T = ResourceEndpoint> {
     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::<guard::RegisteredMethods>().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
+    /// If a custom default service is not registered, an empty `405 Method Not Allowed` response
-    /// sent to the client instead. Unlike [`Scope`](crate::Scope)s, a [`Resource`] does **not**
+    /// with an appropriate Allow header will be sent instead.
-    /// inherit its parent's default service.
+    ///
+    /// # 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<F, U>(mut self, f: F) -> Self
     where
         F: IntoServiceFactory<U, ServiceRequest>,
@@ -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(