From 9b955747e46d671983d72caebf03791b64ddc5a2 Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Fri, 19 May 2017 03:29:08 -0700 Subject: [PATCH] Remove config global state. Use Responder::respond_to. This commit includes two major changes to core: 1. Configuration state is no longer global. The `config::active()` function has been removed. The active configuration can be retrieved via the `config` method on a `Rocket` instance. 2. The `Responder` trait has changed. `Responder::respond(self)` has been removed in favor of `Responder::respond_to(self, &Request)`. This allows responders to dynamically adjust their response based on the incoming request. Additionally, it includes the following changes to core and codegen: * The `Request::guard` method was added to allow for simple retrivial of request guards. * The `Request::limits` method was added to retrieve configured limits. * The `File` `Responder` implementation now uses a fixed size body instead of a chunked body. * The `Outcome::of(R)` method was removed while `Outcome::from Vec; @@ -71,7 +71,8 @@ pub fn error_decorator(ecx: &mut ExtCtxt, $req_ident: &'_b ::rocket::Request) -> ::rocket::response::Result<'_b> { let user_response = $user_fn_name($fn_arguments); - let response = ::rocket::response::Responder::respond(user_response)?; + let response = ::rocket::response::Responder::respond_to(user_response, + $req_ident)?; let status = ::rocket::http::Status::raw($code); ::rocket::response::Response::build().status(status).merge(response).ok() } diff --git a/codegen/src/decorators/route.rs b/codegen/src/decorators/route.rs index b060f300..44aa76f4 100644 --- a/codegen/src/decorators/route.rs +++ b/codegen/src/decorators/route.rs @@ -78,7 +78,7 @@ impl RouteGenerateExt for RouteParams { let mut items = ::rocket::request::FormItems::from($form_string); let obj = match ::rocket::request::FromForm::from_form_items(items.by_ref()) { Ok(v) => v, - Err(_) => return ::rocket::Outcome::Forward(_data) + Err(_) => return ::rocket::Outcome::Forward(__data) }; if !items.exhaust() { @@ -106,7 +106,7 @@ impl RouteGenerateExt for RouteParams { let ty = strip_ty_lifetimes(arg.ty.clone()); Some(quote_stmt!(ecx, let $name: $ty = - match ::rocket::data::FromData::from_data(_req, _data) { + match ::rocket::data::FromData::from_data(__req, __data) { ::rocket::Outcome::Success(d) => d, ::rocket::Outcome::Forward(d) => return ::rocket::Outcome::Forward(d), @@ -120,9 +120,9 @@ impl RouteGenerateExt for RouteParams { fn generate_query_statement(&self, ecx: &ExtCtxt) -> Option { let param = self.query_param.as_ref(); let expr = quote_expr!(ecx, - match _req.uri().query() { + match __req.uri().query() { Some(query) => query, - None => return ::rocket::Outcome::Forward(_data) + None => return ::rocket::Outcome::Forward(__data) } ); @@ -149,13 +149,13 @@ impl RouteGenerateExt for RouteParams { // Note: the `None` case shouldn't happen if a route is matched. let ident = param.ident().prepend(PARAM_PREFIX); let expr = match param { - Param::Single(_) => quote_expr!(ecx, match _req.get_param_str($i) { + Param::Single(_) => quote_expr!(ecx, match __req.get_param_str($i) { Some(s) => <$ty as ::rocket::request::FromParam>::from_param(s), - None => return ::rocket::Outcome::Forward(_data) + None => return ::rocket::Outcome::Forward(__data) }), - Param::Many(_) => quote_expr!(ecx, match _req.get_raw_segments($i) { + Param::Many(_) => quote_expr!(ecx, match __req.get_raw_segments($i) { Some(s) => <$ty as ::rocket::request::FromSegments>::from_segments(s), - None => return ::rocket::Outcome::Forward(_data) + None => return ::rocket::Outcome::Forward(__data) }), }; @@ -166,7 +166,7 @@ impl RouteGenerateExt for RouteParams { Err(e) => { println!(" => Failed to parse '{}': {:?}", stringify!($original_ident), e); - return ::rocket::Outcome::Forward(_data) + return ::rocket::Outcome::Forward(__data) } }; ).expect("declared param parsing statement")); @@ -195,10 +195,10 @@ impl RouteGenerateExt for RouteParams { fn_param_statements.push(quote_stmt!(ecx, #[allow(non_snake_case)] let $ident: $ty = match - ::rocket::request::FromRequest::from_request(_req) { + ::rocket::request::FromRequest::from_request(__req) { ::rocket::outcome::Outcome::Success(v) => v, ::rocket::outcome::Outcome::Forward(_) => - return ::rocket::Outcome::forward(_data), + return ::rocket::Outcome::forward(__data), ::rocket::outcome::Outcome::Failure((code, _)) => { return ::rocket::Outcome::Failure(code) }, @@ -254,13 +254,13 @@ fn generic_route_decorator(known_method: Option>, // Allow the `unreachable_code` lint for those FromParam impls that have // an `Error` associated type of !. #[allow(unreachable_code)] - fn $route_fn_name<'_b>(_req: &'_b ::rocket::Request, _data: ::rocket::Data) + fn $route_fn_name<'_b>(__req: &'_b ::rocket::Request, __data: ::rocket::Data) -> ::rocket::handler::Outcome<'_b> { $param_statements $query_statement $data_statement let responder = $user_fn_name($fn_arguments); - ::rocket::handler::Outcome::of(responder) + ::rocket::handler::Outcome::from(__req, responder) } ).unwrap()); diff --git a/codegen/src/lints/utils.rs b/codegen/src/lints/utils.rs index baa3cee6..50a5175f 100644 --- a/codegen/src/lints/utils.rs +++ b/codegen/src/lints/utils.rs @@ -174,11 +174,11 @@ pub fn msg_and_help<'a, T: LintContext<'a>>(cx: &T, note: &str, help_sp: Option, help: &str) { - let mut b = cx.struct_span_lint(lint, msg_sp, msg); - b.note(note); + // Be conservative. If we don't know the receiver, don't emit the warning. if let Some(span) = help_sp { - b.span_help(span, help); + cx.struct_span_lint(lint, msg_sp, msg) + .note(note) + .span_help(span, help) + .emit() } - - b.emit(); } diff --git a/contrib/Cargo.toml b/contrib/Cargo.toml index ed29af91..4b3d0601 100644 --- a/contrib/Cargo.toml +++ b/contrib/Cargo.toml @@ -18,8 +18,7 @@ tera_templates = ["tera", "templates"] handlebars_templates = ["handlebars", "templates"] # Internal use only. -templates = ["serde", "serde_json", "lazy_static_macro", "glob"] -lazy_static_macro = ["lazy_static"] +templates = ["serde", "serde_json", "glob"] [dependencies] rocket = { version = "0.2.6", path = "../lib/" } @@ -36,5 +35,4 @@ rmp-serde = { version = "^0.13", optional = true } # Templating dependencies only. handlebars = { version = "^0.26.1", optional = true } glob = { version = "^0.2", optional = true } -lazy_static = { version = "^0.2", optional = true } tera = { version = "^0.10", optional = true } diff --git a/contrib/src/json.rs b/contrib/src/json.rs index 75d1115d..f0338e7e 100644 --- a/contrib/src/json.rs +++ b/contrib/src/json.rs @@ -1,7 +1,6 @@ use std::ops::{Deref, DerefMut}; use std::io::Read; -use rocket::config; use rocket::outcome::{Outcome, IntoOutcome}; use rocket::request::Request; use rocket::data::{self, Data, FromData}; @@ -80,6 +79,7 @@ impl JSON { /// let my_json = JSON(string); /// assert_eq!(my_json.into_inner(), "Hello".to_string()); /// ``` + #[inline(always)] pub fn into_inner(self) -> T { self.0 } @@ -97,10 +97,7 @@ impl FromData for JSON { return Outcome::Forward(data); } - let size_limit = config::active() - .and_then(|c| c.limits.get("json")) - .unwrap_or(LIMIT); - + let size_limit = request.limits().get("json").unwrap_or(LIMIT); serde_json::from_reader(data.open().take(size_limit)) .map(|val| JSON(val)) .map_err(|e| { error_!("Couldn't parse JSON body: {:?}", e); e }) @@ -112,9 +109,9 @@ impl FromData for JSON { /// JSON and a fixed-size body with the serialized value. If serialization /// fails, an `Err` of `Status::InternalServerError` is returned. impl Responder<'static> for JSON { - fn respond(self) -> response::Result<'static> { + fn respond_to(self, req: &Request) -> response::Result<'static> { serde_json::to_string(&self.0).map(|string| { - content::JSON(string).respond().unwrap() + content::JSON(string).respond_to(req).unwrap() }).map_err(|e| { error_!("JSON failed to serialize: {:?}", e); Status::InternalServerError @@ -125,12 +122,14 @@ impl Responder<'static> for JSON { impl Deref for JSON { type Target = T; + #[inline(always)] fn deref<'a>(&'a self) -> &'a T { &self.0 } } impl DerefMut for JSON { + #[inline(always)] fn deref_mut<'a>(&'a mut self) -> &'a mut T { &mut self.0 } diff --git a/contrib/src/lib.rs b/contrib/src/lib.rs index a90c4dbc..795a90e4 100644 --- a/contrib/src/lib.rs +++ b/contrib/src/lib.rs @@ -1,4 +1,7 @@ #![feature(drop_types_in_const, macro_reexport)] +#![cfg_attr(feature = "templates", feature(conservative_impl_trait))] +#![cfg_attr(feature = "templates", feature(associated_consts))] +#![cfg_attr(feature = "templates", feature(struct_field_attributes))] //! This crate contains officially sanctioned contributor libraries that provide //! functionality commonly used by Rocket applications. @@ -37,10 +40,6 @@ #[macro_use] extern crate log; #[macro_use] extern crate rocket; -#[cfg_attr(feature = "lazy_static_macro", macro_use)] -#[cfg(feature = "lazy_static_macro")] -extern crate lazy_static; - #[cfg(feature = "serde")] extern crate serde; diff --git a/contrib/src/msgpack.rs b/contrib/src/msgpack.rs index f95da24e..3bd7a76e 100644 --- a/contrib/src/msgpack.rs +++ b/contrib/src/msgpack.rs @@ -3,7 +3,6 @@ extern crate rmp_serde; use std::ops::{Deref, DerefMut}; use std::io::{Cursor, Read}; -use rocket::config; use rocket::outcome::{Outcome, IntoOutcome}; use rocket::request::Request; use rocket::data::{self, Data, FromData}; @@ -111,11 +110,8 @@ impl FromData for MsgPack { return Outcome::Forward(data); } - let size_limit = config::active() - .and_then(|c| c.limits.get("msgpack")) - .unwrap_or(LIMIT); - let mut buf = Vec::new(); + let size_limit = request.limits().get("msgpack").unwrap_or(LIMIT); if let Err(e) = data.open().take(size_limit).read_to_end(&mut buf) { let e = MsgPackError::InvalidDataRead(e); error_!("Couldn't read request data: {:?}", e); @@ -132,7 +128,7 @@ impl FromData for MsgPack { /// Content-Type `MsgPack` and a fixed-size body with the serialization. If /// serialization fails, an `Err` of `Status::InternalServerError` is returned. impl Responder<'static> for MsgPack { - fn respond(self) -> response::Result<'static> { + fn respond_to(self, _: &Request) -> response::Result<'static> { rmp_serde::to_vec(&self.0).map_err(|e| { error_!("MsgPack failed to serialize: {:?}", e); Status::InternalServerError diff --git a/contrib/src/templates/context.rs b/contrib/src/templates/context.rs new file mode 100644 index 00000000..39cfa090 --- /dev/null +++ b/contrib/src/templates/context.rs @@ -0,0 +1,131 @@ +use std::path::{Path, PathBuf}; +use std::collections::HashMap; + +use super::{Engines, TemplateInfo}; +use super::glob; + +use rocket::http::ContentType; + +pub struct Context { + /// The root of the template directory. + pub root: PathBuf, + /// Mapping from template name to its information. + pub templates: HashMap, + /// Mapping from template name to its information. + pub engines: Engines +} + +impl Context { + pub fn initialize(root: PathBuf) -> Option { + let mut templates: HashMap = HashMap::new(); + for ext in Engines::ENABLED_EXTENSIONS { + let mut glob_path = root.join("**").join("*"); + glob_path.set_extension(ext); + let glob_path = glob_path.to_str().expect("valid glob path string"); + + for path in glob(glob_path).unwrap().filter_map(Result::ok) { + let (name, data_type_str) = split_path(&root, &path); + if let Some(info) = templates.get(&*name) { + warn_!("Template name '{}' does not have a unique path.", name); + info_!("Existing path: {:?}", info.path); + info_!("Additional path: {:?}", path); + warn_!("Using existing path for template '{}'.", name); + continue; + } + + let data_type = data_type_str.as_ref() + .map(|ext| ContentType::from_extension(ext)) + .unwrap_or(ContentType::HTML); + + templates.insert(name, TemplateInfo { + path: path.to_path_buf(), + extension: ext.to_string(), + data_type: data_type, + }); + } + } + + Engines::init(&templates).map(|engines| { + Context { root, templates, engines } + }) + } +} + +/// Removes the file path's extension or does nothing if there is none. +fn remove_extension>(path: P) -> PathBuf { + let path = path.as_ref(); + let stem = match path.file_stem() { + Some(stem) => stem, + None => return path.to_path_buf() + }; + + match path.parent() { + Some(parent) => parent.join(stem), + None => PathBuf::from(stem) + } +} + +/// Splits a path into a name that may be used to identify the template, and the +/// template's data type, if any. +fn split_path(root: &Path, path: &Path) -> (String, Option) { + let rel_path = path.strip_prefix(root).unwrap().to_path_buf(); + let path_no_ext = remove_extension(&rel_path); + let data_type = path_no_ext.extension(); + let mut name = remove_extension(&path_no_ext).to_string_lossy().into_owned(); + + // Ensure template name consistency on Windows systems + if cfg!(windows) { + name = name.replace("\\", "/"); + } + + (name, data_type.map(|d| d.to_string_lossy().into_owned())) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn template_path_index_html() { + for root in &["/", "/a/b/c/", "/a/b/c/d/", "/a/"] { + for filename in &["index.html.hbs", "index.html.tera"] { + let path = Path::new(root).join(filename); + let (name, data_type) = split_path(Path::new(root), &path); + + assert_eq!(name, "index"); + assert_eq!(data_type, Some("html".into())); + } + } + } + + #[test] + fn template_path_subdir_index_html() { + for root in &["/", "/a/b/c/", "/a/b/c/d/", "/a/"] { + for sub in &["a/", "a/b/", "a/b/c/", "a/b/c/d/"] { + for filename in &["index.html.hbs", "index.html.tera"] { + let path = Path::new(root).join(sub).join(filename); + let (name, data_type) = split_path(Path::new(root), &path); + + let expected_name = format!("{}index", sub); + assert_eq!(name, expected_name.as_str()); + assert_eq!(data_type, Some("html".into())); + } + } + } + } + + #[test] + fn template_path_doc_examples() { + fn name_for(path: &str) -> String { + split_path(Path::new("templates/"), &Path::new("templates/").join(path)).0 + } + + assert_eq!(name_for("index.html.hbs"), "index"); + assert_eq!(name_for("index.tera"), "index"); + assert_eq!(name_for("index.hbs"), "index"); + assert_eq!(name_for("dir/index.hbs"), "dir/index"); + assert_eq!(name_for("dir/index.html.tera"), "dir/index"); + assert_eq!(name_for("index.template.html.hbs"), "index.template"); + assert_eq!(name_for("subdir/index.template.html.hbs"), "subdir/index.template"); + } +} diff --git a/contrib/src/templates/engine.rs b/contrib/src/templates/engine.rs new file mode 100644 index 00000000..0136c4b1 --- /dev/null +++ b/contrib/src/templates/engine.rs @@ -0,0 +1,72 @@ +use std::collections::HashMap; + +use super::serde::Serialize; +use super::TemplateInfo; + +#[cfg(feature = "tera_templates")] use super::tera_templates::Tera; +#[cfg(feature = "handlebars_templates")] use super::handlebars_templates::Handlebars; + +pub trait Engine: Send + Sync + 'static { + const EXT: &'static str; + + fn init(templates: &[(&str, &TemplateInfo)]) -> Option where Self: Sized; + fn render(&self, name: &str, context: C) -> Option; +} + +pub struct Engines { + #[cfg(feature = "tera_templates")] + tera: Tera, + #[cfg(feature = "handlebars_templates")] + handlebars: Handlebars, +} + +impl Engines { + pub const ENABLED_EXTENSIONS: &'static [&'static str] = &[ + #[cfg(feature = "tera_templates")] Tera::EXT, + #[cfg(feature = "handlebars_templates")] Handlebars::EXT, + ]; + + pub fn init(templates: &HashMap) -> Option { + fn inner(templates: &HashMap) -> Option { + let named_templates = templates.iter() + .filter(|&(_, i)| i.extension == E::EXT) + .map(|(k, i)| (k.as_str(), i)) + .collect::>(); + + E::init(&*named_templates) + } + + Some(Engines { + #[cfg(feature = "tera_templates")] + tera: match inner::(templates) { + Some(tera) => tera, + None => return None + }, + #[cfg(feature = "handlebars_templates")] + handlebars: match inner::(templates) { + Some(hb) => hb, + None => return None + }, + }) + } + + pub fn render(&self, name: &str, info: &TemplateInfo, c: C) -> Option + where C: Serialize + { + #[cfg(feature = "tera_templates")] + { + if info.extension == Tera::EXT { + return Engine::render(&self.tera, name, c); + } + } + + #[cfg(feature = "handlebars_templates")] + { + if info.extension == Handlebars::EXT { + return Engine::render(&self.handlebars, name, c); + } + } + + None + } +} diff --git a/contrib/src/templates/handlebars_templates.rs b/contrib/src/templates/handlebars_templates.rs index 85dcdec8..6e2d6e70 100644 --- a/contrib/src/templates/handlebars_templates.rs +++ b/contrib/src/templates/handlebars_templates.rs @@ -1,57 +1,40 @@ extern crate handlebars; use super::serde::Serialize; -use super::TemplateInfo; +use super::{Engine, TemplateInfo}; -use self::handlebars::Handlebars; +pub use self::handlebars::Handlebars; -static mut HANDLEBARS: Option = None; +impl Engine for Handlebars { + const EXT: &'static str = "hbs"; -pub const EXT: &'static str = "hbs"; - -// This function must be called a SINGLE TIME from A SINGLE THREAD for safety to -// hold here and in `render`. -pub unsafe fn register(templates: &[(&str, &TemplateInfo)]) -> bool { - if HANDLEBARS.is_some() { - error_!("Internal error: reregistering handlebars!"); - return false; - } - - let mut hb = Handlebars::new(); - let mut success = true; - for &(name, info) in templates { - let path = &info.full_path; - if let Err(e) = hb.register_template_file(name, path) { - error_!("Handlebars template '{}' failed registry: {:?}", name, e); - success = false; + fn init(templates: &[(&str, &TemplateInfo)]) -> Option { + let mut hb = Handlebars::new(); + for &(name, info) in templates { + let path = &info.path; + if let Err(e) = hb.register_template_file(name, path) { + error!("Error in Handlebars template '{}'.", name); + info_!("{}", e); + info_!("Template path: '{}'.", path.to_string_lossy()); + return None; + } } + + Some(hb) } - HANDLEBARS = Some(hb); - success -} - -pub fn render(name: &str, _info: &TemplateInfo, context: &T) -> Option - where T: Serialize -{ - let hb = match unsafe { HANDLEBARS.as_ref() } { - Some(hb) => hb, - None => { - error_!("Internal error: `render` called before handlebars init."); + fn render(&self, name: &str, context: C) -> Option { + if self.get_template(name).is_none() { + error_!("Handlebars template '{}' does not exist.", name); return None; } - }; - if hb.get_template(name).is_none() { - error_!("Handlebars template '{}' does not exist.", name); - return None; - } - - match hb.render(name, context) { - Ok(string) => Some(string), - Err(e) => { - error_!("Error rendering Handlebars template '{}': {}", name, e); - None + match self.render(name, &context) { + Ok(string) => Some(string), + Err(e) => { + error_!("Error rendering Handlebars template '{}': {}", name, e); + None + } } } } diff --git a/contrib/src/templates/macros.rs b/contrib/src/templates/macros.rs deleted file mode 100644 index d4649d73..00000000 --- a/contrib/src/templates/macros.rs +++ /dev/null @@ -1,53 +0,0 @@ -/// Returns a hashset with the extensions of all of the enabled template -/// engines from the set of template engined passed in. -macro_rules! engine_set { - ($($feature:expr => $engine:ident),+,) => ({ - type RegisterFn = for<'a, 'b> unsafe fn(&'a [(&'b str, &TemplateInfo)]) -> bool; - - let mut set = Vec::new(); - $( - #[cfg(feature = $feature)] - fn $engine(set: &mut Vec<(&'static str, RegisterFn)>) { - set.push(($engine::EXT, $engine::register)); - } - - #[cfg(not(feature = $feature))] - fn $engine(_: &mut Vec<(&'static str, RegisterFn)>) { } - - $engine(&mut set); - )+ - - set - }); -} - -/// Renders the template named `name` with the given template info `info` and -/// context `ctxt` using one of the templates in the template set passed in. It -/// does this by checking if the template's extension matches the engine's -/// extension, and if so, calls the engine's `render` method. All of this only -/// happens for engine's that have been enabled as features by the user. -macro_rules! render_set { - ($name:expr, $info:expr, $ctxt:expr, $($feature:expr => $engine:ident),+,) => ({ - $( - #[cfg(feature = $feature)] - fn $engine(name: &str, info: &TemplateInfo, c: &T) - -> Option