Skip to content

Commit 7571ca4

Browse files
committed
Disable Watcher script optimization for stored scripts (#53497)
The watcher TextTemplateEngine uses a fast path mechanism where it checks for the existence of `{{` to decide if a mustache script 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
1 parent 91ca9c5 commit 7571ca4

File tree

4 files changed

+40
-8
lines changed

4 files changed

+40
-8
lines changed

x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/text/TextTemplate.java

+12-6
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,12 @@ public class TextTemplate implements ToXContent {
2929

3030
private final Script script;
3131
private final String inlineTemplate;
32-
private final boolean isUsingMustache;
32+
private final boolean mayRequireCompilation;
3333

3434
public TextTemplate(String template) {
3535
this.script = null;
3636
this.inlineTemplate = template;
37-
this.isUsingMustache = template.contains("{{");
37+
this.mayRequireCompilation = template.contains("{{");
3838
}
3939

4040
public TextTemplate(String template, @Nullable XContentType contentType, ScriptType type,
@@ -50,14 +50,14 @@ public TextTemplate(String template, @Nullable XContentType contentType, ScriptT
5050
params = new HashMap<>();
5151
}
5252
this.script = new Script(type, type == ScriptType.STORED ? null : Script.DEFAULT_TEMPLATE_LANG, template, options, params);
53-
this.isUsingMustache = template.contains("{{");
5453
this.inlineTemplate = null;
54+
this.mayRequireCompilation = script.getType() == ScriptType.STORED || script.getIdOrCode().contains("{{");
5555
}
5656

5757
public TextTemplate(Script script) {
5858
this.script = script;
5959
this.inlineTemplate = null;
60-
this.isUsingMustache = script.getIdOrCode().contains("{{");
60+
this.mayRequireCompilation = script.getType() == ScriptType.STORED || script.getIdOrCode().contains("{{");
6161
}
6262

6363
public Script getScript() {
@@ -68,8 +68,14 @@ public String getTemplate() {
6868
return script != null ? script.getIdOrCode() : inlineTemplate;
6969
}
7070

71-
public boolean isUsingMustache() {
72-
return isUsingMustache;
71+
/**
72+
* Check if compilation may be required.
73+
* If a stored script is used, we cannot tell at this stage, so we always assume
74+
* that stored scripts require compilation.
75+
* If an inline script is used, we checked for the mustache opening brackets
76+
*/
77+
public boolean mayRequireCompilation() {
78+
return mayRequireCompilation;
7379
}
7480

7581
public XContentType getContentType() {

x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/text/TextTemplateEngine.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public String render(TextTemplate textTemplate, Map<String, Object> model) {
3232
String mediaType = compileParams(detectContentType(template));
3333
template = trimContentType(textTemplate);
3434

35-
if (textTemplate.isUsingMustache() == false) {
35+
if (textTemplate.mayRequireCompilation() == false) {
3636
return template;
3737
}
3838

x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailTemplate.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,7 @@ public boolean handle(String fieldName, XContentParser parser) throws IOExceptio
455455
static void validateEmailAddresses(TextTemplate ... emails) {
456456
for (TextTemplate emailTemplate : emails) {
457457
// no mustache, do validation
458-
if (emailTemplate.isUsingMustache() == false) {
458+
if (emailTemplate.mayRequireCompilation() == false) {
459459
String email = emailTemplate.getTemplate();
460460
try {
461461
for (Email.Address address : Email.AddressList.parse(email)) {

x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/common/text/TextTemplateTests.java

+26
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,32 @@ public void testParserInvalidMissingText() throws Exception {
225225
assertEquals("[1:2] [script] unknown field [type]", ex.getMessage());
226226
}
227227

228+
public void testMustacheTemplateRequiresCompilation() {
229+
final TextTemplate inlineTemplateRequiresCompilation = createTextTemplate(ScriptType.INLINE, "{{foo.bar}}");
230+
assertThat(inlineTemplateRequiresCompilation.mayRequireCompilation(), is(true));
231+
232+
final TextTemplate inlineTemplateNoRequiresCompilation = createTextTemplate(ScriptType.INLINE, "script without mustache");
233+
assertThat(inlineTemplateNoRequiresCompilation.mayRequireCompilation(), is(false));
234+
235+
final TextTemplate storedScriptTemplate = createTextTemplate(ScriptType.STORED, "stored_script_id");
236+
assertThat(storedScriptTemplate.mayRequireCompilation(), is(true));
237+
}
238+
239+
private TextTemplate createTextTemplate(ScriptType type, String idOrCode) {
240+
final TextTemplate template;
241+
if (randomBoolean()) {
242+
if (type == ScriptType.STORED) {
243+
template = new TextTemplate(new Script(type, null, idOrCode, null, Collections.emptyMap()));
244+
} else {
245+
template = new TextTemplate(new Script(type, lang, idOrCode, Collections.emptyMap(), Collections.emptyMap()));
246+
}
247+
} else {
248+
template = new TextTemplate(idOrCode, null, type, null);
249+
}
250+
251+
return template;
252+
}
253+
228254
public void testNullObject() throws Exception {
229255
assertThat(engine.render(null ,new HashMap<>()), is(nullValue()));
230256
}

0 commit comments

Comments
 (0)