From 7251c5b2defbff9c466938fafd33c9c2daded4e6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jesper=20Steen=20M=C3=B8ller?= <jesper@selskabet.org>
Date: Wed, 20 Sep 2023 02:25:08 +0200
Subject: [PATCH 1/4] Fix #1224 by searching routes after failed match

---
 core/codegen/tests/route-format.rs            |  9 ++--
 core/codegen/tests/route.rs                   |  4 +-
 core/lib/src/router/matcher.rs                |  4 +-
 core/lib/src/router/router.rs                 | 43 +++++++++++++++++++
 core/lib/src/server.rs                        | 10 +++++
 core/lib/tests/form_method-issue-45.rs        |  2 +-
 .../tests/precise-content-type-matching.rs    |  2 +-
 examples/error-handling/src/tests.rs          |  9 ++++
 8 files changed, 74 insertions(+), 9 deletions(-)

diff --git a/core/codegen/tests/route-format.rs b/core/codegen/tests/route-format.rs
index c0d927d7da..6f139914a0 100644
--- a/core/codegen/tests/route-format.rs
+++ b/core/codegen/tests/route-format.rs
@@ -61,7 +61,7 @@ fn test_formats() {
     assert_eq!(response.into_string().unwrap(), "plain");
 
     let response = client.put("/").header(ContentType::HTML).dispatch();
-    assert_eq!(response.status(), Status::NotFound);
+    assert_eq!(response.status(), Status::MethodNotAllowed);
 }
 
 // Test custom formats.
@@ -109,9 +109,12 @@ fn test_custom_formats() {
     let response = client.get("/").dispatch();
     assert_eq!(response.into_string().unwrap(), "get_foo");
 
+    let response = client.get("/").header(Accept::JPEG).dispatch();
+    assert_eq!(response.status(), Status::NotAcceptable); // Route can't produce JPEG
+
     let response = client.put("/").header(ContentType::HTML).dispatch();
-    assert_eq!(response.status(), Status::NotFound);
+    assert_eq!(response.status(), Status::UnsupportedMediaType); // Route expects "bar/baz"
 
     let response = client.post("/").header(ContentType::HTML).dispatch();
-    assert_eq!(response.status(), Status::NotFound);
+    assert_eq!(response.status(), Status::UnsupportedMediaType); // Route expects "foo"
 }
diff --git a/core/codegen/tests/route.rs b/core/codegen/tests/route.rs
index d8e4509418..78a6094c7f 100644
--- a/core/codegen/tests/route.rs
+++ b/core/codegen/tests/route.rs
@@ -105,7 +105,7 @@ fn test_full_route() {
     assert_eq!(response.status(), Status::NotFound);
 
     let response = client.post(format!("/1{}", uri)).body(simple).dispatch();
-    assert_eq!(response.status(), Status::NotFound);
+    assert_eq!(response.status(), Status::UnsupportedMediaType);
 
     let response = client
         .post(format!("/1{}", uri))
@@ -117,7 +117,7 @@ fn test_full_route() {
             sky, name.percent_decode().unwrap(), "A A", "inside", path, simple, expected_uri));
 
     let response = client.post(format!("/2{}", uri)).body(simple).dispatch();
-    assert_eq!(response.status(), Status::NotFound);
+    assert_eq!(response.status(), Status::UnsupportedMediaType);
 
     let response = client
         .post(format!("/2{}", uri))
diff --git a/core/lib/src/router/matcher.rs b/core/lib/src/router/matcher.rs
index 83d30c876b..70d0982cdd 100644
--- a/core/lib/src/router/matcher.rs
+++ b/core/lib/src/router/matcher.rs
@@ -138,7 +138,7 @@ impl Catcher {
     }
 }
 
