From 0aead1575ff118ae1de3f64af87e918937b53761 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Kijewski?= Date: Sun, 7 Jul 2024 01:58:02 +0200 Subject: [PATCH] Simplify `|escape`: make it a "normal" filter --- rinja/benches/escape.rs | 12 +- rinja/benches/to-json.rs | 11 +- rinja/src/filters/escape.rs | 217 +++++++++++++++------------------- rinja/src/filters/mod.rs | 48 +------- rinja/src/lib.rs | 1 - rinja_derive/src/config.rs | 26 ++-- rinja_derive/src/generator.rs | 18 +-- rinja_derive/src/tests.rs | 7 +- 8 files changed, 129 insertions(+), 211 deletions(-) diff --git a/rinja/benches/escape.rs b/rinja/benches/escape.rs index ddd923af..a8e55f7b 100644 --- a/rinja/benches/escape.rs +++ b/rinja/benches/escape.rs @@ -1,5 +1,5 @@ use criterion::{criterion_group, criterion_main, Criterion}; -use rinja::{Html, MarkupDisplay}; +use rinja::filters::{escape, Html}; criterion_main!(benches); criterion_group!(benches, functions); @@ -65,10 +65,10 @@ quis lacus at, gravida maximus elit. Duis tristique, nisl nullam. "#; b.iter(|| { - format!("{}", MarkupDisplay::new_unsafe(string_long, Html)); - format!("{}", MarkupDisplay::new_unsafe(string_short, Html)); - format!("{}", MarkupDisplay::new_unsafe(empty, Html)); - format!("{}", MarkupDisplay::new_unsafe(no_escape, Html)); - format!("{}", MarkupDisplay::new_unsafe(no_escape_long, Html)); + format!("{}", escape(string_long, Html).unwrap()); + format!("{}", escape(string_short, Html).unwrap()); + format!("{}", escape(empty, Html).unwrap()); + format!("{}", escape(no_escape, Html).unwrap()); + format!("{}", escape(no_escape_long, Html).unwrap()); }); } diff --git a/rinja/benches/to-json.rs b/rinja/benches/to-json.rs index 35c561c0..bd8711ca 100644 --- a/rinja/benches/to-json.rs +++ b/rinja/benches/to-json.rs @@ -1,6 +1,5 @@ use criterion::{criterion_group, criterion_main, Criterion}; -use rinja::filters::{json, json_pretty}; -use rinja::{Html, MarkupDisplay}; +use rinja::filters::{escape, json, json_pretty, Html}; criterion_main!(benches); criterion_group!(benches, functions); @@ -9,7 +8,6 @@ fn functions(c: &mut Criterion) { c.bench_function("escape JSON", escape_json); c.bench_function("escape JSON (pretty)", escape_json_pretty); c.bench_function("escape JSON for HTML", escape_json_for_html); - c.bench_function("escape JSON for HTML (pretty)", escape_json_for_html); c.bench_function("escape JSON for HTML (pretty)", escape_json_for_html_pretty); } @@ -32,7 +30,7 @@ fn escape_json_pretty(b: &mut criterion::Bencher<'_>) { fn escape_json_for_html(b: &mut criterion::Bencher<'_>) { b.iter(|| { for &s in STRINGS { - format!("{}", MarkupDisplay::new_unsafe(json(s).unwrap(), Html)); + format!("{}", escape(json(s).unwrap(), Html).unwrap()); } }); } @@ -40,10 +38,7 @@ fn escape_json_for_html(b: &mut criterion::Bencher<'_>) { fn escape_json_for_html_pretty(b: &mut criterion::Bencher<'_>) { b.iter(|| { for &s in STRINGS { - format!( - "{}", - MarkupDisplay::new_unsafe(json_pretty(s, 2).unwrap(), Html), - ); + format!("{}", escape(json_pretty(s, 2).unwrap(), Html).unwrap(),); } }); } diff --git a/rinja/src/filters/escape.rs b/rinja/src/filters/escape.rs index 2f5d4f0f..50e9d2a6 100644 --- a/rinja/src/filters/escape.rs +++ b/rinja/src/filters/escape.rs @@ -1,112 +1,74 @@ -use core::fmt::{self, Display, Formatter, Write}; -use core::str; - -#[derive(Debug)] -pub struct MarkupDisplay -where - E: Escaper, - T: Display, -{ - value: DisplayValue, - escaper: E, +use std::convert::Infallible; +use std::fmt::{self, Display, Formatter, Write}; +use std::str; + +/// Marks a string (or other `Display` type) as safe +/// +/// Use this is you want to allow markup in an expression, or if you know +/// that the expression's contents don't need to be escaped. +/// +/// Rinja will automatically insert the first (`Escaper`) argument, +/// so this filter only takes a single argument of any type that implements +/// `Display`. +#[inline] +pub fn safe(text: impl fmt::Display, escaper: impl Escaper) -> Result { + let _ = escaper; // it should not be part of the interface that the `escaper` is unused + Ok(text) } -impl MarkupDisplay -where - E: Escaper, - T: Display, -{ - pub fn new_unsafe(value: T, escaper: E) -> Self { - Self { - value: DisplayValue::Unsafe(value), - escaper, +/// Escapes strings according to the escape mode. +/// +/// Rinja will automatically insert the first (`Escaper`) argument, +/// so this filter only takes a single argument of any type that implements +/// `Display`. +/// +/// It is possible to optionally specify an escaper other than the default for +/// the template's extension, like `{{ val|escape("txt") }}`. +#[inline] +pub fn escape(text: impl fmt::Display, escaper: impl Escaper) -> Result { + struct EscapeDisplay(T, E); + struct EscapeWriter(W, E); + + impl fmt::Display for EscapeDisplay { + #[inline] + fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result { + write!(EscapeWriter(fmt, self.1), "{}", &self.0) } } - pub fn new_safe(value: T, escaper: E) -> Self { - Self { - value: DisplayValue::Safe(value), - escaper, + impl Write for EscapeWriter { + #[inline] + fn write_str(&mut self, s: &str) -> fmt::Result { + self.1.write_escaped_str(&mut self.0, s) } - } - - #[must_use] - pub fn mark_safe(mut self) -> MarkupDisplay { - self.value = match self.value { - DisplayValue::Unsafe(t) => DisplayValue::Safe(t), - _ => self.value, - }; - self - } -} -impl Display for MarkupDisplay -where - E: Escaper, - T: Display, -{ - fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result { - match self.value { - DisplayValue::Unsafe(ref t) => write!( - EscapeWriter { - fmt, - escaper: &self.escaper - }, - "{t}" - ), - DisplayValue::Safe(ref t) => t.fmt(fmt), + #[inline] + fn write_char(&mut self, c: char) -> fmt::Result { + self.1.write_escaped_char(&mut self.0, c) } } -} - -#[derive(Debug)] -pub struct EscapeWriter<'a, E, W> { - fmt: W, - escaper: &'a E, -} - -impl Write for EscapeWriter<'_, E, W> -where - W: Write, - E: Escaper, -{ - fn write_str(&mut self, s: &str) -> fmt::Result { - self.escaper.write_escaped(&mut self.fmt, s) - } -} -pub fn escape(string: &str, escaper: E) -> Escaped<'_, E> -where - E: Escaper, -{ - Escaped { string, escaper } + Ok(EscapeDisplay(text, escaper)) } -#[derive(Debug)] -pub struct Escaped<'a, E> -where - E: Escaper, -{ - string: &'a str, - escaper: E, -} - -impl Display for Escaped<'_, E> -where - E: Escaper, -{ - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - self.escaper.write_escaped(fmt, self.string) - } +/// Alias for [`escape()`] +#[inline] +pub fn e(text: impl fmt::Display, escaper: impl Escaper) -> Result { + escape(text, escaper) } +/// Escape characters in a safe way for HTML texts and attributes +/// +/// * `<` => `<` +/// * `>` => `>` +/// * `&` => `&` +/// * `"` => `"` +/// * `'` => `'` +#[derive(Debug, Clone, Copy, Default)] pub struct Html; impl Escaper for Html { - fn write_escaped(&self, mut fmt: W, string: &str) -> fmt::Result - where - W: Write, - { + fn write_escaped_str(&self, mut fmt: W, string: &str) -> fmt::Result { let mut last = 0; for (index, byte) in string.bytes().enumerate() { const MIN_CHAR: u8 = b'"'; @@ -133,48 +95,55 @@ impl Escaper for Html { } fmt.write_str(&string[last..]) } + + fn write_escaped_char(&self, mut fmt: W, c: char) -> fmt::Result { + fmt.write_str(match (c.is_ascii(), c as u8) { + (true, b'<') => "<", + (true, b'>') => ">", + (true, b'&') => "&", + (true, b'"') => """, + (true, b'\'') => "'", + _ => return fmt.write_char(c), + }) + } } +/// Don't escape the input but return in verbatim +#[derive(Debug, Clone, Copy, Default)] pub struct Text; impl Escaper for Text { - fn write_escaped(&self, mut fmt: W, string: &str) -> fmt::Result - where - W: Write, - { + #[inline] + fn write_escaped_str(&self, mut fmt: W, string: &str) -> fmt::Result { fmt.write_str(string) } -} - -#[derive(Debug, PartialEq)] -enum DisplayValue -where - T: Display, -{ - Safe(T), - Unsafe(T), -} -pub trait Escaper { - fn write_escaped(&self, fmt: W, string: &str) -> fmt::Result - where - W: Write; + #[inline] + fn write_escaped_char(&self, mut fmt: W, c: char) -> fmt::Result { + fmt.write_char(c) + } } -#[cfg(test)] -mod tests { - extern crate std; - - use std::string::ToString; - - use super::*; +pub trait Escaper: Copy { + fn write_escaped_str(&self, fmt: W, string: &str) -> fmt::Result; - #[test] - fn test_escape() { - assert_eq!(escape("", Html).to_string(), ""); - assert_eq!(escape("<&>", Html).to_string(), "<&>"); - assert_eq!(escape("bla&", Html).to_string(), "bla&"); - assert_eq!(escape("(&self, fmt: W, c: char) -> fmt::Result { + self.write_escaped_str(fmt, c.encode_utf8(&mut [0; 4])) } } + +#[test] +fn test_escape() { + assert_eq!(escape("", Html).unwrap().to_string(), ""); + assert_eq!(escape("<&>", Html).unwrap().to_string(), "<&>"); + assert_eq!(escape("bla&", Html).unwrap().to_string(), "bla&"); + assert_eq!(escape("", Text).unwrap().to_string(), "<&>"); + assert_eq!(escape("bla&", Text).unwrap().to_string(), "bla&"); + assert_eq!(escape("(e: E, v: T) -> Result, Infallible> -where - E: Escaper, - T: fmt::Display, -{ - Ok(MarkupDisplay::new_safe(v, e)) -} - -/// Escapes strings according to the escape mode. -/// -/// Rinja will automatically insert the first (`Escaper`) argument, -/// so this filter only takes a single argument of any type that implements -/// `Display`. -/// -/// It is possible to optionally specify an escaper other than the default for -/// the template's extension, like `{{ val|escape("txt") }}`. -#[inline] -pub fn escape(e: E, v: T) -> Result, Infallible> -where - E: Escaper, - T: fmt::Display, -{ - Ok(MarkupDisplay::new_unsafe(v, e)) -} - -/// Alias for [`escape()`] -#[inline] -pub fn e(e: E, v: T) -> Result, Infallible> -where - E: Escaper, - T: fmt::Display, -{ - escape(e, v) -} - #[cfg(feature = "humansize")] /// Returns adequate string representation (in KB, ..) of number of bytes /// diff --git a/rinja/src/lib.rs b/rinja/src/lib.rs index 2c6d2a76..5afcb165 100644 --- a/rinja/src/lib.rs +++ b/rinja/src/lib.rs @@ -70,7 +70,6 @@ pub mod helpers; use std::fmt; -pub use filters::escape::{Html, MarkupDisplay, Text}; pub use rinja_derive::Template; #[doc(hidden)] diff --git a/rinja_derive/src/config.rs b/rinja_derive/src/config.rs index 2c4ffece..471557ff 100644 --- a/rinja_derive/src/config.rs +++ b/rinja_derive/src/config.rs @@ -100,8 +100,11 @@ impl<'a> Config<'a> { escapers.push((str_set(&escaper.extensions), escaper.path.into())); } } - for (extensions, path) in DEFAULT_ESCAPERS { - escapers.push((str_set(extensions), format!("{CRATE}{path}").into())); + for (extensions, name) in DEFAULT_ESCAPERS { + escapers.push(( + str_set(extensions), + format!("{CRATE}::filters::{name}").into(), + )); } Ok(Config { @@ -316,9 +319,11 @@ pub(crate) fn get_template_source( static CONFIG_FILE_NAME: &str = "rinja.toml"; static DEFAULT_SYNTAX_NAME: &str = "default"; static DEFAULT_ESCAPERS: &[(&[&str], &str)] = &[ - (&["html", "htm", "svg", "xml"], "::Html"), - (&["md", "none", "txt", "yml", ""], "::Text"), - (&["j2", "jinja", "jinja2"], "::Html"), + ( + &["html", "htm", "j2", "jinja", "jinja2", "svg", "xml"], + "Html", + ), + (&["md", "none", "txt", "yml", ""], "Text"), ]; #[cfg(test)] @@ -571,7 +576,7 @@ mod tests { let config = Config::new( r#" [[escaper]] - path = "::rinja::Js" + path = "::my_filters::Js" extensions = ["js"] "#, None, @@ -581,16 +586,15 @@ mod tests { assert_eq!( config.escapers, vec![ - (str_set(&["js"]), "::rinja::Js".into()), + (str_set(&["js"]), "::my_filters::Js".into()), ( - str_set(&["html", "htm", "svg", "xml"]), - "::rinja::Html".into() + str_set(&["html", "htm", "j2", "jinja", "jinja2", "svg", "xml"]), + "::rinja::filters::Html".into() ), ( str_set(&["md", "none", "txt", "yml", ""]), - "::rinja::Text".into() + "::rinja::filters::Text".into() ), - (str_set(&["j2", "jinja", "jinja2"]), "::rinja::Html".into()), ] ); } diff --git a/rinja_derive/src/generator.rs b/rinja_derive/src/generator.rs index 4376590b..63a7dfff 100644 --- a/rinja_derive/src/generator.rs +++ b/rinja_derive/src/generator.rs @@ -1166,8 +1166,8 @@ impl<'a> Generator<'a> { let expression = match wrapped { DisplayWrap::Wrapped => expr, DisplayWrap::Unwrapped => format!( - "{CRATE}::MarkupDisplay::new_unsafe(&({}), {})", - expr, self.input.escaper + "{CRATE}::filters::escape(&({expr}), {})?", + self.input.escaper, ), }; let id = match expr_cache.entry(expression) { @@ -1393,12 +1393,9 @@ impl<'a> Generator<'a> { if args.len() != 1 { return Err(ctx.generate_error("unexpected argument(s) in `safe` filter", node)); } - buf.write(CRATE); - buf.write("::filters::safe("); - buf.write(self.input.escaper); - buf.write(", "); + buf.write(format_args!("{CRATE}::filters::safe(")); self._visit_args(ctx, buf, args)?; - buf.write(")?"); + buf.write(format_args!(", {})?", self.input.escaper)); Ok(DisplayWrap::Wrapped) } @@ -1433,12 +1430,9 @@ impl<'a> Generator<'a> { .ok_or_else(|| ctx.generate_error("invalid escaper for escape filter", node))?, None => self.input.escaper, }; - buf.write(CRATE); - buf.write("::filters::escape("); - buf.write(escaper); - buf.write(", "); + buf.write(format_args!("{CRATE}::filters::escape(")); self._visit_args(ctx, buf, &args[..1])?; - buf.write(")?"); + buf.write(format_args!(", {escaper})?")); Ok(DisplayWrap::Wrapped) } diff --git a/rinja_derive/src/tests.rs b/rinja_derive/src/tests.rs index 03f64db5..daa899a3 100644 --- a/rinja_derive/src/tests.rs +++ b/rinja_derive/src/tests.rs @@ -8,6 +8,7 @@ use crate::build_template; fn check_if_let() { // This function makes it much easier to compare expected code by adding the wrapping around // the code we want to check. + #[track_caller] fn compare(jinja: &str, expected: &str) { let jinja = format!( r##"#[template(source = r#"{jinja}"#, ext = "txt")] @@ -57,7 +58,7 @@ impl ::std::fmt::Display for Foo {{ ::std::write!( writer, "{expr0}", - expr0 = &::rinja::MarkupDisplay::new_unsafe(&(query), ::rinja::Text), + expr0 = &::rinja::filters::escape(&(query), ::rinja::filters::Text)?, )?; }"#, ); @@ -70,7 +71,7 @@ impl ::std::fmt::Display for Foo {{ ::std::write!( writer, "{expr0}", - expr0 = &::rinja::MarkupDisplay::new_unsafe(&(s), ::rinja::Text), + expr0 = &::rinja::filters::escape(&(s), ::rinja::filters::Text)?, )?; }"#, ); @@ -83,7 +84,7 @@ impl ::std::fmt::Display for Foo {{ ::std::write!( writer, "{expr0}", - expr0 = &::rinja::MarkupDisplay::new_unsafe(&(s), ::rinja::Text), + expr0 = &::rinja::filters::escape(&(s), ::rinja::filters::Text)?, )?; }"#, );