Fix escaping of mustache strings.

I think the escaping done in XMustacheFactory (and by extension JsonEscapingMustacheFactory in core) is broken.
You cannot just escape any control character by sticking a '\' in front of it. For example a new line character it '\n' but this will be rendered as a line break. Simply prepending a '\' to this just results in a '\' and then a new line !
Added support for different escaping strategies based on the XContentType of a template for XMustacheEngine.
Currently only JSON escaping is supported using jackson.JsonStringEncoder.
Templates will be prepended with __<contentType>__:: when the content type is set. If this is set to JSON we will json escape the content.

Fixes: elastic/elasticsearch#404

Original commit: elastic/x-pack-elasticsearch@1400cba659
This commit is contained in:
Brian Murphy 2015-05-13 15:36:38 -04:00
parent 09621f1267
commit 54926ec336
5 changed files with 159 additions and 79 deletions

View File

@ -12,6 +12,7 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.script.ExecutableScript;
import org.elasticsearch.script.mustache.MustacheScriptEngineService;
import org.elasticsearch.watcher.support.init.proxy.ScriptServiceProxy;
import org.elasticsearch.watcher.support.template.xmustache.XMustacheScriptEngineService;
import java.util.HashMap;
import java.util.Map;
@ -34,7 +35,9 @@ public class MustacheTemplateEngine extends AbstractComponent implements Templat
Map<String, Object> mergedModel = new HashMap<>();
mergedModel.putAll(template.getParams());
mergedModel.putAll(model);
ExecutableScript executable = service.executable(MustacheScriptEngineService.NAME, template.getTemplate(), template.getType(), mergedModel);
ExecutableScript executable = service.executable(MustacheScriptEngineService.NAME,
XMustacheScriptEngineService.prepareTemplate(template.getTemplate(), template.getContentType()),
template.getType(), mergedModel);
Object result = executable.run();
if (result instanceof BytesReference) {
return ((BytesReference) result).toUtf8();

View File

@ -6,9 +6,11 @@
package org.elasticsearch.watcher.support.template.xmustache;
import org.elasticsearch.common.collect.Iterables;
import org.elasticsearch.common.jackson.core.io.JsonStringEncoder;
import org.elasticsearch.common.mustache.DefaultMustacheFactory;
import org.elasticsearch.common.mustache.MustacheException;
import org.elasticsearch.common.mustache.reflect.ReflectionObjectHandler;
import org.elasticsearch.common.xcontent.XContentType;
import java.io.IOException;
import java.io.Writer;
@ -22,7 +24,10 @@ import java.util.*;
*/
public class XMustacheFactory extends DefaultMustacheFactory {
public XMustacheFactory() {
final XContentType contentType;
public XMustacheFactory(XContentType contentType) {
this.contentType = contentType;
setObjectHandler(new ReflectionObjectHandler() {
@Override
public Object coerce(Object object) {
@ -41,39 +46,16 @@ public class XMustacheFactory extends DefaultMustacheFactory {
@Override
public void encode(String value, Writer writer) {
try {
escape(value, writer);
if (contentType == XContentType.JSON) {
writer.write(JsonStringEncoder.getInstance().quoteAsString(value));
} else {
writer.write(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;
}
static class ArrayMap extends AbstractMap<Object, Object> implements Iterable<Object> {
private final Object array;

View File

@ -13,6 +13,7 @@ import org.elasticsearch.common.io.UTF8StreamWriter;
import org.elasticsearch.common.io.stream.BytesStreamOutput;
import org.elasticsearch.common.mustache.Mustache;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.script.CompiledScript;
import org.elasticsearch.script.ExecutableScript;
import org.elasticsearch.script.ScriptEngineService;
@ -22,6 +23,7 @@ import org.elasticsearch.search.lookup.SearchLookup;
import java.io.IOException;
import java.lang.ref.SoftReference;
import java.util.Collections;
import java.util.Locale;
import java.util.Map;
/**
@ -50,7 +52,9 @@ public class XMustacheScriptEngineService extends AbstractComponent implements S
@Override
public Object compile(String template) {
/** Factory to generate Mustache objects from. */
return (new XMustacheFactory()).compile(new FastStringReader(template), "query-template");
XContentType xContentType = detectContentType(template);
template = trimContentType(template);
return (new XMustacheFactory(xContentType)).compile(new FastStringReader(template), "query-template");
}
/**
@ -125,6 +129,17 @@ public class XMustacheScriptEngineService extends AbstractComponent implements S
// Nothing to do here
}
public static String prepareTemplate(String template, @Nullable XContentType contentType) {
if (contentType == null) {
return template;
}
return new StringBuilder("__")
.append(contentType.shortName().toLowerCase(Locale.ROOT))
.append("__::")
.append(template)
.toString();
}
/**
* Used at query execution time by script service in order to execute a query template.
* */
@ -188,4 +203,30 @@ public class XMustacheScriptEngineService extends AbstractComponent implements S
writer.reset();
return writer;
}
private String trimContentType(String template) {
if (!template.startsWith("__")){
return template; //Doesn't even start with __ so can't have a content type
}
int index = template.indexOf("__::", 3); //There must be a __<content_type__:: prefix so the minimum length before detecting '__::' is 3
if (index >= 0 && index < 12) { //Assume that the content type name is less than 10 characters long otherwise we may falsely detect strings that start with '__ and have '__::' somewhere in the content
if (template.length() == 6) {
template = "";
} else {
template = template.substring(index + 4);
}
}
return template;
}
private XContentType detectContentType(String template) {
if (template.startsWith("__")) {
int endOfContentName = template.indexOf("__::", 3); //There must be a __<content_type__:: prefix so the minimum length before detecting '__::' is 3
if (endOfContentName != -1) {
return XContentType.fromRestContentType(template.substring(2, endOfContentName));
}
}
return null;
}
}

View File

@ -5,21 +5,19 @@
*/
package org.elasticsearch.watcher.support.template.xmustache;
import com.carrotsearch.randomizedtesting.generators.RandomPicks;
import com.carrotsearch.randomizedtesting.annotations.Repeat;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.test.ElasticsearchTestCase;
import org.junit.Before;
import org.junit.Test;
import java.io.IOException;
import java.io.StringWriter;
import java.io.Writer;
import java.nio.charset.Charset;
import java.util.HashMap;
import java.util.Map;
import static org.hamcrest.Matchers.equalTo;
/**
*
*/
@ -35,7 +33,7 @@ public class XMustacheScriptEngineTests extends ElasticsearchTestCase {
@Test
public void testSimpleParameterReplace() {
{
String template = "GET _search {\"query\": " + "{\"boosting\": {" + "\"positive\": {\"match\": {\"body\": \"gift\"}},"
String template = "__json__::GET _search {\"query\": " + "{\"boosting\": {" + "\"positive\": {\"match\": {\"body\": \"gift\"}},"
+ "\"negative\": {\"term\": {\"body\": {\"value\": \"solr\"}" + "}}, \"negative_boost\": {{boost_val}} } }}";
Map<String, Object> vars = new HashMap<>();
vars.put("boost_val", "0.3");
@ -45,7 +43,7 @@ public class XMustacheScriptEngineTests extends ElasticsearchTestCase {
new String(o.toBytes(), Charset.forName("UTF-8")));
}
{
String template = "GET _search {\"query\": " + "{\"boosting\": {" + "\"positive\": {\"match\": {\"body\": \"gift\"}},"
String template = "__json__::GET _search {\"query\": " + "{\"boosting\": {" + "\"positive\": {\"match\": {\"body\": \"gift\"}},"
+ "\"negative\": {\"term\": {\"body\": {\"value\": \"{{body_val}}\"}" + "}}, \"negative_boost\": {{boost_val}} } }}";
Map<String, Object> vars = new HashMap<>();
vars.put("boost_val", "0.3");
@ -57,47 +55,27 @@ public class XMustacheScriptEngineTests extends ElasticsearchTestCase {
}
}
@Test
public void testEscapeJson() throws IOException {
{
StringWriter writer = new StringWriter();
XMustacheFactory.escape("hello \n world", writer);
assertThat(writer.toString(), equalTo("hello \\\n world"));
}
{
StringWriter writer = new StringWriter();
XMustacheFactory.escape("\n", writer);
assertThat(writer.toString(), equalTo("\\\n"));
@Test @Repeat(iterations = 100)
public void testInvalidPrefixes() throws Exception {
String[] specialStrings = new String[]{"\f", "\n", "\r", "\"", "\\", "\t", "\b", "__::", "__" };
String prefix = randomFrom("", "__", "____::", "___::", "____", "::", "++json__::", "__json__", "+_json__::", "__json__:");
String template = prefix + " {{test_var1}} {{test_var2}}";
Map<String, Object> vars = new HashMap<>();
Writer var1Writer = new StringWriter();
Writer var2Writer = new StringWriter();
for(int i = 0; i < scaledRandomIntBetween(10,1000); ++i) {
var1Writer.write(randomRealisticUnicodeOfCodepointLengthBetween(0, 10));
var2Writer.write(randomRealisticUnicodeOfCodepointLengthBetween(0, 10));
var1Writer.append(randomFrom(specialStrings));
var2Writer.append(randomFrom(specialStrings));
}
Character[] specialChars = new Character[]{'\f', '\n', '\r', '"', '\\', (char) 11, '\t', '\b' };
int iters = scaledRandomIntBetween(100, 1000);
for (int i = 0; i < iters; i++) {
int rounds = scaledRandomIntBetween(1, 20);
StringWriter escaped = 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);
vars.put("test_var1", var1Writer.toString());
vars.put("test_var2", var2Writer.toString());
BytesReference o = (BytesReference) engine.execute(engine.compile(template), vars);
String s1 = o.toUtf8();
String s2 = prefix + " " + var1Writer.toString() + " " + var2Writer.toString();
assertEquals(s1, s2);
}
StringWriter target = new StringWriter();
assertThat(escaped.toString(), equalTo(XMustacheFactory.escape(writer.toString(), target).toString()));
}
}
private String getChars() {
String string = randomRealisticUnicodeOfCodepointLengthBetween(0, 10);
for (int i = 0; i < string.length(); i++) {
if (XMustacheFactory.isEscapeChar(string.charAt(i))) {
return string.substring(0, i);
}
}
return string;
}
}

View File

@ -10,12 +10,16 @@ import com.carrotsearch.randomizedtesting.annotations.Repeat;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.collect.ImmutableMap;
import org.elasticsearch.common.collect.ImmutableSet;
import org.elasticsearch.common.jackson.core.io.JsonStringEncoder;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.script.ScriptEngineService;
import org.elasticsearch.test.ElasticsearchTestCase;
import org.junit.Before;
import org.junit.Test;
import java.io.IOException;
import java.io.StringWriter;
import java.util.HashMap;
import java.util.Map;
@ -84,4 +88,76 @@ public class XMustacheTests extends ElasticsearchTestCase {
BytesReference bytes = (BytesReference) output;
assertThat(bytes.toUtf8(), equalTo("foo bar"));
}
@Test @Repeat(iterations = 10)
public void testEscaping() throws Exception {
XContentType contentType = randomFrom(XContentType.values());
if (rarely()) {
contentType = null;
}
Character[] specialChars = new Character[]{'\f', '\n', '\r', '"', '\\', (char) 11, '\t', '\b' };
int iters = scaledRandomIntBetween(100, 1000);
for (int i = 0; i < iters; i++) {
int rounds = scaledRandomIntBetween(1, 20);
StringWriter escaped = new StringWriter(); //This will be escaped as it is constructed
StringWriter unescaped = new StringWriter(); //This will be escaped at the end
for (int j = 0; j < rounds; j++) {
String s = getChars();
unescaped.write(s);
if (contentType == XContentType.JSON) {
escaped.write(JsonStringEncoder.getInstance().quoteAsString(s));
} else {
escaped.write(s);
}
char c = randomFrom(specialChars);
unescaped.append(c);
if (contentType == XContentType.JSON) {
escaped.write(JsonStringEncoder.getInstance().quoteAsString("" + c));
} else {
escaped.append(c);
}
}
if (contentType == XContentType.JSON) {
assertThat(escaped.toString(), equalTo(new String(JsonStringEncoder.getInstance().quoteAsString(unescaped.toString()))));
}
else {
assertThat(escaped.toString(), equalTo(unescaped.toString()));
}
String template = XMustacheScriptEngineService.prepareTemplate("{{data}}", contentType);
Map<String, Object> dataMap = new HashMap<>();
dataMap.put("data", unescaped.toString());
Object mustache = engine.compile(template);
Object output = engine.execute(mustache, dataMap);
assertThat(output, notNullValue());
assertThat(output, instanceOf(BytesReference.class));
BytesReference bytes = (BytesReference) output;
String renderedTemplate = bytes.toUtf8();
if (contentType == XContentType.JSON) {
if (!escaped.toString().equals(renderedTemplate)) {
String escapedString = escaped.toString();
for (int l = 0; l < renderedTemplate.length() && l < escapedString.length(); ++l) {
if (renderedTemplate.charAt(l) != escapedString.charAt(l)) {
logger.error("at [{}] expected [{}] but got [{}]", l, renderedTemplate.charAt(l), escapedString.charAt(l));
}
}
}
assertThat(escaped.toString(), equalTo(renderedTemplate));
} else {
assertThat(unescaped.toString(), equalTo(renderedTemplate));
}
}
}
private String getChars() throws IOException {
return randomRealisticUnicodeOfCodepointLengthBetween(0, 10);
}
}