-fn paths_match(route: &Route, req: &Request<'_>) -> bool {
+pub(crate) fn paths_match(route: &Route, req: &Request<'_>) -> bool {
     trace!("checking path match: route {} vs. request {}", route, req);
     let route_segments = &route.uri.metadata.uri_segments;
     let req_segments = req.uri().path().segments();
@@ -170,7 +170,7 @@ fn paths_match(route: &Route, req: &Request<'_>) -> bool {
     true
 }
 
-fn queries_match(route: &Route, req: &Request<'_>) -> bool {
+pub(crate) fn queries_match(route: &Route, req: &Request<'_>) -> bool {
     trace!("checking query match: route {} vs. request {}", route, req);
     if matches!(route.uri.metadata.query_color, None | Some(Color::Wild)) {
         return true;
diff --git a/core/lib/src/router/router.rs b/core/lib/src/router/router.rs
index 5617f4fbcd..37b7cb6584 100644
--- a/core/lib/src/router/router.rs
+++ b/core/lib/src/router/router.rs
@@ -55,6 +55,33 @@ impl Router {
             .flat_map(move |routes| routes.iter().filter(move |r| r.matches(req)))
     }
 
+    pub(crate) fn matches_except_formats<'r, 'a: 'r>(
+        &'a self,
+        req: &'r Request<'r>
+    ) -> bool {
+        self.routes.get(&req.method())
+            .into_iter()
+            .flatten()
+            .any(|route| super::matcher::paths_match(route, req) && super::matcher::queries_match(route, req))
+    }
+
+    const ALL_METHODS: &[Method] = &[
+        Method::Get, Method::Put, Method::Post, Method::Delete, Method::Options,
+        Method::Head, Method::Trace, Method::Connect, Method::Patch,
+    ];
+
+    pub(crate) fn matches_except_method<'r, 'a: 'r>(
+        &'a self,
+        req: &'r Request<'r>
+    ) -> bool {
+        Self::ALL_METHODS
+            .iter()
+            .filter(|method| *method != &req.method())
+            .filter_map(|method| self.routes.get(method))
+            .flatten()
+            .any(|route| super::matcher::paths_match(route, req))
+    }
+
     // For many catchers, using aho-corasick or similar should be much faster.
     pub fn catch<'r>(&self, status: Status, req: &'r Request<'r>) -> Option<&Catcher> {
         // Note that catchers are presorted by descending base length.
@@ -368,6 +395,22 @@ mod test {
         assert!(route(&router, Get, "/prefi/").is_none());
     }
 
+    fn has_mismatched_method<'a>(router: &'a Router, method: Method, uri: &'a str) -> bool {
+        let client = Client::debug_with(vec![]).expect("client");
+        let request = client.req(method, Origin::parse(uri).unwrap());
+        router.matches_except_method(&request)
+    }
+
+    #[test]
+    fn test_bad_method_routing() {
+        let router = router_with_routes(&["/hello"]);
+        assert!(route(&router, Put, "/hello").is_none());
+        assert!(has_mismatched_method(&router, Put, "/hello"));
+        assert!(has_mismatched_method(&router, Post, "/hello"));
+
+        assert!(! has_mismatched_method(&router, Get, "/hello"));
+    }
+
     /// Asserts that `$to` routes to `$want` given `$routes` are present.
     macro_rules! assert_ranked_match {
         ($routes:expr, $to:expr => $want:expr) => ({
diff --git a/core/lib/src/server.rs b/core/lib/src/server.rs
index e3836984fd..48a86ac4fc 100644
--- a/core/lib/src/server.rs
+++ b/core/lib/src/server.rs
@@ -342,6 +342,16 @@ impl Rocket<Orbit> {
         }
 
         error_!("No matching routes for {}.", request);
+        if request.route().is_none() {
+            // We failed to find a route which matches on path, query AND formats
+            if self.router.matches_except_formats(request) {
+                // Tailor the error code to the interpretation of the request in question.
+                status = if request.method().supports_payload() { Status::UnsupportedMediaType } else { Status::NotAcceptable };
+            } else if self.router.matches_except_method(request) {
+                // Found a more suitable error code from simple route paths implemented on different methods
+                status = Status::MethodNotAllowed;
+            }
+        }
         Outcome::Forward((data, status))
     }
 
diff --git a/core/lib/tests/form_method-issue-45.rs b/core/lib/tests/form_method-issue-45.rs
index ac70461958..d680dd7655 100644
--- a/core/lib/tests/form_method-issue-45.rs
+++ b/core/lib/tests/form_method-issue-45.rs
@@ -37,6 +37,6 @@ mod tests {
             .body("_method=patch&form_data=Form+data")
             .dispatch();
 
-        assert_eq!(response.status(), Status::NotFound);
+        assert_eq!(response.status(), Status::MethodNotAllowed);
     }
 }
diff --git a/core/lib/tests/precise-content-type-matching.rs b/core/lib/tests/precise-content-type-matching.rs
index 6848c738bb..e57a431bc9 100644
--- a/core/lib/tests/precise-content-type-matching.rs
+++ b/core/lib/tests/precise-content-type-matching.rs
@@ -48,7 +48,7 @@ mod tests {
             let body: Option<&'static str> = $body;
             match body {
                 Some(string) => assert_eq!(body_str, Some(string.to_string())),
-                None => assert_eq!(status, Status::NotFound)
+                None => assert_eq!(status, Status::UnsupportedMediaType)
             }
         )
     }
diff --git a/examples/error-handling/src/tests.rs b/examples/error-handling/src/tests.rs
index db5ac18d7f..097e9000f5 100644
--- a/examples/error-handling/src/tests.rs
+++ b/examples/error-handling/src/tests.rs
@@ -55,6 +55,15 @@ fn test_hello_invalid_age() {
     }
 }
 
+#[test]
+fn test_method_not_allowed() {
+    let client = Client::tracked(super::rocket()).unwrap();
+    let (name, age) = ("Pat", 86);
+    let request = client.post(format!("/hello/{}/{}", name, age)).body("body");
+    let response = request.dispatch();
+    assert_eq!(response.status(), Status::MethodNotAllowed);
+}
+
 #[test]
 fn test_hello_sergio() {
     let client = Client::tracked(super::rocket()).unwrap();

From bad92b47b9cf7d21658fa0fa40f51d8c542aaeef Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jesper=20Steen=20M=C3=B8ller?= <jesper@selskabet.org>
Date: Wed, 20 Sep 2023 02:35:15 +0200
Subject: [PATCH 2/4] Fix long lines

---
 core/lib/src/router/router.rs |  6 ++++--
 core/lib/src/server.rs        | 10 +++++++---
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/core/lib/src/router/router.rs b/core/lib/src/router/router.rs
index 37b7cb6584..48d61b85dd 100644
--- a/core/lib/src/router/router.rs
+++ b/core/lib/src/router/router.rs
@@ -6,6 +6,8 @@ use crate::http::{Method, Status};
 use crate::{Route, Catcher};
 use crate::router::Collide;
 
+use super::matcher::{paths_match, queries_match};
+
 #[derive(Debug, Default)]
 pub(crate) struct Router {
     routes: HashMap<Method, Vec<Route>>,
@@ -62,7 +64,7 @@ impl Router {
         self.routes.get(&req.method())
             .into_iter()
             .flatten()
-            .any(|route| super::matcher::paths_match(route, req) && super::matcher::queries_match(route, req))
+            .any(|route| paths_match(route, req) && queries_match(route, req))
     }
 
     const ALL_METHODS: &[Method] = &[
@@ -79,7 +81,7 @@ impl Router {
             .filter(|method| *method != &req.method())
             .filter_map(|method| self.routes.get(method))
             .flatten()
-            .any(|route| super::matcher::paths_match(route, req))
+            .any(|route| paths_match(route, req))
     }
 
     // For many catchers, using aho-corasick or similar should be much faster.
diff --git a/core/lib/src/server.rs b/core/lib/src/server.rs
index 48a86ac4fc..b37b3247d0 100644
--- a/core/lib/src/server.rs
+++ b/core/lib/src/server.rs
@@ -343,12 +343,16 @@ impl Rocket<Orbit> {
 
         error_!("No matching routes for {}.", request);
         if request.route().is_none() {
-            // We failed to find a route which matches on path, query AND formats
+            // We failed to find a route which matches on path, query AND formats.
             if self.router.matches_except_formats(request) {
                 // Tailor the error code to the interpretation of the request in question.
-                status = if request.method().supports_payload() { Status::UnsupportedMediaType } else { Status::NotAcceptable };
+                if request.method().supports_payload() {
+                    status = Status::UnsupportedMediaType;
+                } else {
+                    status = Status::NotAcceptable;
+                }
             } else if self.router.matches_except_method(request) {
-                // Found a more suitable error code from simple route paths implemented on different methods
+                // Found a more suitable error code for paths implemented on different methods.
                 status = Status::MethodNotAllowed;
             }
         }

From 8faaff9a888f042176db1e48aea47f76c9426a02 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jesper=20Steen=20M=C3=B8ller?= <jesper@selskabet.org>
Date: Wed, 20 Sep 2023 02:55:56 +0200
Subject: [PATCH 3/4] Fix code formatting and example tests.

---
 core/lib/src/router/router.rs    | 2 +-
 examples/responders/src/tests.rs | 3 +--
 examples/templating/src/tests.rs | 3 +--
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/core/lib/src/router/router.rs b/core/lib/src/router/router.rs
index 48d61b85dd..0fa535e5de 100644
--- a/core/lib/src/router/router.rs
+++ b/core/lib/src/router/router.rs
@@ -67,7 +67,7 @@ impl Router {
             .any(|route| paths_match(route, req) && queries_match(route, req))
     }
 
-    const ALL_METHODS: &[Method] = &[
+    const ALL_METHODS: &'static [Method] = &[
         Method::Get, Method::Put, Method::Post, Method::Delete, Method::Options,
         Method::Head, Method::Trace, Method::Connect, Method::Patch,
     ];
diff --git a/examples/responders/src/tests.rs b/examples/responders/src/tests.rs
index a084b1beda..c1e76ca982 100644
--- a/examples/responders/src/tests.rs
+++ b/examples/responders/src/tests.rs
@@ -126,8 +126,7 @@ fn test_xml() {
     assert_eq!(r.into_string().unwrap(), r#"{ "payload": "I'm here" }"#);
 
     let r = client.get(uri!(super::xml)).header(Accept::CSV).dispatch();
-    assert_eq!(r.status(), Status::NotFound);
-    assert!(r.into_string().unwrap().contains("not supported"));
+    assert_eq!(r.status(), Status::NotAcceptable);
 
     let r = client.get("/content/i/dont/exist").header(Accept::HTML).dispatch();
     assert_eq!(r.content_type().unwrap(), ContentType::HTML);
diff --git a/examples/templating/src/tests.rs b/examples/templating/src/tests.rs
index afdbc0450c..fd3557841b 100644
--- a/examples/templating/src/tests.rs
+++ b/examples/templating/src/tests.rs
@@ -22,8 +22,7 @@ fn test_root(kind: &str) {
         let expected = Template::show(client.rocket(), format!("{}/error/404", kind), &context);
 
         let response = client.req(*method, format!("/{}", kind)).dispatch();
-        assert_eq!(response.status(), Status::NotFound);
-        assert_eq!(response.into_string(), expected);
+        assert_eq!(response.status(), Status::MethodNotAllowed);
     }
 }
 

From 2a2272e9deedf690e746a30c28dd1c36175cc510 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jesper=20Steen=20M=C3=B8ller?= <jesper@selskabet.org>
Date: Wed, 20 Sep 2023 08:40:23 +0200
Subject: [PATCH 4/4] Added guide documentation for new status codes.

---
 site/guide/4-requests.md | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/site/guide/4-requests.md b/site/guide/4-requests.md
index 4fe8fee5b7..32d2038ceb 100644
--- a/site/guide/4-requests.md
+++ b/site/guide/4-requests.md
@@ -2020,6 +2020,13 @@ fn main() {
 }
 ```
 
+Besides `404 Not Found` for unknown URIs, Rocket may also produce
+`405 Method Not Allowed` if a request matches a URI but not a declared method
+for that URI. For routes declaring formats, Rocket will produce
+`406 Not Acceptable` status for a client request _accepting_ a format which
+isn't declared by the matching routes, or `415 Unsupported Media Type` in case
+the _payload_ of a `PUT` or `POST` is not allowed by the route.
+
 ### Scoping
 
 The first argument to `register()` is a path to scope the catcher under called