Deprecate accepting malformed requests in stored script API (#28939)

The stored scripts API today accepts malformed requests instead of throwing an exception.
This PR deprecates accepting malformed put stored script requests (requests not using the official script format).

Relates to #27612
This commit is contained in:
Sohaib Iftikhar 2018-05-29 15:45:53 +02:00 committed by Martijn van Groningen
parent eaee530778
commit 3c918d799c
5 changed files with 59 additions and 25 deletions

View File

@ -198,6 +198,7 @@ public class SearchTemplateIT extends ESSingleNodeTestCase {
getResponse = client().admin().cluster().prepareGetStoredScript("testTemplate").get(); getResponse = client().admin().cluster().prepareGetStoredScript("testTemplate").get();
assertNull(getResponse.getSource()); assertNull(getResponse.getSource());
assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element.");
} }
public void testIndexedTemplate() throws Exception { public void testIndexedTemplate() throws Exception {
@ -267,6 +268,7 @@ public class SearchTemplateIT extends ESSingleNodeTestCase {
.setScript("2").setScriptType(ScriptType.STORED).setScriptParams(templateParams) .setScript("2").setScriptType(ScriptType.STORED).setScriptParams(templateParams)
.get(); .get();
assertHitCount(searchResponse.getResponse(), 1); assertHitCount(searchResponse.getResponse(), 1);
assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element.");
} }
// Relates to #10397 // Relates to #10397
@ -311,6 +313,7 @@ public class SearchTemplateIT extends ESSingleNodeTestCase {
.get(); .get();
assertHitCount(searchResponse.getResponse(), 1); assertHitCount(searchResponse.getResponse(), 1);
} }
assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element.");
} }
public void testIndexedTemplateWithArray() throws Exception { public void testIndexedTemplateWithArray() throws Exception {
@ -339,6 +342,7 @@ public class SearchTemplateIT extends ESSingleNodeTestCase {
.setScript("4").setScriptType(ScriptType.STORED).setScriptParams(arrayTemplateParams) .setScript("4").setScriptType(ScriptType.STORED).setScriptParams(arrayTemplateParams)
.get(); .get();
assertHitCount(searchResponse.getResponse(), 5); assertHitCount(searchResponse.getResponse(), 5);
assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element.");
} }
} }

View File

