From 30a85cc06754ab8573c293acd25905c136c36128 Mon Sep 17 00:00:00 2001 From: Sohaib Iftikhar Date: Tue, 29 May 2018 15:45:53 +0200 Subject: [PATCH] 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 --- .../script/mustache/SearchTemplateIT.java | 4 ++ .../script/StoredScriptSource.java | 61 ++++++++++++------- .../script/ScriptMetaDataTests.java | 7 +++ .../script/StoredScriptSourceTests.java | 4 +- .../script/StoredScriptTests.java | 8 ++- 5 files changed, 59 insertions(+), 25 deletions(-) diff --git a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java index fe2fedf62b559..884e26e7df855 100644 --- a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java +++ b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java @@ -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 { @@ -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 @@ -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 { @@ -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."); } } diff --git a/server/src/main/java/org/elasticsearch/script/StoredScriptSource.java b/server/src/main/java/org/elasticsearch/script/StoredScriptSource.java index da6dad1dff384..11f8769c86b1f 100644 --- a/server/src/main/java/org/elasticsearch/script/StoredScriptSource.java +++ b/server/src/main/java/org/elasticsearch/script/StoredScriptSource.java @@ -74,6 +74,11 @@ public class StoredScriptSource extends AbstractDiffable 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. */ @@ -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: * @@ -304,38 +329,28 @@ public static StoredScriptSource parse(BytesReference content, XContentType xCon } else { throw new ParsingException(parser.getTokenLocation(), "unexpected token [" + token + "], expected [{, ]"); } - } 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); diff --git a/server/src/test/java/org/elasticsearch/script/ScriptMetaDataTests.java b/server/src/test/java/org/elasticsearch/script/ScriptMetaDataTests.java index 32d4d48a44810..bef20190acf87 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptMetaDataTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptMetaDataTests.java @@ -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(); @@ -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); diff --git a/server/src/test/java/org/elasticsearch/script/StoredScriptSourceTests.java b/server/src/test/java/org/elasticsearch/script/StoredScriptSourceTests.java index 8aa4ca57acfed..49e2623626895 100644 --- a/server/src/test/java/org/elasticsearch/script/StoredScriptSourceTests.java +++ b/server/src/test/java/org/elasticsearch/script/StoredScriptSourceTests.java @@ -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); } diff --git a/server/src/test/java/org/elasticsearch/script/StoredScriptTests.java b/server/src/test/java/org/elasticsearch/script/StoredScriptTests.java index 79e3195f3d923..04483c869d9b3 100644 --- a/server/src/test/java/org/elasticsearch/script/StoredScriptTests.java +++ b/server/src/test/java/org/elasticsearch/script/StoredScriptTests.java @@ -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 @@ -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 @@ -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 @@ -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)) {