No longer add illegal content type option to stored search templates (#24251)

When parsing StoredSearchScript we were adding a Content type option that was forbidden (by a check that threw an exception) by the parser thats used to parse the template when we read it from the cluster state. This was stopping Elastisearch from starting after stored search templates had been added.

This change no longer adds the content type option to the StoredScriptSource object when parsing from the put search template request.  This is safe because the StoredScriptSource content is always JSON when its stored in the cluster state since we do a conversion to JSON before this point.

Also removes the check for the content type in the options when parsing StoredScriptSource so users who already have stored scripts can start Elasticsearch.

Closes #24227
This commit is contained in:
Colin Goodheart-Smithe 2017-04-22 18:37:04 +01:00 committed by Nik Everett
parent c5396839c4
commit d4a6ba8ec9
3 changed files with 75 additions and 29 deletions

View File

@ -123,10 +123,6 @@ public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> imp
* Appends the user-defined compiler options with the internal compiler options.
*/
private void setOptions(Map<String, String> options) {
if (options.containsKey(Script.CONTENT_TYPE_OPTION)) {
throw new IllegalArgumentException(Script.CONTENT_TYPE_OPTION + " cannot be user-specified");
}
this.options.putAll(options);
}
@ -266,8 +262,7 @@ public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> imp
//this is really for search templates, that need to be converted to json format
try (XContentBuilder builder = XContentFactory.jsonBuilder()) {
builder.copyCurrentStructure(parser);
return new StoredScriptSource(lang, builder.string(),
Collections.singletonMap(Script.CONTENT_TYPE_OPTION, XContentType.JSON.mediaType()));
return new StoredScriptSource(lang, builder.string(), Collections.emptyMap());
}
}
@ -283,8 +278,7 @@ public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> imp
token = parser.nextToken();
if (token == Token.VALUE_STRING) {
return new StoredScriptSource(lang, parser.text(),
Collections.singletonMap(Script.CONTENT_TYPE_OPTION, XContentType.JSON.mediaType()));
return new StoredScriptSource(lang, parser.text(), Collections.emptyMap());
}
}
@ -297,8 +291,7 @@ public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> imp
builder.copyCurrentStructure(parser);
}
return new StoredScriptSource(lang, builder.string(),
Collections.singletonMap(Script.CONTENT_TYPE_OPTION, XContentType.JSON.mediaType()));
return new StoredScriptSource(lang, builder.string(), Collections.emptyMap());
}
}
} catch (IOException ioe) {

View File

@ -0,0 +1,68 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.script;
import org.elasticsearch.common.io.stream.Writeable.Reader;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.test.AbstractSerializingTestCase;
import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
public class StoredScriptSourceTests extends AbstractSerializingTestCase<StoredScriptSource> {
@Override
protected StoredScriptSource createTestInstance() {
String lang = randomAlphaOfLengthBetween(1, 20);
XContentType xContentType = randomFrom(XContentType.JSON, XContentType.YAML);
try {
XContentBuilder template = XContentBuilder.builder(xContentType.xContent());
template.startObject();
template.startObject("query");
template.startObject("match");
template.field("title", "{{query_string}}");
template.endObject();
template.endObject();
template.endObject();
Map<String, String> options = new HashMap<>();
if (randomBoolean()) {
options.put(Script.CONTENT_TYPE_OPTION, xContentType.mediaType());
}
return StoredScriptSource.parse(lang, template.bytes(), xContentType);
} catch (IOException e) {
throw new AssertionError("Failed to create test instance", e);
}
}
@Override
protected StoredScriptSource doParseInstance(XContentParser parser) throws IOException {
return StoredScriptSource.fromXContent(parser);
}
@Override
protected Reader<StoredScriptSource> instanceReader() {
return StoredScriptSource::new;
}
}

View File

@ -20,7 +20,6 @@
package org.elasticsearch.script;
import org.elasticsearch.ResourceNotFoundException;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
@ -198,8 +197,7 @@ public class StoredScriptTests extends AbstractSerializingTestCase<StoredScriptS
builder.startObject().field("template", "code").endObject();
StoredScriptSource parsed = StoredScriptSource.parse("lang", builder.bytes(), XContentType.JSON);
StoredScriptSource source = new StoredScriptSource("lang", "code",
Collections.singletonMap(Script.CONTENT_TYPE_OPTION, builder.contentType().mediaType()));
StoredScriptSource source = new StoredScriptSource("lang", "code", Collections.emptyMap());
assertThat(parsed, equalTo(source));
}
@ -214,8 +212,7 @@ public class StoredScriptTests extends AbstractSerializingTestCase<StoredScriptS
}
StoredScriptSource parsed = StoredScriptSource.parse("lang", builder.bytes(), XContentType.JSON);
StoredScriptSource source = new StoredScriptSource("lang", code,
Collections.singletonMap(Script.CONTENT_TYPE_OPTION, builder.contentType().mediaType()));
StoredScriptSource source = new StoredScriptSource("lang", code, Collections.emptyMap());
assertThat(parsed, equalTo(source));
}
@ -230,8 +227,7 @@ public class StoredScriptTests extends AbstractSerializingTestCase<StoredScriptS
}
StoredScriptSource parsed = StoredScriptSource.parse("lang", builder.bytes(), XContentType.JSON);
StoredScriptSource source = new StoredScriptSource("lang", code,
Collections.singletonMap(Script.CONTENT_TYPE_OPTION, builder.contentType().mediaType()));
StoredScriptSource source = new StoredScriptSource("lang", code, Collections.emptyMap());
assertThat(parsed, equalTo(source));
}
@ -246,8 +242,7 @@ public class StoredScriptTests extends AbstractSerializingTestCase<StoredScriptS
}
StoredScriptSource parsed = StoredScriptSource.parse("lang", builder.bytes(), XContentType.JSON);
StoredScriptSource source = new StoredScriptSource("lang", code,
Collections.singletonMap(Script.CONTENT_TYPE_OPTION, builder.contentType().mediaType()));
StoredScriptSource source = new StoredScriptSource("lang", code, Collections.emptyMap());
assertThat(parsed, equalTo(source));
}
@ -328,16 +323,6 @@ public class StoredScriptTests extends AbstractSerializingTestCase<StoredScriptS
StoredScriptSource.parse(null, builder.bytes(), XContentType.JSON));
assertThat(iae.getMessage(), equalTo("illegal compiler options [{option=option}] specified"));
}
// check for illegal use of content type option
try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) {
builder.startObject().field("script").startObject().field("lang", "lang").field("code", "code")
.startObject("options").field("content_type", "option").endObject().endObject().endObject();
ParsingException pe = expectThrows(ParsingException.class, () ->
StoredScriptSource.parse(null, builder.bytes(), XContentType.JSON));
assertThat(pe.getRootCause().getMessage(), equalTo("content_type cannot be user-specified"));
}
}
@Override