@ -74,6 +74,11 @@ public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> imp
*/ */
public static final ParseField TEMPLATE_PARSE_FIELD = new ParseField("template"); public static final ParseField TEMPLATE_PARSE_FIELD = new ParseField("template");
/**
* Standard {@link ParseField} for query on the inner field.
*/
public static final ParseField TEMPLATE_NO_WRAPPER_PARSE_FIELD = new ParseField("query");
/** /**
* Standard {@link ParseField} for lang on the inner level. * Standard {@link ParseField} for lang on the inner level.
*/ */
@ -189,6 +194,26 @@ public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> imp
PARSER.declareField(Builder::setOptions, XContentParser::mapStrings, OPTIONS_PARSE_FIELD, ValueType.OBJECT); PARSER.declareField(Builder::setOptions, XContentParser::mapStrings, OPTIONS_PARSE_FIELD, ValueType.OBJECT);
} }
private static StoredScriptSource parseRemaining(Token token, XContentParser parser) throws IOException {
try (XContentBuilder builder = XContentFactory.jsonBuilder()) {
if (token != Token.START_OBJECT) {
builder.startObject();
builder.copyCurrentStructure(parser);
builder.endObject();
} else {
builder.copyCurrentStructure(parser);
}
String source = Strings.toString(builder);
if (source == null || source.isEmpty()) {
DEPRECATION_LOGGER.deprecated("empty templates should no longer be used");
}
return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, source, Collections.emptyMap());
}
}
/** /**
* This will parse XContent into a {@link StoredScriptSource}. The following formats can be parsed: * This will parse XContent into a {@link StoredScriptSource}. The following formats can be parsed:
* *
@ -304,10 +329,11 @@ public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> imp
} else { } else {
throw new ParsingException(parser.getTokenLocation(), "unexpected token [" + token + "], expected [{, <source>]"); throw new ParsingException(parser.getTokenLocation(), "unexpected token [" + token + "], expected [{, <source>]");
} }
} else { } else if (TEMPLATE_PARSE_FIELD.getPreferredName().equals(name)) {
if (TEMPLATE_PARSE_FIELD.getPreferredName().equals(name)) {
token = parser.nextToken();
DEPRECATION_LOGGER.deprecated("the template context is now deprecated. Specify templates in a \"script\" element.");
token = parser.nextToken();
if (token == Token.VALUE_STRING) { if (token == Token.VALUE_STRING) {
String source = parser.text(); String source = parser.text();
@ -316,26 +342,15 @@ public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> imp
} }
return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, source, Collections.emptyMap()); return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, source, Collections.emptyMap());
}
}
try (XContentBuilder builder = XContentFactory.jsonBuilder()) {
if (token != Token.START_OBJECT) {
builder.startObject();
builder.copyCurrentStructure(parser);
builder.endObject();
} else { } else {
builder.copyCurrentStructure(parser); return parseRemaining(token, parser);
}
String source = Strings.toString(builder);
if (source == null || source.isEmpty()) {
DEPRECATION_LOGGER.deprecated("empty templates should no longer be used");
}
return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, source, Collections.emptyMap());
} }
} else if (TEMPLATE_NO_WRAPPER_PARSE_FIELD.getPreferredName().equals(name)) {
DEPRECATION_LOGGER.deprecated("the template context is now deprecated. Specify templates in a \"script\" element.");
return parseRemaining(token, parser);
} else {
DEPRECATION_LOGGER.deprecated("scripts should not be stored without a context. Specify them in a \"script\" element.");
return parseRemaining(token, parser);
} }
} catch (IOException ioe) { } catch (IOException ioe) {
throw new UncheckedIOException(ioe); throw new UncheckedIOException(ioe);

View File

@ -81,10 +81,12 @@ public class ScriptMetaDataTests extends AbstractSerializingTestCase<ScriptMetaD
XContentBuilder sourceBuilder = XContentFactory.jsonBuilder(); XContentBuilder sourceBuilder = XContentFactory.jsonBuilder();
sourceBuilder.startObject().startObject("template").field("field", "value").endObject().endObject(); sourceBuilder.startObject().startObject("template").field("field", "value").endObject().endObject();
builder.storeScript("template", StoredScriptSource.parse(BytesReference.bytes(sourceBuilder), sourceBuilder.contentType())); builder.storeScript("template", StoredScriptSource.parse(BytesReference.bytes(sourceBuilder), sourceBuilder.contentType()));
assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element.");
sourceBuilder = XContentFactory.jsonBuilder(); sourceBuilder = XContentFactory.jsonBuilder();
sourceBuilder.startObject().field("template", "value").endObject(); sourceBuilder.startObject().field("template", "value").endObject();
builder.storeScript("template_field", StoredScriptSource.parse(BytesReference.bytes(sourceBuilder), sourceBuilder.contentType())); builder.storeScript("template_field", StoredScriptSource.parse(BytesReference.bytes(sourceBuilder), sourceBuilder.contentType()));
assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element.");
sourceBuilder = XContentFactory.jsonBuilder(); sourceBuilder = XContentFactory.jsonBuilder();
sourceBuilder.startObject().startObject("script").field("lang", "_lang").field("source", "_source").endObject().endObject(); sourceBuilder.startObject().startObject("script").field("lang", "_lang").field("source", "_source").endObject().endObject();
@ -99,14 +101,19 @@ public class ScriptMetaDataTests extends AbstractSerializingTestCase<ScriptMetaD
public void testDiff() throws Exception { public void testDiff() throws Exception {
ScriptMetaData.Builder builder = new ScriptMetaData.Builder(null); ScriptMetaData.Builder builder = new ScriptMetaData.Builder(null);
builder.storeScript("1", StoredScriptSource.parse(new BytesArray("{\"foo\":\"abc\"}"), XContentType.JSON)); builder.storeScript("1", StoredScriptSource.parse(new BytesArray("{\"foo\":\"abc\"}"), XContentType.JSON));
assertWarnings("scripts should not be stored without a context. Specify them in a \"script\" element.");
builder.storeScript("2", StoredScriptSource.parse(new BytesArray("{\"foo\":\"def\"}"), XContentType.JSON)); builder.storeScript("2", StoredScriptSource.parse(new BytesArray("{\"foo\":\"def\"}"), XContentType.JSON));
assertWarnings("scripts should not be stored without a context. Specify them in a \"script\" element.");
builder.storeScript("3", StoredScriptSource.parse(new BytesArray("{\"foo\":\"ghi\"}"), XContentType.JSON)); builder.storeScript("3", StoredScriptSource.parse(new BytesArray("{\"foo\":\"ghi\"}"), XContentType.JSON));
assertWarnings("scripts should not be stored without a context. Specify them in a \"script\" element.");
ScriptMetaData scriptMetaData1 = builder.build(); ScriptMetaData scriptMetaData1 = builder.build();
builder = new ScriptMetaData.Builder(scriptMetaData1); builder = new ScriptMetaData.Builder(scriptMetaData1);
builder.storeScript("2", StoredScriptSource.parse(new BytesArray("{\"foo\":\"changed\"}"), XContentType.JSON)); builder.storeScript("2", StoredScriptSource.parse(new BytesArray("{\"foo\":\"changed\"}"), XContentType.JSON));
assertWarnings("scripts should not be stored without a context. Specify them in a \"script\" element.");
builder.deleteScript("3"); builder.deleteScript("3");
builder.storeScript("4", StoredScriptSource.parse(new BytesArray("{\"foo\":\"jkl\"}"), XContentType.JSON)); builder.storeScript("4", StoredScriptSource.parse(new BytesArray("{\"foo\":\"jkl\"}"), XContentType.JSON));
assertWarnings("scripts should not be stored without a context. Specify them in a \"script\" element.");
ScriptMetaData scriptMetaData2 = builder.build(); ScriptMetaData scriptMetaData2 = builder.build();
ScriptMetaData.ScriptMetadataDiff diff = (ScriptMetaData.ScriptMetadataDiff) scriptMetaData2.diff(scriptMetaData1); ScriptMetaData.ScriptMetadataDiff diff = (ScriptMetaData.ScriptMetadataDiff) scriptMetaData2.diff(scriptMetaData1);

View File

@ -50,7 +50,9 @@ public class StoredScriptSourceTests extends AbstractSerializingTestCase<StoredS
if (randomBoolean()) { if (randomBoolean()) {
options.put(Script.CONTENT_TYPE_OPTION, xContentType.mediaType()); options.put(Script.CONTENT_TYPE_OPTION, xContentType.mediaType());
} }
return StoredScriptSource.parse(BytesReference.bytes(template), xContentType); StoredScriptSource source = StoredScriptSource.parse(BytesReference.bytes(template), xContentType);
assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element.");
return source;
} catch (IOException e) { } catch (IOException e) {
throw new AssertionError("Failed to create test instance", e); throw new AssertionError("Failed to create test instance", e);
} }

View File

@ -74,6 +74,7 @@ public class StoredScriptTests extends AbstractSerializingTestCase<StoredScriptS
StoredScriptSource source = new StoredScriptSource("mustache", "code", Collections.emptyMap()); StoredScriptSource source = new StoredScriptSource("mustache", "code", Collections.emptyMap());
assertThat(parsed, equalTo(source)); assertThat(parsed, equalTo(source));
assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element.");
} }
// complex template with wrapper template object // complex template with wrapper template object
@ -89,6 +90,7 @@ public class StoredScriptTests extends AbstractSerializingTestCase<StoredScriptS
StoredScriptSource source = new StoredScriptSource("mustache", code, Collections.emptyMap()); StoredScriptSource source = new StoredScriptSource("mustache", code, Collections.emptyMap());
assertThat(parsed, equalTo(source)); assertThat(parsed, equalTo(source));
assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element.");
} }
// complex template with no wrapper object // complex template with no wrapper object
@ -104,6 +106,7 @@ public class StoredScriptTests extends AbstractSerializingTestCase<StoredScriptS
StoredScriptSource source = new StoredScriptSource("mustache", code, Collections.emptyMap()); StoredScriptSource source = new StoredScriptSource("mustache", code, Collections.emptyMap());
assertThat(parsed, equalTo(source)); assertThat(parsed, equalTo(source));
assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element.");
} }
// complex template using script as the field name // complex template using script as the field name
@ -223,7 +226,10 @@ public class StoredScriptTests extends AbstractSerializingTestCase<StoredScriptS
StoredScriptSource source = new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, "", Collections.emptyMap()); StoredScriptSource source = new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, "", Collections.emptyMap());
assertThat(parsed, equalTo(source)); assertThat(parsed, equalTo(source));
assertWarnings("empty templates should no longer be used"); assertWarnings(
"the template context is now deprecated. Specify templates in a \"script\" element.",
"empty templates should no longer be used"
);
} }
try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) {