From e917e006612c4ca12ac93d859207b28550b36211 Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Thu, 12 Mar 2020 17:53:46 +0100 Subject: [PATCH] Disable Watcher script optimization for stored scripts The watcher TextTemplateEngine uses a fast path mechanism where it checks for the existence of `{{` to decide if a mustache scripts required compilation. This does not work for stored script, as the field that is checked contains the id of the script, which means, the name of the script is returned as its value. This commit checks for the script type and does not involve this fast path check if a stored script is used. Closes #40212 --- .../watcher/common/text/TextTemplate.java | 18 ++++++++----- .../common/text/TextTemplateEngine.java | 2 +- .../notification/email/EmailTemplate.java | 2 +- .../common/text/TextTemplateTests.java | 26 +++++++++++++++++++ 4 files changed, 40 insertions(+), 8 deletions(-) diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/text/TextTemplate.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/text/TextTemplate.java index 224dcc7140d2a..a261e6d01af1c 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/text/TextTemplate.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/text/TextTemplate.java @@ -29,12 +29,12 @@ public class TextTemplate implements ToXContent { private final Script script; private final String inlineTemplate; - private final boolean isUsingMustache; + private final boolean mayRequireCompilation; public TextTemplate(String template) { this.script = null; this.inlineTemplate = template; - this.isUsingMustache = template.contains("{{"); + this.mayRequireCompilation = template.contains("{{"); } public TextTemplate(String template, @Nullable XContentType contentType, ScriptType type, @@ -50,14 +50,14 @@ public TextTemplate(String template, @Nullable XContentType contentType, ScriptT params = new HashMap<>(); } this.script = new Script(type, type == ScriptType.STORED ? null : Script.DEFAULT_TEMPLATE_LANG, template, options, params); - this.isUsingMustache = template.contains("{{"); this.inlineTemplate = null; + this.mayRequireCompilation = script.getType() == ScriptType.STORED || script.getIdOrCode().contains("{{"); } public TextTemplate(Script script) { this.script = script; this.inlineTemplate = null; - this.isUsingMustache = script.getIdOrCode().contains("{{"); + this.mayRequireCompilation = script.getType() == ScriptType.STORED || script.getIdOrCode().contains("{{"); } public Script getScript() { @@ -68,8 +68,14 @@ public String getTemplate() { return script != null ? script.getIdOrCode() : inlineTemplate; } - public boolean isUsingMustache() { - return isUsingMustache; + /** + * Check if compilation may be required. + * If a stored script is used, we cannot tell at this stage, so we always assume + * that stored scripts require compilation. + * If an inline script is used, we checked for the mustache opening brackets + */ + public boolean mayRequireCompilation() { + return mayRequireCompilation; } public XContentType getContentType() { diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/text/TextTemplateEngine.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/text/TextTemplateEngine.java index 2f2d3d7b9f31a..fed102f1d5c1f 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/text/TextTemplateEngine.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/text/TextTemplateEngine.java @@ -32,7 +32,7 @@ public String render(TextTemplate textTemplate, Map model) { String mediaType = compileParams(detectContentType(template)); template = trimContentType(textTemplate); - if (textTemplate.isUsingMustache() == false) { + if (textTemplate.mayRequireCompilation() == false) { return template; } diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailTemplate.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailTemplate.java index dc48dd2b9f23f..2735ad9fc93b1 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailTemplate.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailTemplate.java @@ -455,7 +455,7 @@ public boolean handle(String fieldName, XContentParser parser) throws IOExceptio static void validateEmailAddresses(TextTemplate ... emails) { for (TextTemplate emailTemplate : emails) { // no mustache, do validation - if (emailTemplate.isUsingMustache() == false) { + if (emailTemplate.mayRequireCompilation() == false) { String email = emailTemplate.getTemplate(); try { for (Email.Address address : Email.AddressList.parse(email)) { diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/common/text/TextTemplateTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/common/text/TextTemplateTests.java index 2ebaccc61801c..a563695732a18 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/common/text/TextTemplateTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/common/text/TextTemplateTests.java @@ -225,6 +225,32 @@ public void testParserInvalidMissingText() throws Exception { assertEquals("[1:2] [script] unknown field [type]", ex.getMessage()); } + public void testMustacheTemplateRequiresCompilation() { + final TextTemplate inlineTemplateRequiresCompilation = createTextTemplate(ScriptType.INLINE, "{{foo.bar}}"); + assertThat(inlineTemplateRequiresCompilation.mayRequireCompilation(), is(true)); + + final TextTemplate inlineTemplateNoRequiresCompilation = createTextTemplate(ScriptType.INLINE, "script without mustache"); + assertThat(inlineTemplateNoRequiresCompilation.mayRequireCompilation(), is(false)); + + final TextTemplate storedScriptTemplate = createTextTemplate(ScriptType.STORED, "stored_script_id"); + assertThat(storedScriptTemplate.mayRequireCompilation(), is(true)); + } + + private TextTemplate createTextTemplate(ScriptType type, String idOrCode) { + final TextTemplate template; + if (randomBoolean()) { + if (type == ScriptType.STORED) { + template = new TextTemplate(new Script(type, null, idOrCode, null, Collections.emptyMap())); + } else { + template = new TextTemplate(new Script(type, lang, idOrCode, Collections.emptyMap(), Collections.emptyMap())); + } + } else { + template = new TextTemplate(idOrCode, null, type, null); + } + + return template; + } + public void testNullObject() throws Exception { assertThat(engine.render(null ,new HashMap<>()), is(nullValue())); }