From 0b2fcb9f4b462b5c1ecef45ef1c47c8d43fd18db Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Fri, 18 Jun 2021 22:53:28 -0700 Subject: [PATCH] Use Figment's 'Value' in contrib templating. Previously, 'serde_json::Value' was used to store the serialized template context. This value does not represent all of serde's data model. This means we may fail to serialize a valid Rust value into it, for instance, 128-bit integers. This is reduced with Figment's 'Value', which supports the majority if not all of the serde data model. At present, all supported templating engines use 'serde_json::Value', so in practice, this commit has no effect but to reduce local dependencies and provide better error messages for bad contexts. --- contrib/dyn_templates/Cargo.toml | 2 -- contrib/dyn_templates/src/engine.rs | 2 +- .../dyn_templates/src/handlebars_templates.rs | 12 +++---- contrib/dyn_templates/src/lib.rs | 24 ++++++------- contrib/dyn_templates/src/tera_templates.rs | 15 +++----- contrib/dyn_templates/tests/templates.rs | 36 +++++++++++++++++-- 6 files changed, 52 insertions(+), 39 deletions(-) diff --git a/contrib/dyn_templates/Cargo.toml b/contrib/dyn_templates/Cargo.toml index 49b8c42e..d1b570d5 100644 --- a/contrib/dyn_templates/Cargo.toml +++ b/contrib/dyn_templates/Cargo.toml @@ -16,8 +16,6 @@ tera = ["tera_"] handlebars = ["handlebars_"] [dependencies] -serde = "1.0" -serde_json = "1.0.26" glob = "0.3" notify = "4.0.6" normpath = "0.3" diff --git a/contrib/dyn_templates/src/engine.rs b/contrib/dyn_templates/src/engine.rs index d4bc0d77..a1669142 100644 --- a/contrib/dyn_templates/src/engine.rs +++ b/contrib/dyn_templates/src/engine.rs @@ -1,7 +1,7 @@ use std::path::Path; use std::collections::HashMap; -use serde::Serialize; +use rocket::serde::Serialize; use crate::TemplateInfo; diff --git a/contrib/dyn_templates/src/handlebars_templates.rs b/contrib/dyn_templates/src/handlebars_templates.rs index f36e7efd..82a13022 100644 --- a/contrib/dyn_templates/src/handlebars_templates.rs +++ b/contrib/dyn_templates/src/handlebars_templates.rs @@ -1,6 +1,6 @@ use std::path::Path; -use serde::Serialize; +use rocket::serde::Serialize; use crate::engine::Engine; pub use crate::handlebars::Handlebars; @@ -29,12 +29,8 @@ impl Engine for Handlebars<'static> { return None; } - match Handlebars::render(self, name, &context) { - Ok(string) => Some(string), - Err(e) => { - error_!("Error rendering Handlebars template '{}': {}", name, e); - None - } - } + Handlebars::render(self, name, &context) + .map_err(|e| error_!("Handlebars: {}", e)) + .ok() } } diff --git a/contrib/dyn_templates/src/lib.rs b/contrib/dyn_templates/src/lib.rs index e3d61cc4..f27193f5 100644 --- a/contrib/dyn_templates/src/lib.rs +++ b/contrib/dyn_templates/src/lib.rs @@ -118,8 +118,8 @@ //! //! Templates are rendered with the `render` method. The method takes in the //! name of a template and a context to render the template with. The context -//! can be any type that implements [`Serialize`] from [`serde`] and would -//! serialize to an `Object` value. +//! can be any type that implements [`Serialize`] and would serialize to an +//! `Object` value. //! //! ## Automatic Reloading //! @@ -127,8 +127,6 @@ //! will be automatically reloaded from disk if any changes have been made to //! the templates directory since the previous request. In release builds, //! template reloading is disabled to improve performance and cannot be enabled. -//! -//! [`Serialize`]: serde::Serialize #![doc(html_root_url = "https://api.rocket.rs/v0.5-rc/rocket_dyn_templates")] #![doc(html_favicon_url = "https://rocket.rs/images/favicon.ico")] @@ -166,18 +164,16 @@ pub use self::metadata::Metadata; use self::fairing::TemplateFairing; use self::context::{Context, ContextManager}; -use serde::Serialize; -use serde_json::{Value, to_value}; - use std::borrow::Cow; use std::path::PathBuf; -use std::error::Error; use rocket::{Rocket, Orbit, Ignite, Sentinel}; use rocket::request::Request; use rocket::fairing::Fairing; use rocket::response::{self, Responder}; use rocket::http::{ContentType, Status}; +use rocket::figment::{value::Value, error::Error}; +use rocket::serde::Serialize; const DEFAULT_TEMPLATE_DIR: &str = "templates"; @@ -191,7 +187,7 @@ const DEFAULT_TEMPLATE_DIR: &str = "templates"; #[derive(Debug)] pub struct Template { name: Cow<'static, str>, - value: Option + value: Result } #[derive(Debug)] @@ -299,7 +295,7 @@ impl Template { /// } /// ``` pub fn try_custom(f: F) -> impl Fairing - where F: Fn(&mut Engines) -> Result<(), Box> + where F: Fn(&mut Engines) -> Result<(), Box> { TemplateFairing { callback: Box::new(f) } } @@ -325,7 +321,7 @@ impl Template { pub fn render(name: S, context: C) -> Template where S: Into>, C: Serialize { - Template { name: name.into(), value: to_value(context).ok() } + Template { name: name.into(), value: Value::serialize(context) } } /// Render the template named `name` with the context `context` into a @@ -386,13 +382,13 @@ impl Template { let info = ctxt.templates.get(name).ok_or_else(|| { let ts: Vec<_> = ctxt.templates.keys().map(|s| s.as_str()).collect(); error_!("Template '{}' does not exist.", name); - info_!("Known templates: {}", ts.join(", ")); + info_!("Known templates: {}.", ts.join(", ")); info_!("Searched in {:?}.", ctxt.root); Status::InternalServerError })?; - let value = self.value.ok_or_else(|| { - error_!("The provided template context failed to serialize."); + let value = self.value.map_err(|e| { + error_!("Template context failed to serialize: {}.", e); Status::InternalServerError })?; diff --git a/contrib/dyn_templates/src/tera_templates.rs b/contrib/dyn_templates/src/tera_templates.rs index a98bb92d..36d17cbf 100644 --- a/contrib/dyn_templates/src/tera_templates.rs +++ b/contrib/dyn_templates/src/tera_templates.rs @@ -1,7 +1,7 @@ use std::path::Path; use std::error::Error; -use serde::Serialize; +use rocket::serde::Serialize; use crate::engine::Engine; @@ -42,16 +42,9 @@ impl Engine for Tera { return None; }; - let tera_ctx = match Context::from_serialize(context) { - Ok(ctx) => ctx, - Err(_) => { - error_!( - "Error generating context when rendering Tera template '{}'.", - name - ); - return None; - } - }; + let tera_ctx = Context::from_serialize(context) + .map_err(|e| error_!("Tera context error: {}.", e)) + .ok()?; match Tera::render(self, name, &tera_ctx) { Ok(string) => Some(string), diff --git a/contrib/dyn_templates/tests/templates.rs b/contrib/dyn_templates/tests/templates.rs index 1d91abb0..a1c86a6c 100644 --- a/contrib/dyn_templates/tests/templates.rs +++ b/contrib/dyn_templates/tests/templates.rs @@ -126,6 +126,21 @@ mod tera_tests { assert_eq!(template, Some(ESCAPED_EXPECTED.into())); } + // u128 is not supported. enable when it is. + // #[test] + // fn test_tera_u128() { + // const EXPECTED: &'static str + // = "\nh_start\ntitle: 123\nh_end\n\n\n1208925819614629174706176\n\nfoot\n"; + // + // let client = Client::debug(rocket()).unwrap(); + // let mut map = HashMap::new(); + // map.insert("title", 123); + // map.insert("number", 1u128 << 80); + // + // let template = Template::show(client.rocket(), "tera/txt_test", &map); + // assert_eq!(template, Some(EXPECTED.into())); + // } + #[test] fn test_template_metadata_with_tera() { let client = Client::debug(rocket()).unwrap(); @@ -151,11 +166,11 @@ mod handlebars_tests { use rocket::http::Status; use rocket::local::blocking::Client; - const EXPECTED: &'static str - = "Hello _test_!\n\n
<script /> hi
\nDone.\n\n"; - #[test] fn test_handlebars_templates() { + const EXPECTED: &'static str + = "Hello _test_!\n\n
<script /> hi
\nDone.\n\n"; + let client = Client::debug(rocket()).unwrap(); let mut map = HashMap::new(); map.insert("title", "_test_"); @@ -166,6 +181,21 @@ mod handlebars_tests { assert_eq!(template, Some(EXPECTED.into())); } + // u128 is not supported. enable when it is. + // #[test] + // fn test_handlebars_u128() { + // const EXPECTED: &'static str + // = "Hello 123!\n\n
1208925819614629174706176
\nDone.\n\n"; + // + // let client = Client::debug(rocket()).unwrap(); + // let mut map = HashMap::new(); + // map.insert("title", 123); + // map.insert("number", 1u128 << 80); + // + // let template = Template::show(client.rocket(), "hbs/test", &map); + // assert_eq!(template, Some(EXPECTED.into())); + // } + #[test] fn test_template_metadata_with_handlebars() { let client = Client::debug(rocket()).unwrap();