Improve forwarding status code precision.

Previously, the `NotFound` status code was used to signal many kinds of
recoverable, forwarding errors. This included validation errors, incorrect
Content-Type errors, and more.

This commit modifies the status code used to forward in these instances to more
precisely indicate the forwarding condition. In particular:

  * Parameter `FromParam` errors now forward as 422 (`UnprocessableEntity`).
  * Query paramater errors now forward as 422 (`UnprocessableEntity`).
  * Use of incorrect form content-type forwards as 413 (`UnsupportedMediaType`).
  * `WebSocket` guard now forwards as 400 (`BadRequest`).
  * `&Host`, `&Accept`, `&ContentType`, `IpAddr`, and `SocketAddr` all forward
    with a 500 (`InternalServerError`).

Additionally, the `IntoOutcome` trait was overhauled to support functionality
previously offered by methods on `Outcome`. The `Outcome::forward()` method now
requires a status code to use for the forwarding outcome.

Finally, logging of `Outcome`s now includes the relevant status code.

Resolves #2626.

contrib/sync_db_pools/lib/src/connection.rs

@@ -210,7 +210,7 @@ impl<'r, K: 'static, C: Poolable> FromRequest<'r> for Connection<K, C> {
     #[inline]
     async fn from_request(request: &'r Request<'_>) -> Outcome<Self, ()> {
         match request.rocket().state::<ConnectionPool<K, C>>() {
-            Some(c) => c.get().await.into_outcome((Status::ServiceUnavailable, ())),
+            Some(c) => c.get().await.or_error((Status::ServiceUnavailable, ())),
             None => {
                 error_!("Missing database fairing for `{}`", std::any::type_name::<K>());
                 Outcome::Error((Status::InternalServerError, ()))

contrib/ws/src/websocket.rs

@@ -30,7 +30,7 @@ use crate::result::{Result, Error};
 /// ### Forwarding
 ///
 /// If the incoming request is not a valid WebSocket request, the guard
-/// forwards. The guard never fails.
+/// forwards with a status of `BadRequest`. The guard never fails.
 pub struct WebSocket {
     config: Config,
     key: String,
@@ -203,7 +203,7 @@ impl<'r> FromRequest<'r> for WebSocket {
         let key = headers.get_one("Sec-WebSocket-Key").map(|k| derive_accept_key(k.as_bytes()));
         match key {
             Some(key) if is_upgrade && is_ws && is_13 => Outcome::Success(WebSocket::new(key)),
-            Some(_) | None => Outcome::Forward(Status::NotFound)
+            Some(_) | None => Outcome::Forward(Status::BadRequest)
         }
     }
 }

core/codegen/src/attribute/route/mod.rs

@@ -105,7 +105,7 @@ fn query_decls(route: &Route) -> Option<TokenStream> {
             if !__e.is_empty() {
                 #_log::warn_!("Query string failed to match route declaration.");
                 for _err in __e { #_log::warn_!("{}", _err); }
-                return #Outcome::Forward((#__data, #Status::NotFound));
+                return #Outcome::Forward((#__data, #Status::UnprocessableEntity));
             }
 
             (#(#ident.unwrap()),*)
@@ -146,7 +146,7 @@ fn param_guard_decl(guard: &Guard) -> TokenStream {
         #_log::warn_!("Parameter guard `{}: {}` is forwarding: {:?}.",
             #name, stringify!(#ty), __error);
 
-        #Outcome::Forward((#__data, #Status::NotFound))
+        #Outcome::Forward((#__data, #Status::UnprocessableEntity))
     });
 
     // All dynamic parameters should be found if this function is being called;

core/codegen/tests/route.rs

@@ -269,7 +269,7 @@ fn test_query_collection() {
 
         let colors = &["red"];
         let dog = &["name=Fido"];
-        assert_eq!(run(&client, colors, dog).0, Status::NotFound);
+        assert_eq!(run(&client, colors, dog).0, Status::UnprocessableEntity);
 
         let colors = &["red"];
         let dog = &["name=Fido", "age=2"];

core/lib/src/data/data.rs

@@ -112,7 +112,7 @@ impl<'r> Data<'r> {
     ///
     ///     async fn from_data(r: &'r Request<'_>, mut data: Data<'r>) -> Outcome<'r, Self> {
     ///         if data.peek(2).await != b"hi" {
-    ///             return Outcome::Forward((data, Status::NotFound))
+    ///             return Outcome::Forward((data, Status::BadRequest))
     ///         }
     ///
     ///         /* .. */

core/lib/src/data/from_data.rs

@@ -9,27 +9,6 @@ use crate::outcome::{self, IntoOutcome, try_outcome, Outcome::*};
 pub type Outcome<'r, T, E = <T as FromData<'r>>::Error>
     = outcome::Outcome<T, (Status, E), (Data<'r>, Status)>;
 
-impl<'r, S, E> IntoOutcome<S, (Status, E), (Data<'r>, Status)> for Result<S, E> {
-    type Error = Status;
-    type Forward = (Data<'r>, Status);
-
-    #[inline]
-    fn into_outcome(self, status: Status) -> Outcome<'r, S, E> {
-        match self {
-            Ok(val) => Success(val),
-            Err(err) => Error((status, err))
-        }
-    }
-
-    #[inline]
-    fn or_forward(self, (data, status): (Data<'r>, Status)) -> Outcome<'r, S, E> {
-        match self {
-            Ok(val) => Success(val),
-            Err(_) => Forward((data, status))
-        }
-    }
-}
-
 /// Trait implemented by data guards to derive a value from request body data.
 ///
 /// # Data Guards
@@ -271,7 +250,7 @@ impl<'r, S, E> IntoOutcome<S, (Status, E), (Data<'r>, Status)> for Result<S, E>
 ///         // Ensure the content type is correct before opening the data.
 ///         let person_ct = ContentType::new("application", "x-person");
 ///         if req.content_type() != Some(&person_ct) {
-///             return Outcome::Forward((data, Status::NotFound));
+///             return Outcome::Forward((data, Status::UnsupportedMediaType));
 ///         }
 ///
 ///         // Use a configured limit with name 'person' or fallback to default.
@@ -343,7 +322,7 @@ impl<'r> FromData<'r> for Capped<String> {
 
     async fn from_data(req: &'r Request<'_>, data: Data<'r>) -> Outcome<'r, Self> {
         let limit = req.limits().get("string").unwrap_or(Limits::STRING);
-        data.open(limit).into_string().await.into_outcome(Status::BadRequest)
+        data.open(limit).into_string().await.or_error(Status::BadRequest)
     }
 }
 
@@ -406,7 +385,7 @@ impl<'r> FromData<'r> for Capped<Vec<u8>> {
 
     async fn from_data(req: &'r Request<'_>, data: Data<'r>) -> Outcome<'r, Self> {
         let limit = req.limits().get("bytes").unwrap_or(Limits::BYTES);
-        data.open(limit).into_bytes().await.into_outcome(Status::BadRequest)
+        data.open(limit).into_bytes().await.or_error(Status::BadRequest)
     }
 }
 

core/lib/src/form/parser.rs

@@ -35,7 +35,7 @@ impl<'r, 'i> Parser<'r, 'i> {
         let parser = match req.content_type() {
             Some(c) if c.is_form() => Self::from_form(req, data).await,
             Some(c) if c.is_form_data() => Self::from_multipart(req, data).await,
-            _ => return Outcome::Forward((data, Status::NotFound)),
+            _ => return Outcome::Forward((data, Status::UnsupportedMediaType)),
         };
 
         match parser {

core/lib/src/fs/server.rs

@@ -1,9 +1,10 @@
 use std::path::{PathBuf, Path};
 
 use crate::{Request, Data};
-use crate::http::{Method, uri::Segments, ext::IntoOwned};
+use crate::http::{Method, Status, uri::Segments, ext::IntoOwned};
 use crate::route::{Route, Handler, Outcome};
-use crate::response::Redirect;
+use crate::response::{Redirect, Responder};
+use crate::outcome::IntoOutcome;
 use crate::fs::NamedFile;
 
 /// Custom handler for serving static files.
@@ -203,10 +204,10 @@ impl Handler for FileServer {
             };
 
             if segments.is_empty() {
-                let file = NamedFile::open(&self.root).await.ok();
-                return Outcome::from_or_forward(req, data, file);
+                let file = NamedFile::open(&self.root).await;
+                return file.respond_to(req).or_forward((data, Status::NotFound));
             } else {
-                return Outcome::forward(data);
+                return Outcome::forward(data, Status::NotFound);
             }
         }
 
@@ -224,18 +225,23 @@ impl Handler for FileServer {
                         .expect("adding a trailing slash to a known good path => valid path")
                         .into_owned();
 
-                    return Outcome::from_or_forward(req, data, Redirect::permanent(normal));
+                    return Redirect::permanent(normal)
+                        .respond_to(req)
+                        .or_forward((data, Status::InternalServerError));
                 }
 
                 if !options.contains(Options::Index) {
-                    return Outcome::forward(data);
+                    return Outcome::forward(data, Status::NotFound);
                 }
 
-                let index = NamedFile::open(p.join("index.html")).await.ok();
-                Outcome::from_or_forward(req, data, index)
+                let index = NamedFile::open(p.join("index.html")).await;
+                index.respond_to(req).or_forward((data, Status::NotFound))
             },
-            Some(p) => Outcome::from_or_forward(req, data, NamedFile::open(p).await.ok()),
-            None => Outcome::forward(data),
+            Some(p) => {
+                let file = NamedFile::open(p).await;
+                file.respond_to(req).or_forward((data, Status::NotFound))
+            }
+            None => Outcome::forward(data, Status::NotFound),
         }
     }
 }

core/lib/src/fs/temp_file.rs

@@ -543,7 +543,7 @@ impl<'r> FromData<'r> for Capped<TempFile<'_>> {
         }
 
         TempFile::from(req, data, None, req.content_type().cloned()).await
-            .into_outcome(Status::BadRequest)
+            .or_error(Status::BadRequest)
     }
 }
 

core/lib/src/mtls.rs

@@ -20,6 +20,6 @@ impl<'r> FromRequest<'r> for Certificate<'r> {
     async fn from_request(req: &'r Request<'_>) -> Outcome<Self, Self::Error> {
         let certs = req.connection.client_certificates.as_ref().or_forward(Status::Unauthorized);
         let data = try_outcome!(try_outcome!(certs).chain_data().or_forward(Status::Unauthorized));
-        Certificate::parse(data).into_outcome(Status::Unauthorized)
+        Certificate::parse(data).or_error(Status::Unauthorized)
     }
 }