Skip to content

Commit 26ca19e

Browse files
jdconradjdconrad
committed
Deprecate Empty Templates (#30194)
Deprecate the use of empty templates. Bug fix allows empty templates/scripts to be loaded on start up for upgrades/restarts, but empty templates can no longer be created.
1 parent 0536c72 commit 26ca19e

File tree

5 files changed

+148
-14
lines changed

5 files changed

+148
-14
lines changed

‎server/src/main/java/org/elasticsearch/script/ScriptMetaData.java

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import org.elasticsearch.common.io.stream.StreamInput;
3030
import org.elasticsearch.common.io.stream.StreamOutput;
3131
import org.elasticsearch.common.io.stream.Writeable;
32+
import org.elasticsearch.common.logging.DeprecationLogger;
33+
import org.elasticsearch.common.logging.Loggers;
3234
import org.elasticsearch.common.xcontent.ToXContentFragment;
3335
import org.elasticsearch.common.xcontent.XContentBuilder;
3436
import org.elasticsearch.common.xcontent.XContentParser;
@@ -46,6 +48,11 @@
4648
*/
4749
public final class ScriptMetaData implements MetaData.Custom, Writeable, ToXContentFragment {
4850

51+
/**
52+
* Standard deprecation logger for used to deprecate allowance of empty templates.
53+
*/
54+
private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(ScriptMetaData.class));
55+
4956
/**
5057
* A builder used to modify the currently stored scripts data held within
5158
* the {@link ClusterState}. Scripts can be added or deleted, then built
@@ -161,8 +168,8 @@ static ScriptMetaData deleteStoredScript(ScriptMetaData previous, String id) {
161168
*
162169
* {@code
163170
* {
164-
* "<id>" : "<{@link StoredScriptSource#fromXContent(XContentParser)}>",
165-
* "<id>" : "<{@link StoredScriptSource#fromXContent(XContentParser)}>",
171+
* "<id>" : "<{@link StoredScriptSource#fromXContent(XContentParser, boolean)}>",
172+
* "<id>" : "<{@link StoredScriptSource#fromXContent(XContentParser, boolean)}>",
166173
* ...
167174
* }
168175
* }
@@ -209,6 +216,14 @@ public static ScriptMetaData fromXContent(XContentParser parser) throws IOExcept
209216
lang = id.substring(0, split);
210217
id = id.substring(split + 1);
211218
source = new StoredScriptSource(lang, parser.text(), Collections.emptyMap());
219+
220+
if (source.getSource().isEmpty()) {
221+
if (source.getLang().equals(Script.DEFAULT_TEMPLATE_LANG)) {
222+
DEPRECATION_LOGGER.deprecated("empty templates should no longer be used");
223+
} else {
224+
DEPRECATION_LOGGER.deprecated("empty scripts should no longer be used");
225+
}
226+
}
212227
}
213228

214229
exists = scripts.get(id);
@@ -231,7 +246,7 @@ public static ScriptMetaData fromXContent(XContentParser parser) throws IOExcept
231246
}
232247

233248
exists = scripts.get(id);
234-
source = StoredScriptSource.fromXContent(parser);
249+
source = StoredScriptSource.fromXContent(parser, true);
235250

236251
if (exists == null) {
237252
scripts.put(id, source);

‎server/src/main/java/org/elasticsearch/script/StoredScriptSource.java

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
import org.elasticsearch.common.io.stream.StreamInput;
3333
import org.elasticsearch.common.io.stream.StreamOutput;
3434
import org.elasticsearch.common.io.stream.Writeable;
35+
import org.elasticsearch.common.logging.DeprecationLogger;
36+
import org.elasticsearch.common.logging.Loggers;
3537
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
3638
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
3739
import org.elasticsearch.common.xcontent.ObjectParser;
@@ -57,6 +59,11 @@
5759
*/
5860
public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> implements Writeable, ToXContentObject {
5961

62+
/**
63+
* Standard deprecation logger for used to deprecate allowance of empty templates.
64+
*/
65+
private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(StoredScriptSource.class));
66+
6067
/**
6168
* Standard {@link ParseField} for outer level of stored script source.
6269
*/
@@ -109,7 +116,7 @@ private void setLang(String lang) {
109116
private void setSource(XContentParser parser) {
110117
try {
111118
if (parser.currentToken() == Token.START_OBJECT) {
112-
//this is really for search templates, that need to be converted to json format
119+
// this is really for search templates, that need to be converted to json format
113120
XContentBuilder builder = XContentFactory.jsonBuilder();
114121
source = Strings.toString(builder.copyCurrentStructure(parser));
115122
options.put(Script.CONTENT_TYPE_OPTION, XContentType.JSON.mediaType());
@@ -131,18 +138,38 @@ private void setOptions(Map<String, String> options) {
131138

132139
/**
133140
* Validates the parameters and creates an {@link StoredScriptSource}.
141+
*
142+
* @param ignoreEmpty Specify as {@code true} to ignoreEmpty the empty source check.
143+
* This allow empty templates to be loaded for backwards compatibility.
144+
* This allow empty templates to be loaded for backwards compatibility.
134145
*/
135-
private StoredScriptSource build() {
146+
private StoredScriptSource build(boolean ignoreEmpty) {
136147
if (lang == null) {
137148
throw new IllegalArgumentException("must specify lang for stored script");
138149
} else if (lang.isEmpty()) {
139150
throw new IllegalArgumentException("lang cannot be empty");
140151
}
141152

142153
if (source == null) {
143-
throw new IllegalArgumentException("must specify source for stored script");
154+
if (ignoreEmpty || Script.DEFAULT_TEMPLATE_LANG.equals(lang)) {
155+
if (Script.DEFAULT_TEMPLATE_LANG.equals(lang)) {
156+
DEPRECATION_LOGGER.deprecated("empty templates should no longer be used");
157+
} else {
158+
DEPRECATION_LOGGER.deprecated("empty scripts should no longer be used");
159+
}
160+
} else {
161+
throw new IllegalArgumentException("must specify source for stored script");
162+
}
144163
} else if (source.isEmpty()) {
145-
throw new IllegalArgumentException("source cannot be empty");
164+
if (ignoreEmpty || Script.DEFAULT_TEMPLATE_LANG.equals(lang)) {
165+
if (Script.DEFAULT_TEMPLATE_LANG.equals(lang)) {
166+
DEPRECATION_LOGGER.deprecated("empty templates should no longer be used");
167+
} else {
168+
DEPRECATION_LOGGER.deprecated("empty scripts should no longer be used");
169+
}
170+
} else {
171+
throw new IllegalArgumentException("source cannot be empty");
172+
}
146173
}
147174

148175
if (options.size() > 1 || options.size() == 1 && options.get(Script.CONTENT_TYPE_OPTION) == null) {
@@ -257,6 +284,8 @@ public static StoredScriptSource parse(BytesReference content, XContentType xCon
257284
token = parser.nextToken();
258285

259286
if (token == Token.END_OBJECT) {
287+
DEPRECATION_LOGGER.deprecated("empty templates should no longer be used");
288+
260289
return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, "", Collections.emptyMap());
261290
}
262291

@@ -271,7 +300,7 @@ public static StoredScriptSource parse(BytesReference content, XContentType xCon
271300
token = parser.nextToken();
272301

273302
if (token == Token.START_OBJECT) {
274-
return PARSER.apply(parser, null).build();
303+
return PARSER.apply(parser, null).build(false);
275304
} else {
276305
throw new ParsingException(parser.getTokenLocation(), "unexpected token [" + token + "], expected [{, <source>]");
277306
}
@@ -280,7 +309,13 @@ public static StoredScriptSource parse(BytesReference content, XContentType xCon
280309
token = parser.nextToken();
281310

282311
if (token == Token.VALUE_STRING) {
283-
return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, parser.text(), Collections.emptyMap());
312+
String source = parser.text();
313+
314+
if (source == null || source.isEmpty()) {
315+
DEPRECATION_LOGGER.deprecated("empty templates should no longer be used");
316+
}
317+
318+
return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, source, Collections.emptyMap());
284319
}
285320
}
286321

@@ -293,7 +328,13 @@ public static StoredScriptSource parse(BytesReference content, XContentType xCon
293328
builder.copyCurrentStructure(parser);
294329
}
295330

296-
return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, Strings.toString(builder), Collections.emptyMap());
331+
String source = Strings.toString(builder);
332+
333+
if (source == null || source.isEmpty()) {
334+
DEPRECATION_LOGGER.deprecated("empty templates should no longer be used");
335+
}
336+
337+
return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, source, Collections.emptyMap());
297338
}
298339
}
299340
} catch (IOException ioe) {
@@ -320,9 +361,12 @@ public static StoredScriptSource parse(BytesReference content, XContentType xCon
320361
*
321362
* Note that the "source" parameter can also handle template parsing including from
322363
* a complex JSON object.
364+
*
365+
* @param ignoreEmpty Specify as {@code true} to ignoreEmpty the empty source check.
366+
* This allows empty templates to be loaded for backwards compatibility.
323367
*/
324-
public static StoredScriptSource fromXContent(XContentParser parser) {
325-
return PARSER.apply(parser, null).build();
368+
public static StoredScriptSource fromXContent(XContentParser parser, boolean ignoreEmpty) {
369+
return PARSER.apply(parser, null).build(ignoreEmpty);
326370
}
327371

328372
/**

‎server/src/test/java/org/elasticsearch/script/ScriptMetaDataTests.java

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import org.elasticsearch.common.bytes.BytesArray;
2323
import org.elasticsearch.common.bytes.BytesReference;
2424
import org.elasticsearch.common.io.stream.Writeable;
25+
import org.elasticsearch.common.logging.DeprecationLogger;
26+
import org.elasticsearch.common.logging.Loggers;
2527
import org.elasticsearch.common.xcontent.DeprecationHandler;
2628
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
2729
import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -130,6 +132,45 @@ public void testBuilder() {
130132
assertEquals("1 + 1", result.getStoredScript("_id").getSource());
131133
}
132134

135+
public void testLoadEmptyScripts() throws IOException {
136+
XContentBuilder builder = XContentFactory.jsonBuilder();
137+
builder.startObject().field("mustache#empty", "").endObject();
138+
XContentParser parser = XContentType.JSON.xContent()
139+
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
140+
BytesReference.bytes(builder).streamInput());
141+
ScriptMetaData.fromXContent(parser);
142+
assertWarnings("empty templates should no longer be used");
143+
144+
builder = XContentFactory.jsonBuilder();
145+
builder.startObject().field("lang#empty", "").endObject();
146+
parser = XContentType.JSON.xContent()
147+
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
148+
BytesReference.bytes(builder).streamInput());
149+
ScriptMetaData.fromXContent(parser);
150+
assertWarnings("empty scripts should no longer be used");
151+
152+
builder = XContentFactory.jsonBuilder();
153+
builder.startObject().startObject("script").field("lang", "lang").field("source", "").endObject().endObject();
154+
parser = XContentType.JSON.xContent()
155+
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
156+
BytesReference.bytes(builder).streamInput());
157+
ScriptMetaData.fromXContent(parser);
158+
assertWarnings("empty scripts should no longer be used");
159+
160+
builder = XContentFactory.jsonBuilder();
161+
builder.startObject().startObject("script").field("lang", "mustache").field("source", "").endObject().endObject();
162+
parser = XContentType.JSON.xContent()
163+
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
164+
BytesReference.bytes(builder).streamInput());
165+
ScriptMetaData.fromXContent(parser);
166+
assertWarnings("empty templates should no longer be used");
167+
}
168+
169+
@Override
170+
protected boolean enableWarningsCheck() {
171+
return true;
172+
}
173+
133174
private ScriptMetaData randomScriptMetaData(XContentType sourceContentType, int minNumberScripts) throws IOException {
134175
ScriptMetaData.Builder builder = new ScriptMetaData.Builder(null);
135176
int numScripts = scaledRandomIntBetween(minNumberScripts, 32);

‎server/src/test/java/org/elasticsearch/script/StoredScriptSourceTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ protected StoredScriptSource createTestInstance() {
5858

5959
@Override
6060
protected StoredScriptSource doParseInstance(XContentParser parser) {
61-
return StoredScriptSource.fromXContent(parser);
61+
return StoredScriptSource.fromXContent(parser, false);
6262
}
6363

6464
@Override

‎server/src/test/java/org/elasticsearch/script/StoredScriptTests.java

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.elasticsearch.common.xcontent.XContentType;
3030
import org.elasticsearch.test.AbstractSerializingTestCase;
3131

32+
import java.io.IOException;
3233
import java.util.Collections;
3334
import java.util.HashMap;
3435
import java.util.Map;
@@ -204,6 +205,39 @@ public void testSourceParsingErrors() throws Exception {
204205
}
205206
}
206207

208+
public void testEmptyTemplateDeprecations() throws IOException {
209+
try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) {
210+
builder.startObject().endObject();
211+
212+
StoredScriptSource parsed = StoredScriptSource.parse(BytesReference.bytes(builder), XContentType.JSON);
213+
StoredScriptSource source = new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, "", Collections.emptyMap());
214+
215+
assertThat(parsed, equalTo(source));
216+
assertWarnings("empty templates should no longer be used");
217+
}
218+
219+
try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) {
220+
builder.startObject().field("template", "").endObject();
221+
222+
StoredScriptSource parsed = StoredScriptSource.parse(BytesReference.bytes(builder), XContentType.JSON);
223+
StoredScriptSource source = new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, "", Collections.emptyMap());
224+
225+
assertThat(parsed, equalTo(source));
226+
assertWarnings("empty templates should no longer be used");
227+
}
228+
229+
try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) {
230+
builder.startObject().field("script").startObject().field("lang", "mustache")
231+
.field("source", "").endObject().endObject();
232+
233+
StoredScriptSource parsed = StoredScriptSource.parse(BytesReference.bytes(builder), XContentType.JSON);
234+
StoredScriptSource source = new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, "", Collections.emptyMap());
235+
236+
assertThat(parsed, equalTo(source));
237+
assertWarnings("empty templates should no longer be used");
238+
}
239+
}
240+
207241
@Override
208242
protected StoredScriptSource createTestInstance() {
209243
return new StoredScriptSource(
@@ -219,7 +253,7 @@ protected Writeable.Reader<StoredScriptSource> instanceReader() {
219253

220254
@Override
221255
protected StoredScriptSource doParseInstance(XContentParser parser) {
222-
return StoredScriptSource.fromXContent(parser);
256+
return StoredScriptSource.fromXContent(parser, false);
223257
}
224258

225259
@Override

0 commit comments

Comments
 (0)