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
This commit is contained in:
Isabel Drost-Fromm 2015-04-27 10:53:36 +02:00
parent 5812753dbc
commit 180403fc32
2 changed files with 75 additions and 44 deletions

View File

@ -18,6 +18,7 @@
*/ */
package org.elasticsearch.script.mustache; package org.elasticsearch.script.mustache;
import com.fasterxml.jackson.core.io.JsonStringEncoder;
import com.github.mustachejava.DefaultMustacheFactory; import com.github.mustachejava.DefaultMustacheFactory;
import com.github.mustachejava.MustacheException; import com.github.mustachejava.MustacheException;
@ -32,36 +33,10 @@ public final class JsonEscapingMustacheFactory extends DefaultMustacheFactory {
@Override @Override
public void encode(String value, Writer writer) { public void encode(String value, Writer writer) {
try { try {
escape(value, writer); JsonStringEncoder utils = new JsonStringEncoder();
writer.write(utils.quoteAsString(value));;
} catch (IOException e) { } catch (IOException e) {
throw new MustacheException("Failed to encode value: " + value); 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;
}
} }

View File

@ -18,7 +18,6 @@
*/ */
package org.elasticsearch.script.mustache; package org.elasticsearch.script.mustache;
import com.carrotsearch.randomizedtesting.generators.RandomPicks;
import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.test.ElasticsearchTestCase; import org.elasticsearch.test.ElasticsearchTestCase;
@ -38,10 +37,12 @@ import static org.hamcrest.Matchers.equalTo;
*/ */
public class MustacheScriptEngineTest extends ElasticsearchTestCase { public class MustacheScriptEngineTest extends ElasticsearchTestCase {
private MustacheScriptEngineService qe; private MustacheScriptEngineService qe;
private JsonEscapingMustacheFactory escaper;
@Before @Before
public void setup() { public void setup() {
qe = new MustacheScriptEngineService(ImmutableSettings.Builder.EMPTY_SETTINGS); qe = new MustacheScriptEngineService(ImmutableSettings.Builder.EMPTY_SETTINGS);
escaper = new JsonEscapingMustacheFactory();
} }
@Test @Test
@ -73,43 +74,98 @@ public class MustacheScriptEngineTest extends ElasticsearchTestCase {
public void testEscapeJson() throws IOException { public void testEscapeJson() throws IOException {
{ {
StringWriter writer = new StringWriter(); StringWriter writer = new StringWriter();
JsonEscapingMustacheFactory.escape("hello \n world", writer); escaper.encode("hello \n world", writer);
assertThat(writer.toString(), equalTo("hello \\\n world")); assertThat(writer.toString(), equalTo("hello \\n world"));
} }
{ {
StringWriter writer = new StringWriter(); StringWriter writer = new StringWriter();
JsonEscapingMustacheFactory.escape("\n", writer); escaper.encode("\n", writer);
assertThat(writer.toString(), equalTo("\\\n")); 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); int iters = scaledRandomIntBetween(100, 1000);
for (int i = 0; i < iters; i++) { for (int i = 0; i < iters; i++) {
int rounds = scaledRandomIntBetween(1, 20); int rounds = scaledRandomIntBetween(1, 20);
StringWriter escaped = new StringWriter(); StringWriter expect = new StringWriter();
StringWriter writer = new StringWriter(); StringWriter writer = new StringWriter();
for (int j = 0; j < rounds; j++) { for (int j = 0; j < rounds; j++) {
String s = getChars(); String s = getChars();
writer.write(s); writer.write(s);
escaped.write(s); expect.write(s);
char c = RandomPicks.randomFrom(getRandom(), specialChars);
writer.append(c); int charIndex = randomInt(7);
escaped.append('\\'); writer.append(specialChars[charIndex]);
escaped.append(c); expect.append(escapedChars[charIndex]);
} }
StringWriter target = new StringWriter(); 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() { private String getChars() {
String string = randomRealisticUnicodeOfCodepointLengthBetween(0, 10); String string = randomRealisticUnicodeOfCodepointLengthBetween(0, 10);
for (int i = 0; i < string.length(); i++) { 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.substring(0, i);
} }
} }
return string; 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;
}
} }