Fix JSON encoding for Mustache templates.

This pull request replaces the current self-made implementation of JSON encoding special chars with re-using the Jackson JsonStringEncoder. Turns out the previous implementation also missed a few special chars so had to adjust the tests accordingly (looked at RFC 4627 for reference).

Note: There's another JSON String encoder on our classpath (org.apache.commons.lang3.StringEscapeUtils) that essentially does the same thing but adds quoting to more characters than the Jackson Encoder above.

Relates to #5473

src/main/java/org/elasticsearch/script/mustache/JsonEscapingMustacheFactory.java

@@ -18,6 +18,7 @@
  */
 package org.elasticsearch.script.mustache;
 
+import com.fasterxml.jackson.core.io.JsonStringEncoder;
 import com.github.mustachejava.DefaultMustacheFactory;
 import com.github.mustachejava.MustacheException;
 
@@ -28,40 +29,14 @@
  * A MustacheFactory that does simple JSON escaping.
  */
 public final class JsonEscapingMustacheFactory extends DefaultMustacheFactory {
-
+    
     @Override
     public void encode(String value, Writer writer) {
         try {
-            escape(value, writer);
+            JsonStringEncoder utils = new JsonStringEncoder();
+            writer.write(utils.quoteAsString(value));;
         } catch (IOException e) {
             throw new MustacheException("Failed to encode value: " + value);
         }
     }
-
-    public static Writer escape(String value, Writer writer) throws IOException {
-        for (int i = 0; i < value.length(); i++) {
-            final char character = value.charAt(i);
-            if (isEscapeChar(character)) {
-                writer.write('\\');
-            }
-            writer.write(character);
-        }
-        return writer;
-    }
-
-    public static boolean isEscapeChar(char c) {
-        switch(c) {
-            case '\b':
-            case '\f':
-            case '\n':
-            case '\r':
-            case '"':
-            case '\\':
-            case '\u000B': // vertical tab
-            case '\t':
-                return true;
-        }
-        return false;
-    }
-
 }

src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTest.java

@@ -18,7 +18,6 @@
  */
 package org.elasticsearch.script.mustache;
 
-import com.carrotsearch.randomizedtesting.generators.RandomPicks;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.settings.ImmutableSettings;
 import org.elasticsearch.test.ElasticsearchTestCase;
@@ -38,10 +37,12 @@
  */
 public class MustacheScriptEngineTest extends ElasticsearchTestCase {
     private MustacheScriptEngineService qe;
+    private JsonEscapingMustacheFactory escaper;
 
     @Before
     public void setup() {
         qe = new MustacheScriptEngineService(ImmutableSettings.Builder.EMPTY_SETTINGS);
+        escaper = new JsonEscapingMustacheFactory();
     }
 
     @Test
@@ -73,43 +74,98 @@ public void testSimpleParameterReplace() {
     public void testEscapeJson() throws IOException {
         {
             StringWriter writer = new StringWriter();
-            JsonEscapingMustacheFactory.escape("hello \n world", writer);
-            assertThat(writer.toString(), equalTo("hello \\\n world"));
+            escaper.encode("hello \n world", writer);
+            assertThat(writer.toString(), equalTo("hello \\n world"));
         }
         {
             StringWriter writer = new StringWriter();
-            JsonEscapingMustacheFactory.escape("\n", writer);
-            assertThat(writer.toString(), equalTo("\\\n"));
+            escaper.encode("\n", writer);
+            assertThat(writer.toString(), equalTo("\\n"));
         }
 
-        Character[] specialChars = new Character[]{'\f', '\n', '\r', '"', '\\', (char) 11, '\t', '\b' };
+        Character[] specialChars = new Character[]{
+                '\"', 
+                '\\', 
+                '\u0000', 
+                '\u0001',
+                '\u0002',
+                '\u0003',
+                '\u0004',
+                '\u0005',
+                '\u0006',
+                '\u0007',
+                '\u0008',
+                '\u0009',
+                '\u000B',
+                '\u000C',
+                '\u000E',
+                '\u000F',
+                '\u001F'};
+        String[] escapedChars = new String[]{
+                "\\\"", 
+                "\\\\", 
+                "\\u0000", 
+                "\\u0001",
+                "\\u0002",
+                "\\u0003",
+                "\\u0004",
+                "\\u0005",
+                "\\u0006",
+                "\\u0007",
+                "\\u0008",
+                "\\u0009",
+                "\\u000B",
+                "\\u000C",
+                "\\u000E",
+                "\\u000F",
+                "\\u001F"};
         int iters = scaledRandomIntBetween(100, 1000);
         for (int i = 0; i < iters; i++) {
             int rounds = scaledRandomIntBetween(1, 20);
-            StringWriter escaped = new StringWriter();
+            StringWriter expect = new StringWriter();
             StringWriter writer = new StringWriter();
             for (int j = 0; j < rounds; j++) {
                 String s = getChars();
                 writer.write(s);
-                escaped.write(s);
-                char c = RandomPicks.randomFrom(getRandom(), specialChars);
-                writer.append(c);
-                escaped.append('\\');
-                escaped.append(c);
+                expect.write(s);
+
+                int charIndex = randomInt(7);
+                writer.append(specialChars[charIndex]);
+                expect.append(escapedChars[charIndex]);
             }
             StringWriter target = new StringWriter();
-            assertThat(escaped.toString(), equalTo(JsonEscapingMustacheFactory.escape(writer.toString(), target).toString()));
+            escaper.encode(writer.toString(), target);
+            assertThat(expect.toString(), equalTo(target.toString()));
         }
     }
 
     private String getChars() {
         String string = randomRealisticUnicodeOfCodepointLengthBetween(0, 10);
         for (int i = 0; i < string.length(); i++) {
-            if (JsonEscapingMustacheFactory.isEscapeChar(string.charAt(i))) {
+            if (isEscapeChar(string.charAt(i))) {
                 return string.substring(0, i);
             }
         }
         return string;
     }
-
+
+    /**
+     * From https://www.ietf.org/rfc/rfc4627.txt:
+     * 
+     * All Unicode characters may be placed within the
+     * quotation marks except for the characters that must be escaped:
+     * quotation mark, reverse solidus, and the control characters (U+0000
+     * through U+001F). 
+     * */
+    private static boolean isEscapeChar(char c) {
+        switch (c) {
+        case '"':
+        case '\\':
+            return true;
+        }
+
+        if (c < '\u002F')
+            return true;
+        return false;
+    }
 }