Skip to content

Tweak lang-mustache factory (#92211) #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MitchLewis930 wants to merge 1 commit into
base: pr_014_before
Choose a base branch
from
Open

Tweak lang-mustache factory (#92211) #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions .../lang-mustache/src/main/java/org/elasticsearch/script/mustache/CustomMustacheFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
import com.github.mustachejava.Mustache;
import com.github.mustachejava.MustacheException;
import com.github.mustachejava.MustacheVisitor;
import com.github.mustachejava.SafeMustacheFactory;
import com.github.mustachejava.TemplateContext;
import com.github.mustachejava.TemplateFunction;
import com.github.mustachejava.codes.DefaultMustache;
import com.github.mustachejava.codes.IterableCode;
import com.github.mustachejava.codes.WriteCode;
Expand All @@ -34,12 +36,11 @@
import java.util.Map;
import java.util.Objects;
import java.util.StringJoiner;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class CustomMustacheFactory extends DefaultMustacheFactory {
public class CustomMustacheFactory extends SafeMustacheFactory {
static final String V7_JSON_MEDIA_TYPE_WITH_CHARSET = "application/json; charset=UTF-8";
static final String JSON_MEDIA_TYPE_WITH_CHARSET = "application/json;charset=utf-8";
static final String JSON_MEDIA_TYPE = "application/json";
Expand All @@ -64,7 +65,7 @@ public class CustomMustacheFactory extends DefaultMustacheFactory {
private final Encoder encoder;

public CustomMustacheFactory(String mediaType) {
super();
super(Collections.emptySet(), ".");
setObjectHandler(new CustomReflectionObjectHandler());
this.encoder = createEncoder(mediaType);
}
Expand Down Expand Up @@ -145,7 +146,7 @@ protected void tag(Writer writer, String tag) throws IOException {
writer.write(tc.endChars());
}

protected abstract Function<String, String> createFunction(Object resolved);
protected abstract TemplateFunction createFunction(Object resolved);
Copy link

@coderabbitai coderabbitai bot Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -name "CustomMustacheFactory.java" -type f

Repository: Signal65/elasticsearch-CodeRabbit

Length of output: 175

🏁 Script executed:

cat -n modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/CustomMustacheFactory.java | head -100

Repository: Signal65/elasticsearch-CodeRabbit

Length of output: 4362

🏁 Script executed:

cat -n modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/CustomMustacheFactory.java | sed -n '140,160p'

Repository: Signal65/elasticsearch-CodeRabbit

Length of output: 1042

🏁 Script executed:

cat -n modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/CustomMustacheFactory.java | sed -n '180,220p'

Repository: Signal65/elasticsearch-CodeRabbit

Length of output: 2028

🏁 Script executed:

cat -n modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/CustomMustacheFactory.java | sed -n '235,260p'

Repository: Signal65/elasticsearch-CodeRabbit

Length of output: 1100

🌐 Web query:

mustache.java 0.9.10 TemplateFunction interface documentation behavior

💡 Result:

In mustache.java 0.9.10, TemplateFunction is a marker interface for section lambdas that tells the engine to treat the lambda’s return value as Mustache source and compile/render it again (i.e., “reparse the results again”). [2]

This contrasts with using a plain Java 8 Function<String,String> for a section ({{#func}}...{{/func}}):

  • Function<String,String>: the section body is rendered first (post-substitution) and then passed to apply; the returned string is inserted as text (not recompiled as a Mustache template). [1]
  • TemplateFunction: the lambda is invoked in the pre-substitution mode; the returned string is treated as Mustache template text and is reparsed/re-rendered against the current context. [1], [2]

Practical implication: implement TemplateFunction when you want your lambda to generate Mustache tags (e.g., return "Hello {{name}}") and have those tags evaluated; use plain Function when you only want to transform already-rendered text. [1], [2]

🌐 Web query:

mustache.java TemplateFunction vs Function lambda section rendering escaping

💡 Result:

In mustache.java there are two “lambda section” styles, and they differ mainly in when rendering happens (which directly affects escaping):

Function<String,String> (Java 8 Function) = post-substitution

  • mustache.java renders the section body first (i.e., {{name}} etc. inside the section are already expanded/escaped), then passes the resulting text to your Function (“post-substitution”). (github.com)
  • The Function’s returned string is then written out as plain text (it is not re-parsed as a Mustache template). (github.com)
    Escaping implication: anything inside the section like {{name}} was already escaped by the normal rules before your function sees it; your function output isn’t “auto-escaped” afterward (it’s just output).

TemplateFunction = pre-substitution + reparse

  • TemplateFunction exists specifically so mustache.java will reparse (re-render) the function’s returned string as a Mustache template. (javadoc.io)
  • mustache.java describes this as “reparse the results … again” and contrasts it with Function post-substitution behavior. (github.com)
    Escaping implication: because the returned string is treated as a template, any {{var}} that appears in what you return will be rendered with normal Mustache escaping rules during that re-render (and {{{var}}} / {{& var}} would be unescaped per Mustache rules). (github.com)

If you tell me your exact template snippet + which type you’re using, I can spell out precisely what gets escaped at each step.

Citations:

Investigate whether TemplateFunction is the correct type for these lambdas

In mustache.java 0.9.10, TemplateFunction causes the lambda's returned string to be re-parsed as Mustache template source, whereas Function<String,String> treats the return value as plain text. This distinction is critical:

  • ToJsonCode (lines 190-214): Returns JSON-formatted strings that may contain {{ characters. If re-parsed as templates, literal JSON syntax could be misinterpreted as Mustache directives.
  • JoinerCode (lines 241-255): Returns concatenated strings. If delimiters or items contain {{, they would trigger template re-parsing unintentionally.

Verify that TemplateFunction is the intended type (not a regression from a prior Function<String,String> implementation) and confirm these lambdas do not produce output that would cause unintended re-parsing.

🤖 Prompt for AI Agents 
In
`@modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/CustomMustacheFactory.java`
at line 149, The abstract method createFunction currently returns
mustache.TemplateFunction which causes lambda return values to be re-parsed as
Mustache templates; update the API so these lambdas are plain-text functions:
change the return type of createFunction to
java.util.function.Function<String,String> (or otherwise return a plain text
Function) and adjust implementations for ToJsonCode and JoinerCode to implement
Function<String,String> so their outputs are not re-parsed as templates; update
any call sites that expect TemplateFunction to accept Function<String,String>
and ensure ToJsonCode and JoinerCode produce safe literal strings (no mustache
re-parsing).


/**
* At compile time, this function extracts the name of the variable:
Expand Down Expand Up @@ -188,7 +189,7 @@ static class ToJsonCode extends CustomCode {

@Override
@SuppressWarnings("unchecked")
protected Function<String, String> createFunction(Object resolved) {
protected TemplateFunction createFunction(Object resolved) {
return s -> {
if (resolved == null) {
return null;
Expand Down Expand Up @@ -238,7 +239,7 @@ static class JoinerCode extends CustomCode {
}

@Override
protected Function<String, String> createFunction(Object resolved) {
protected TemplateFunction createFunction(Object resolved) {
return s -> {
if (s == null) {
return null;
Expand All @@ -260,7 +261,7 @@ static boolean match(String variable) {

static class CustomJoinerCode extends JoinerCode {

private static final Pattern PATTERN = Pattern.compile("^(?:" + CODE + " delimiter='(.*)')$");
private static final Pattern PATTERN = Pattern.compile("^" + CODE + " delimiter='(.*)'$");

CustomJoinerCode(TemplateContext tc, DefaultMustacheFactory df, Mustache mustache, String variable) {
super(tc, df, mustache, extractDelimiter(variable));
Expand Down Expand Up @@ -357,7 +358,7 @@ static class UrlEncoder implements Encoder {

@Override
public void encode(String s, Writer writer) throws IOException {
writer.write(URLEncoder.encode(s, StandardCharsets.UTF_8.name()));
writer.write(URLEncoder.encode(s, StandardCharsets.UTF_8));
}
}
}
7 changes: 0 additions & 7 deletions ...stache/src/main/java/org/elasticsearch/script/mustache/CustomReflectionObjectHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import com.github.mustachejava.reflect.ReflectionObjectHandler;

import org.elasticsearch.common.util.CollectionUtils;
import org.elasticsearch.common.util.Maps;
import org.elasticsearch.common.util.iterable.Iterables;

Expand Down Expand Up @@ -144,10 +143,4 @@ public Iterator<Object> iterator() {
return col.iterator();
}
}

@Override
public String stringify(Object object) {
CollectionUtils.ensureNoSelfReferences(object, "CustomReflectionObjectHandler stringify");
return super.stringify(object);
}
}
2 changes: 2 additions & 0 deletions x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/watch/Payload.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package org.elasticsearch.xpack.core.watcher.watch;

import org.elasticsearch.common.collect.MapBuilder;
import org.elasticsearch.common.util.CollectionUtils;
import org.elasticsearch.xcontent.ToXContentObject;
import org.elasticsearch.xcontent.XContentBuilder;

Expand Down Expand Up @@ -37,6 +38,7 @@ public Simple(String key, Object value) {
}

public Simple(Map<String, Object> data) {
CollectionUtils.ensureNoSelfReferences(data, "watcher action payload");
this.data = data;
}

Expand Down
1 change: 1 addition & 0 deletions x-pack/plugin/watcher/qa/rest/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ tasks.named("yamlRestTestV7CompatTransform").configure{ task ->
task.skipTest("mustache/30_search_input/Test search input mustache integration (using request body and rest_total_hits_as_int)", "remove JodaCompatibleDateTime -- ZonedDateTime doesn't output millis/nanos if they're 0 (#78417)")
task.skipTest("mustache/30_search_input/Test search input mustache integration (using request body)", "remove JodaCompatibleDateTime -- ZonedDateTime doesn't output millis/nanos if they're 0 (#78417)")
task.skipTest("mustache/40_search_transform/Test search transform mustache integration (using request body)", "remove JodaCompatibleDateTime -- ZonedDateTime doesn't output millis/nanos if they're 0 (#78417)")
task.skipTest("painless/40_exception/Test painless exceptions are returned when logging a broken response", "Exceptions are no longer thrown from Mustache, but from the transform action itself")
task.replaceKeyInDo("watcher.ack_watch", "xpack-watcher.ack_watch")
task.replaceKeyInDo("watcher.activate_watch", "xpack-watcher.activate_watch")
task.replaceKeyInDo("watcher.deactivate_watch", "xpack-watcher.deactivate_watch")
Expand Down
39 changes: 6 additions & 33 deletions ...n/watcher/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/painless/40_exception.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@

---
"Test painless exceptions are returned when logging a broken response":
- skip:
version: " - 8.6.99"
reason: "self-referencing objects were fixed to be in Painless instead of Mustache in 8.7"

- do:
cluster.health:
wait_for_status: green
Expand Down Expand Up @@ -75,36 +79,5 @@
- match: { watch_record.trigger_event.type: "manual" }
- match: { watch_record.state: "executed" }
- match: { watch_record.result.actions.0.status: "failure" }
- match: { watch_record.result.actions.0.error.caused_by.caused_by.type: "illegal_argument_exception" }
- match: { watch_record.result.actions.0.error.caused_by.caused_by.reason: "Iterable object is self-referencing itself (CustomReflectionObjectHandler stringify)" }

- do:
catch: bad_request
watcher.execute_watch:
body: >
{
"watch": {
"trigger": {
"schedule": {
"interval": "10s"
}
},
"input": {
"simple": {
"foo": "bar"
}
},
"actions": {
"my-logging": {
"transform": {
"script": {
"source": "def x = [:] ; def y = [:] ; x.a = y ; y.a = x ; return x"
}
},
"logging": {
"text": "{{#join}}ctx.payload{{/join}}"
}
}
}
}
}
- match: { watch_record.result.actions.0.reason: "Failed to transform payload" }
- match: { watch_record.result.actions.0.transform.error.reason: "Iterable object is self-referencing itself (watcher action payload)" }