Merge pull request #77 from Kijewski/pr-safe-paragraphbreaks

 filters: proper escaping for `|linebreaks` and friends

rinja/src/filters/escape.rs

@@ -284,19 +284,26 @@ const _: () = {
         }
     }
 
-    impl<'a, T: fmt::Display, E: Escaper> AutoEscape for &AutoEscaper<'a, MaybeSafe<T>, E> {
-        type Escaped = Wrapped<'a, T, E>;
-        type Error = Infallible;
-
-        #[inline]
-        fn rinja_auto_escape(&self) -> Result<Self::Escaped, Self::Error> {
-            match self.text {
-                MaybeSafe::Safe(t) => Ok(Wrapped::Safe(t)),
-                MaybeSafe::NeedsEscaping(t) => Ok(Wrapped::NeedsEscaping(t, self.escaper)),
+    macro_rules! add_ref {
+        ($([$($tt:tt)*])*) => { $(
+            impl<'a, T: fmt::Display, E: Escaper> AutoEscape
+            for &AutoEscaper<'a, $($tt)* MaybeSafe<T>, E> {
+                type Escaped = Wrapped<'a, T, E>;
+                type Error = Infallible;
+
+                #[inline]
+                fn rinja_auto_escape(&self) -> Result<Self::Escaped, Self::Error> {
+                    match self.text {
+                        MaybeSafe::Safe(t) => Ok(Wrapped::Safe(t)),
+                        MaybeSafe::NeedsEscaping(t) => Ok(Wrapped::NeedsEscaping(t, self.escaper)),
+                    }
+                }
             }
-        }
+        )* };
     }
 
+    add_ref!([] [&] [&&] [&&&]);
+
     pub enum Wrapped<'a, T: fmt::Display + ?Sized, E: Escaper> {
         Safe(&'a T),
         NeedsEscaping(&'a T, E),
@@ -362,39 +369,33 @@ const _: () = {
         }
     }
 
-    impl<'a, T: fmt::Display, E: Escaper> AutoEscape for &AutoEscaper<'a, Safe<T>, E> {
-        type Escaped = &'a T;
-        type Error = Infallible;
+    macro_rules! add_ref {
+        ($([$($tt:tt)*])*) => { $(
+            impl<'a, T: fmt::Display, E: Escaper> AutoEscape
+            for &AutoEscaper<'a, $($tt)* Safe<T>, E> {
+                type Escaped = &'a T;
+                type Error = Infallible;
 
-        #[inline]
-        fn rinja_auto_escape(&self) -> Result<Self::Escaped, Self::Error> {
-            Ok(&self.text.0)
-        }
+                #[inline]
+                fn rinja_auto_escape(&self) -> Result<Self::Escaped, Self::Error> {
+                    Ok(&self.text.0)
+                }
+            }
+        )* };
     }
+
+    add_ref!([] [&] [&&] [&&&]);
 };
 
 /// There is not need to mark the output of a custom filter as "unsafe"; this is simply the default
 pub struct Unsafe<T: fmt::Display>(pub T);
 
-const _: () = {
-    // This is the fallback. The filter is not the last element of the filter chain.
-    impl<T: fmt::Display> fmt::Display for Unsafe<T> {
-        #[inline]
-        fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-            write!(f, "{}", self.0)
-        }
-    }
-
-    impl<'a, T: fmt::Display, E: Escaper> AutoEscape for &AutoEscaper<'a, Unsafe<T>, E> {
-        type Escaped = EscapeDisplay<&'a T, E>;
-        type Error = Infallible;
-
-        #[inline]
-        fn rinja_auto_escape(&self) -> Result<Self::Escaped, Self::Error> {
-            Ok(EscapeDisplay(&self.text.0, self.escaper))
-        }
+impl<T: fmt::Display> fmt::Display for Unsafe<T> {
+    #[inline]
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.0)
     }
-};
+}
 
 /// Like [`Safe`], but only for HTML output
 pub struct HtmlSafeOutput<T: fmt::Display>(pub T);
@@ -431,8 +432,8 @@ impl<T: HtmlSafe + ?Sized> HtmlSafe for std::sync::Arc<T> {}
 impl<T: HtmlSafe + ?Sized> HtmlSafe for std::sync::MutexGuard<'_, T> {}
 impl<T: HtmlSafe + ?Sized> HtmlSafe for std::sync::RwLockReadGuard<'_, T> {}
 impl<T: HtmlSafe + ?Sized> HtmlSafe for std::sync::RwLockWriteGuard<'_, T> {}
