Repository: PebbleTemplates/pebble
Pull request #150 (2015-12-17 13:08:50)

Don't Just escape Strings
Escape anything, and not just `Strings`.

Previously, the `EscapeFilter` would ignore anything that wasn't a String, this
meant that an object being printed could contain some XSS content in its
`toString()` method (e.g standard java collections that contain a string with
XSS) that didn't get escaped, even if the escape filter was used explicitly.
The `EscapeFilter` now converts anything to a `String` with `toString()`, and then
proceeds as normal.

This PR will conflict with #149 and one or other of them will need to be rebased when the other gets merged, I will of course do this when necessary.

Also worth noting that even if there were no merge conflicts, tests would fail after both were to get merged in as `RawString` would no longer be ignored as required by `EscapeFilter` (so this change will also need to be made).

samlanning: Should we take a look at this one again do you think? What would be the best way to proceed?
karlpauls: +1, I'd like to second this issue. It would be good to have toString results be part of the EscapeFilter checks.

As of the patch itself I have no idea if it make sense or not or what the problems are with it but it would be nice to have the functionality implemented :-) 

Patch:
diff --git a/src/main/java/com/mitchellbosecke/pebble/extension/escaper/EscapeFilter.java b/src/main/java/com/mitchellbosecke/pebble/extension/escaper/EscapeFilter.java
index 67fe9aca..32ab8c99 100644
--- a/src/main/java/com/mitchellbosecke/pebble/extension/escaper/EscapeFilter.java
+++ b/src/main/java/com/mitchellbosecke/pebble/extension/escaper/EscapeFilter.java
@@ -77,10 +77,10 @@ public class EscapeFilter implements Filter {
     }
 
     public Object apply(Object inputObject, Map<String, Object> args) {
-        if (!(inputObject instanceof String)) {
+        if (inputObject == null || inputObject instanceof SafeString) {
             return inputObject;
         }
-        String input = (String) inputObject;
+        String input = inputObject.toString();
 
         String strategy = defaultStrategy;
 


Test patch:
diff --git a/src/test/java/com/mitchellbosecke/pebble/EscaperExtensionTest.java b/src/test/java/com/mitchellbosecke/pebble/EscaperExtensionTest.java
index 3becdb5e..bc6ec755 100644
--- a/src/test/java/com/mitchellbosecke/pebble/EscaperExtensionTest.java
+++ b/src/test/java/com/mitchellbosecke/pebble/EscaperExtensionTest.java
@@ -14,6 +14,7 @@ import com.mitchellbosecke.pebble.extension.Function;
 import com.mitchellbosecke.pebble.extension.escaper.EscapingStrategy;
 import com.mitchellbosecke.pebble.loader.StringLoader;
 import com.mitchellbosecke.pebble.template.PebbleTemplate;
+
 import org.junit.Test;
 
 import java.io.IOException;
@@ -83,6 +84,17 @@ public class EscaperExtensionTest extends AbstractTest {
         assertEquals("&lt;br /&gt;", writer.toString());
     }
 
+    @Test
+    public void testAutoescapeNonString() throws PebbleException, IOException {
+        PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader()).strictVariables(false).build();
+        PebbleTemplate template = pebble.getTemplate("{{ text }}");
+        Map<String, Object> context = new HashMap<>();
+        context.put("text", Collections.singletonList("<br />"));
+        Writer writer = new StringWriter();
+        template.evaluate(writer, context);
+        assertEquals("[&lt;br /&gt;]", writer.toString());
+    }
+
     @Test
     public void testDisableAutoEscaping() throws PebbleException, IOException {
         PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader()).strictVariables(false)