From 64e46b710788032645feef7c8db76bc65871ba13 Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Fri, 16 Apr 2021 01:23:15 -0700 Subject: [PATCH] Introduce sentinels: auto-discovered launch abort. Sentinels resolve a long-standing usability and functional correctness issue in Rocket: starting an application with guards and/or responders that depend on state that isn't available. The canonical example is the 'State' guard. Prior to this commit, an application with routes that queried unmanaged state via 'State' would fail at runtime. With this commit, the application refuses to launch with a detailed error message. The 'Sentinel' docs explains it as: A sentinel, automatically run on ignition, can trigger a launch abort should an instance fail to meet arbitrary conditions. Every type that appears in a mounted route's type signature is eligible to be a sentinel. Of these, those that implement 'Sentinel' have their 'abort()' method invoked automatically, immediately after ignition, once for each unique type. Sentinels inspect the finalized instance of 'Rocket' and can trigger a launch abort by returning 'true'. The following types are now sentinels: * 'contrib::databases::Connection' (any '#[database]' type) * 'contrib::templates::Metadata' * 'contrib::templates::Template' * 'core::State' The following are "specialized" sentinels, which allow sentinel discovery even through type aliases: * 'Option', 'Debug' if 'T: Sentinel' * 'Result', 'Either' if 'T: Sentinel', 'E: Sentinel' Closes #464. --- contrib/codegen/src/database.rs | 6 + contrib/lib/src/databases/connection.rs | 19 +- contrib/lib/src/templates/metadata.rs | 18 +- contrib/lib/src/templates/mod.rs | 17 +- contrib/lib/tests/databases.rs | 15 +- contrib/lib/tests/templates.rs | 54 +++ core/codegen/src/attribute/route/mod.rs | 67 ++- core/codegen/src/attribute/route/parse.rs | 13 +- core/codegen/src/exports.rs | 1 + core/codegen/src/syn_ext.rs | 92 +++- .../route-attribute-general-syntax.stderr | 4 +- .../route-path-bad-syntax.stderr | 4 +- .../route-attribute-general-syntax.stderr | 4 +- .../route-path-bad-syntax.stderr | 4 +- core/lib/src/error.rs | 20 +- core/lib/src/lib.rs | 2 + core/lib/src/rocket.rs | 17 +- core/lib/src/route/route.rs | 28 +- core/lib/src/router/router.rs | 4 +- core/lib/src/sentinel.rs | 438 ++++++++++++++++++ core/lib/src/state.rs | 18 +- core/lib/tests/sentinel.rs | 236 ++++++++++ examples/error-handling/src/main.rs | 9 +- 23 files changed, 1042 insertions(+), 48 deletions(-) create mode 100644 core/lib/src/sentinel.rs create mode 100644 core/lib/tests/sentinel.rs diff --git a/contrib/codegen/src/database.rs b/contrib/codegen/src/database.rs index 1e88fe51..aad3fca3 100644 --- a/contrib/codegen/src/database.rs +++ b/contrib/codegen/src/database.rs @@ -118,5 +118,11 @@ pub fn database_attr(attr: TokenStream, input: TokenStream) -> Result::from_request(__r).await.map(Self) } } + + impl ::rocket::Sentinel for #guard_type { + fn abort(__r: &::rocket::Rocket<::rocket::Ignite>) -> bool { + <#conn>::abort(__r) + } + } }.into()) } diff --git a/contrib/lib/src/databases/connection.rs b/contrib/lib/src/databases/connection.rs index db35697b..851bc9e9 100644 --- a/contrib/lib/src/databases/connection.rs +++ b/contrib/lib/src/databases/connection.rs @@ -1,7 +1,7 @@ use std::marker::PhantomData; use std::sync::Arc; -use rocket::{Rocket, Phase}; +use rocket::{Phase, Rocket, Ignite, Sentinel}; use rocket::fairing::{AdHoc, Fairing}; use rocket::request::{Request, Outcome, FromRequest}; use rocket::outcome::IntoOutcome; @@ -196,3 +196,20 @@ impl<'r, K: 'static, C: Poolable> FromRequest<'r> for Connection { } } } + +impl Sentinel for Connection { + fn abort(rocket: &Rocket) -> bool { + use rocket::yansi::Paint; + + if rocket.state::>().is_none() { + let conn = Paint::default(std::any::type_name::()).bold(); + let fairing = Paint::default(format!("{}::fairing()", conn)).wrap().bold(); + error!("requesting `{}` DB connection without attaching `{}`.", conn, fairing); + info_!("Attach `{}` to use database connection pooling.", fairing); + info_!("See the `contrib::database` documentation for more information."); + return true; + } + + false + } +} diff --git a/contrib/lib/src/templates/metadata.rs b/contrib/lib/src/templates/metadata.rs index 15c24be6..80590ea5 100644 --- a/contrib/lib/src/templates/metadata.rs +++ b/contrib/lib/src/templates/metadata.rs @@ -1,4 +1,4 @@ -use rocket::{Request, State}; +use rocket::{Request, State, Rocket, Ignite, Sentinel}; use rocket::http::Status; use rocket::request::{self, FromRequest}; @@ -28,7 +28,6 @@ use crate::templates::ContextManager; /// } /// } /// -/// /// fn main() { /// rocket::build() /// .attach(Template::fairing()) @@ -81,6 +80,21 @@ impl Metadata<'_> { } } +impl Sentinel for Metadata<'_> { + fn abort(rocket: &Rocket) -> bool { + if rocket.state::().is_none() { + let md = rocket::yansi::Paint::default("Metadata").bold(); + let fairing = rocket::yansi::Paint::default("Template::fairing()").bold(); + error!("requested `{}` guard without attaching `{}`.", md, fairing); + info_!("To use or query templates, you must attach `{}`.", fairing); + info_!("See the `Template` documentation for more information."); + return true; + } + + false + } +} + /// Retrieves the template metadata. If a template fairing hasn't been attached, /// an error is printed and an empty `Err` with status `InternalServerError` /// (`500`) is returned. diff --git a/contrib/lib/src/templates/mod.rs b/contrib/lib/src/templates/mod.rs index a86e51be..0409b8fc 100644 --- a/contrib/lib/src/templates/mod.rs +++ b/contrib/lib/src/templates/mod.rs @@ -137,7 +137,7 @@ use std::borrow::Cow; use std::path::PathBuf; use std::error::Error; -use rocket::{Rocket, Orbit}; +use rocket::{Rocket, Orbit, Ignite, Sentinel}; use rocket::request::Request; use rocket::fairing::Fairing; use rocket::response::{self, Content, Responder}; @@ -433,3 +433,18 @@ impl<'r> Responder<'r, 'static> for Template { Content(content_type, render).respond_to(req) } } + +impl Sentinel for Template { + fn abort(rocket: &Rocket) -> bool { + if rocket.state::().is_none() { + let template = rocket::yansi::Paint::default("Template").bold(); + let fairing = rocket::yansi::Paint::default("Template::fairing()").bold(); + error!("returning `{}` responder without attaching `{}`.", template, fairing); + info_!("To use or query templates, you must attach `{}`.", fairing); + info_!("See the `Template` documentation for more information."); + return true; + } + + false + } +} diff --git a/contrib/lib/tests/databases.rs b/contrib/lib/tests/databases.rs index 37a1e339..703b0fa1 100644 --- a/contrib/lib/tests/databases.rs +++ b/contrib/lib/tests/databases.rs @@ -56,9 +56,9 @@ mod rusqlite_integration_test { } } -#[cfg(feature = "databases")] #[cfg(test)] -mod drop_runtime_test { +#[cfg(feature = "databases")] +mod sentinel_and_runtime_test { use rocket::{Rocket, Build}; use r2d2::{ManageConnection, Pool}; use rocket_contrib::databases::{database, Poolable, PoolResult}; @@ -107,4 +107,15 @@ mod drop_runtime_test { let rocket = rocket::custom(config).attach(TestDb::fairing()); drop(rocket); } + + #[test] + fn test_sentinel() { + use rocket::{*, local::blocking::Client, error::ErrorKind::SentinelAborts}; + + #[get("/")] + fn use_db(_db: TestDb) {} + + let err = Client::debug_with(routes![use_db]).unwrap_err(); + assert!(matches!(err.kind(), SentinelAborts(vec) if vec.len() == 1)); + } } diff --git a/contrib/lib/tests/templates.rs b/contrib/lib/tests/templates.rs index 70c0502a..e0128e27 100644 --- a/contrib/lib/tests/templates.rs +++ b/contrib/lib/tests/templates.rs @@ -47,6 +47,60 @@ mod templates_tests { } } + #[test] + fn test_sentinel() { + use rocket::{local::blocking::Client, error::ErrorKind::SentinelAborts}; + + let err = Client::debug_with(routes![is_reloading]).unwrap_err(); + assert!(matches!(err.kind(), SentinelAborts(vec) if vec.len() == 1)); + + let err = Client::debug_with(routes![is_reloading, template_check]).unwrap_err(); + assert!(matches!(err.kind(), SentinelAborts(vec) if vec.len() == 2)); + + #[get("/")] + fn return_template() -> Template { + Template::render("foo", ()) + } + + let err = Client::debug_with(routes![return_template]).unwrap_err(); + assert!(matches!(err.kind(), SentinelAborts(vec) if vec.len() == 1)); + + #[get("/")] + fn return_opt_template() -> Option