-impl<T: HtmlSafe> HtmlSafe for HtmlSafeOutput<T> {}
 impl<T: HtmlSafe> HtmlSafe for std::num::Wrapping<T> {}
+impl<T: fmt::Display> HtmlSafe for HtmlSafeOutput<T> {}
 
 impl<T> HtmlSafe for std::borrow::Cow<'_, T>
 where

rinja/src/filters/mod.rs

@@ -177,21 +177,21 @@ pub fn format() {}
 /// A single newline becomes an HTML line break `<br>` and a new line
 /// followed by a blank line becomes a paragraph break `<p>`.
 #[inline]
-pub fn linebreaks(s: impl fmt::Display) -> Result<impl fmt::Display, fmt::Error> {
-    fn linebreaks(s: String) -> Result<String, fmt::Error> {
+pub fn linebreaks(s: impl fmt::Display) -> Result<HtmlSafeOutput<impl fmt::Display>, fmt::Error> {
+    fn linebreaks(s: String) -> String {
         let linebroken = s.replace("\n\n", "</p><p>").replace('\n', "<br/>");
-        Ok(format!("<p>{linebroken}</p>"))
+        format!("<p>{linebroken}</p>")
     }
-    linebreaks(try_to_string(s)?)
+    Ok(HtmlSafeOutput(linebreaks(try_to_string(s)?)))
 }
 
 /// Converts all newlines in a piece of plain text to HTML line breaks
 #[inline]
-pub fn linebreaksbr(s: impl fmt::Display) -> Result<impl fmt::Display, fmt::Error> {
-    fn linebreaksbr(s: String) -> Result<String, fmt::Error> {
-        Ok(s.replace('\n', "<br/>"))
+pub fn linebreaksbr(s: impl fmt::Display) -> Result<HtmlSafeOutput<impl fmt::Display>, fmt::Error> {
+    fn linebreaksbr(s: String) -> String {
+        s.replace('\n', "<br/>")
     }
-    linebreaksbr(try_to_string(s)?)
+    Ok(HtmlSafeOutput(linebreaksbr(try_to_string(s)?)))
 }
 
 /// Replaces only paragraph breaks in plain text with appropriate HTML
@@ -200,12 +200,14 @@ pub fn linebreaksbr(s: impl fmt::Display) -> Result<impl fmt::Display, fmt::Erro
 /// Paragraph tags only wrap content; empty paragraphs are removed.
 /// No `<br/>` tags are added.
 #[inline]
-pub fn paragraphbreaks(s: impl fmt::Display) -> Result<impl fmt::Display, fmt::Error> {
-    fn paragraphbreaks(s: String) -> Result<String, fmt::Error> {
+pub fn paragraphbreaks(
+    s: impl fmt::Display,
+) -> Result<HtmlSafeOutput<impl fmt::Display>, fmt::Error> {
+    fn paragraphbreaks(s: String) -> String {
         let linebroken = s.replace("\n\n", "</p><p>").replace("<p></p>", "");
-        Ok(format!("<p>{linebroken}</p>"))
+        format!("<p>{linebroken}</p>")
     }
-    paragraphbreaks(try_to_string(s)?)
+    Ok(HtmlSafeOutput(paragraphbreaks(try_to_string(s)?)))
 }
 
 /// Converts to lowercase

rinja_derive/src/generator.rs

@@ -1300,6 +1300,9 @@ impl<'a> Generator<'a> {
             "format" => return self._visit_format_filter(ctx, buf, args, filter),
             "join" => return self._visit_join_filter(ctx, buf, args),
             "json" | "tojson" => return self._visit_json_filter(ctx, buf, args, filter),
+            "linebreaks" | "linebreaksbr" | "paragraphbreaks" => {
+                return self._visit_linebreaks_filter(ctx, buf, name, args, filter);
+            }
             "ref" => return self._visit_ref_filter(ctx, buf, args, filter),
             "safe" => return self._visit_safe_filter(ctx, buf, args, filter),
             _ => {}
@@ -1315,6 +1318,31 @@ impl<'a> Generator<'a> {
         Ok(DisplayWrap::Unwrapped)
     }
 
+    fn _visit_linebreaks_filter<T>(
+        &mut self,
+        ctx: &Context<'_>,
+        buf: &mut Buffer,
+        name: &str,
+        args: &[WithSpan<'_, Expr<'_>>],
+        node: &WithSpan<'_, T>,
+    ) -> Result<DisplayWrap, CompileError> {
+        if args.len() != 1 {
+            return Err(
+                ctx.generate_error(&format!("unexpected argument(s) in `{name}` filter"), node)
+            );
+        }
+        buf.write(format_args!(
+            "{CRATE}::filters::{name}(&(&&{CRATE}::filters::AutoEscaper::new(&(",
+        ));
+        self._visit_args(ctx, buf, args)?;
+        // The input is always HTML escaped, regardless of the selected escaper:
+        buf.write(format_args!(
+            "), {CRATE}::filters::Html)).rinja_auto_escape()?)?",
+        ));
+        // The output is marked as HTML safe, not safe in all contexts:
+        Ok(DisplayWrap::Unwrapped)
+    }
+
     fn _visit_ref_filter<T>(
         &mut self,
         ctx: &Context<'_>,
@@ -1759,8 +1787,10 @@ impl<'a> Generator<'a> {
     }
 
     fn visit_filter_source(&mut self, buf: &mut Buffer) -> DisplayWrap {
-        buf.write(FILTER_SOURCE);
-        DisplayWrap::Unwrapped
+        // We can assume that the body of the `{% filter %}` was already escaped.
+        // And if it's not, then this was done intentionally.
+        buf.write(format_args!("{CRATE}::filters::Safe(&{FILTER_SOURCE})"));
+        DisplayWrap::Wrapped
     }
 
     fn visit_bool_lit(&mut self, buf: &mut Buffer, s: &str) -> DisplayWrap {

rinja_derive/src/lib.rs

@@ -313,17 +313,17 @@ const BUILT_IN_FILTERS: &[&str] = &[
     "join",
     "linebreaks",
     "linebreaksbr",
-    "paragraphbreaks",
     "lower",
     "lowercase",
+    "paragraphbreaks",
     "safe",
     "title",
     "trim",
     "truncate",
     "upper",
     "uppercase",
-    "urlencode",
     "urlencode_strict",
+    "urlencode",
     "wordcount",
     // optional features, reserve the names anyway:
     "json",

testing/tests/filter_block.rs

@@ -254,8 +254,7 @@ fn filter_block_include() {
     {% endif -%}
 {% endfilter -%}
 "#,
-    ext = "html",
-    print = "code"
+    ext = "html"
 )]
 struct ConditionInFilter {
     x: usize,
@@ -284,7 +283,7 @@ fn filter_block_conditions() {
         {%- let canary = 3 -%}
         [
             {%- for _ in 0..=count %}
-                {%~ filter paragraphbreaks|safe -%}
+                {%~ filter paragraphbreaks -%}
                     {{v}}
                 {%~ endfilter -%}
             {%- endfor -%}

testing/tests/filters.rs

@@ -392,3 +392,52 @@ fn test_let_borrow() {
     };
     assert_eq!(template.render().unwrap(), "hello1")
 }
+
+#[test]
+fn test_linebreaks() {
+    let s = "<script>\nalert('Hello, world!')\n</script>";
+
+    #[derive(Template)]
+    #[template(source = r#"{{ s|linebreaks }}"#, ext = "html")]
+    struct LineBreaks {
+        s: &'static str,
+    }
+
+    assert_eq!(
+        LineBreaks { s }.render().unwrap(),
+        "<p>&#60;script&#62;<br/>alert(&#39;Hello, world!&#39;)<br/>&#60;/script&#62;</p>",
+    );
+
+    #[derive(Template)]
+    #[template(source = r#"{{ s|escape|linebreaks }}"#, ext = "html")]
+    struct LineBreaksExtraEscape {
+        s: &'static str,
+    }
+
+    assert_eq!(
+        LineBreaksExtraEscape { s }.render().unwrap(),
+        "<p>&#60;script&#62;<br/>alert(&#39;Hello, world!&#39;)<br/>&#60;/script&#62;</p>",
+    );
+
+    #[derive(Template)]
+    #[template(source = r#"{{ s|linebreaks|safe }}"#, ext = "html")]
+    struct LineBreaksExtraSafe {
+        s: &'static str,
+    }
+
+    assert_eq!(
+        LineBreaksExtraSafe { s }.render().unwrap(),
+        "<p>&#60;script&#62;<br/>alert(&#39;Hello, world!&#39;)<br/>&#60;/script&#62;</p>",
+    );
+
+    #[derive(Template)]
+    #[template(source = r#"{{ s|escape|linebreaks|safe }}"#, ext = "html")]
+    struct LineBreaksExtraBoth {
+        s: &'static str,
+    }
+
+    assert_eq!(
+        LineBreaksExtraBoth { s }.render().unwrap(),
+        "<p>&#60;script&#62;<br/>alert(&#39;Hello, world!&#39;)<br/>&#60;/script&#62;</p>",
+    );
+}