Skip to content
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

Deprecate accepting malformed requests in stored script API #28939

Merged
merged 7 commits into from
May 29, 2018
Merged

Deprecate accepting malformed requests in stored script API #28939

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c5510ea
removed support for random scripts. Fixes #27612
sohaibiftikhar Mar 6, 2018
1276bb2
Merge branch 'master' of https://github.com/elastic/elasticsearch int…
sohaibiftikhar May 17, 2018
fa73767
deprecate template context and no context for stored scripts
sohaibiftikhar May 18, 2018
b57f473
fixed tests
sohaibiftikhar May 19, 2018
953b404
Fixed deprecation message.
sohaibiftikhar May 22, 2018
c23ee06
Merge branch 'master' of https://github.com/elastic/elasticsearch int…
sohaibiftikhar May 24, 2018
8e72145
added warnings to mustache lang tests
sohaibiftikhar May 24, 2018
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
4 changes: 4 additions & 0 deletions modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ public void testIndexedTemplateClient() throws Exception {

getResponse = client().admin().cluster().prepareGetStoredScript("testTemplate").get();
assertNull(getResponse.getSource());
assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element.");
}

public void testIndexedTemplate() throws Exception {
Expand Down Expand Up @@ -267,6 +268,7 @@ public void testIndexedTemplate() throws Exception {
.setScript("2").setScriptType(ScriptType.STORED).setScriptParams(templateParams)
.get();
assertHitCount(searchResponse.getResponse(), 1);
assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element.");
}

// Relates to #10397
Expand Down Expand Up @@ -311,6 +313,7 @@ public void testIndexedTemplateOverwrite() throws Exception {
.get();
assertHitCount(searchResponse.getResponse(), 1);
}
assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element.");
}

public void testIndexedTemplateWithArray() throws Exception {
Expand Down Expand Up @@ -339,6 +342,7 @@ public void testIndexedTemplateWithArray() throws Exception {
.setScript("4").setScriptType(ScriptType.STORED).setScriptParams(arrayTemplateParams)
.get();
assertHitCount(searchResponse.getResponse(), 5);
assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element.");
}

}
61 changes: 38 additions & 23 deletions server/src/main/java/org/elasticsearch/script/StoredScriptSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> imp
*/
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.
*/
Expand Down Expand Up @@ -189,6 +194,26 @@ private StoredScriptSource build(boolean ignoreEmpty) {
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:
*
Expand Down Expand Up @@ -304,38 +329,28 @@ public static StoredScriptSource parse(BytesReference content, XContentType xCon
} else {
throw new ParsingException(parser.getTokenLocation(), "unexpected token [" + token + "], expected [{, <source>]");
}
} else {
if (TEMPLATE_PARSE_FIELD.getPreferredName().equals(name)) {
token = parser.nextToken();

if (token == Token.VALUE_STRING) {
String source = parser.text();

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_PARSE_FIELD.getPreferredName().equals(name)) {

try (XContentBuilder builder = XContentFactory.jsonBuilder()) {
if (token != Token.START_OBJECT) {
builder.startObject();
builder.copyCurrentStructure(parser);
builder.endObject();
} else {
builder.copyCurrentStructure(parser);
}
DEPRECATION_LOGGER.deprecated("the template context is now deprecated. Specify templates in a \"script\" element.");

String source = Strings.toString(builder);
token = parser.nextToken();
if (token == Token.VALUE_STRING) {
String source = parser.text();

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 {
return parseRemaining(token, parser);
}
} 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) {
throw new UncheckedIOException(ioe);
Expand Down
7 changes: 7 additions & 0 deletions server/src/test/java/org/elasticsearch/script/ScriptMetaDataTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,12 @@ public void testGetScript() throws Exception {
XContentBuilder sourceBuilder = XContentFactory.jsonBuilder();
sourceBuilder.startObject().startObject("template").field("field", "value").endObject().endObject();
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.startObject().field("template", "value").endObject();
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.startObject().startObject("script").field("lang", "_lang").field("source", "_source").endObject().endObject();
Expand All @@ -99,14 +101,19 @@ public void testGetScript() throws Exception {
public void testDiff() throws Exception {
ScriptMetaData.Builder builder = new ScriptMetaData.Builder(null);
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));
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));
assertWarnings("scripts should not be stored without a context. Specify them in a \"script\" element.");
ScriptMetaData scriptMetaData1 = builder.build();

builder = new ScriptMetaData.Builder(scriptMetaData1);
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.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.ScriptMetadataDiff diff = (ScriptMetaData.ScriptMetadataDiff) scriptMetaData2.diff(scriptMetaData1);
Expand Down
4 changes: 3 additions & 1 deletion server/src/test/java/org/elasticsearch/script/StoredScriptSourceTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ protected StoredScriptSource createTestInstance() {
if (randomBoolean()) {
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) {
throw new AssertionError("Failed to create test instance", e);
}
Expand Down
8 changes: 7 additions & 1 deletion server/src/test/java/org/elasticsearch/script/StoredScriptTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ public void testSourceParsing() throws Exception {
StoredScriptSource source = new StoredScriptSource("mustache", "code", Collections.emptyMap());

assertThat(parsed, equalTo(source));
assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element.");
}

// complex template with wrapper template object
Expand All @@ -89,6 +90,7 @@ public void testSourceParsing() throws Exception {
StoredScriptSource source = new StoredScriptSource("mustache", code, Collections.emptyMap());

assertThat(parsed, equalTo(source));
assertWarnings("the template context is now deprecated. Specify templates in a \"script\" element.");
}

// complex template with no wrapper object
Expand All @@ -104,6 +106,7 @@ public void testSourceParsing() throws Exception {
StoredScriptSource source = new StoredScriptSource("mustache", code, Collections.emptyMap());

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
Expand Down Expand Up @@ -223,7 +226,10 @@ public void testEmptyTemplateDeprecations() throws IOException {
StoredScriptSource source = new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, "", Collections.emptyMap());

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)) {
Expand Down