-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Add validation for dynamic templates #51233
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.
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
Changes from all commits
13adc79
ca7ed5a
d047b8f
a8c1079
a63f193
dbce91c
c5df937
28f6d5e
a4ccb67
44fdd1e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,9 +19,13 @@ | |
|
|
||
| package org.elasticsearch.index.mapper; | ||
|
|
||
| import org.apache.logging.log4j.LogManager; | ||
| import org.apache.logging.log4j.Logger; | ||
| import org.elasticsearch.Version; | ||
| import org.elasticsearch.common.Explicit; | ||
| import org.elasticsearch.common.Nullable; | ||
| import org.elasticsearch.common.Strings; | ||
| import org.elasticsearch.common.logging.DeprecationLogger; | ||
| import org.elasticsearch.common.settings.Settings; | ||
| import org.elasticsearch.common.time.DateFormatter; | ||
| import org.elasticsearch.common.xcontent.ToXContent; | ||
|
|
@@ -34,13 +38,17 @@ | |
| import java.util.Collections; | ||
| import java.util.Iterator; | ||
| import java.util.List; | ||
| import java.util.Locale; | ||
| import java.util.Map; | ||
|
|
||
| import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeBooleanValue; | ||
| import static org.elasticsearch.index.mapper.TypeParsers.parseDateTimeFormatter; | ||
|
|
||
| public class RootObjectMapper extends ObjectMapper { | ||
|
|
||
| private static final Logger LOGGER = LogManager.getLogger(RootObjectMapper.class); | ||
| private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(LOGGER); | ||
|
|
||
| public static class Defaults { | ||
| public static final DateFormatter[] DYNAMIC_DATE_TIME_FORMATTERS = | ||
| new DateFormatter[]{ | ||
|
|
@@ -130,7 +138,7 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext | |
| String fieldName = entry.getKey(); | ||
| Object fieldNode = entry.getValue(); | ||
| if (parseObjectOrDocumentTypeProperties(fieldName, fieldNode, parserContext, builder) | ||
| || processField(builder, fieldName, fieldNode, parserContext.indexVersionCreated())) { | ||
| || processField(builder, fieldName, fieldNode, parserContext)) { | ||
| iterator.remove(); | ||
| } | ||
| } | ||
|
|
@@ -139,7 +147,7 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext | |
|
|
||
| @SuppressWarnings("unchecked") | ||
| protected boolean processField(RootObjectMapper.Builder builder, String fieldName, Object fieldNode, | ||
| Version indexVersionCreated) { | ||
| ParserContext parserContext) { | ||
| if (fieldName.equals("date_formats") || fieldName.equals("dynamic_date_formats")) { | ||
| if (fieldNode instanceof List) { | ||
| List<DateFormatter> formatters = new ArrayList<>(); | ||
|
|
@@ -163,7 +171,7 @@ protected boolean processField(RootObjectMapper.Builder builder, String fieldNam | |
| "template_1" : { | ||
| "match" : "*_test", | ||
| "match_mapping_type" : "string", | ||
| "mapping" : { "type" : "string", "store" : "yes" } | ||
| "mapping" : { "type" : "keyword", "store" : "yes" } | ||
| } | ||
| } | ||
| ] | ||
|
|
@@ -183,6 +191,7 @@ protected boolean processField(RootObjectMapper.Builder builder, String fieldNam | |
| Map<String, Object> templateParams = (Map<String, Object>) entry.getValue(); | ||
| DynamicTemplate template = DynamicTemplate.parse(templateName, templateParams); | ||
| if (template != null) { | ||
| validateDynamicTemplate(parserContext, template); | ||
| templates.add(template); | ||
| } | ||
| } | ||
|
|
@@ -333,4 +342,114 @@ protected void doXContent(XContentBuilder builder, ToXContent.Params params) thr | |
| builder.field("numeric_detection", numericDetection.value()); | ||
| } | ||
| } | ||
|
|
||
| private static void validateDynamicTemplate(Mapper.TypeParser.ParserContext parserContext, | ||
| DynamicTemplate dynamicTemplate) { | ||
|
|
||
| if (containsSnippet(dynamicTemplate.getMapping(), "{name}")) { | ||
| // Can't validate template, because field names can't be guessed up front. | ||
| return; | ||
| } | ||
|
|
||
| final XContentFieldType[] types; | ||
| if (dynamicTemplate.getXContentFieldType() != null) { | ||
| types = new XContentFieldType[]{dynamicTemplate.getXContentFieldType()}; | ||
| } else { | ||
| types = XContentFieldType.values(); | ||
| } | ||
|
|
||
| Exception lastError = null; | ||
| boolean dynamicTemplateInvalid = true; | ||
|
|
||
| for (XContentFieldType contentFieldType : types) { | ||
| String defaultDynamicType = contentFieldType.defaultMappingType(); | ||
| String mappingType = dynamicTemplate.mappingType(defaultDynamicType); | ||
| Mapper.TypeParser typeParser = parserContext.typeParser(mappingType); | ||
| if (typeParser == null) { | ||
| lastError = new IllegalArgumentException("No mapper found for type [" + mappingType + "]"); | ||
| continue; | ||
| } | ||
|
|
||
| Map<String, Object> fieldTypeConfig = dynamicTemplate.mappingForName("__dummy__", defaultDynamicType); | ||
| fieldTypeConfig.remove("type"); | ||
| try { | ||
| Mapper.Builder<?, ?> dummyBuilder = typeParser.parse("__dummy__", fieldTypeConfig, parserContext); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think some fields might do extra validation when calling builder.build(), should we try to build the mapper too?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. I will try to do this here. |
||
| if (fieldTypeConfig.isEmpty()) { | ||
| Settings indexSettings = parserContext.mapperService().getIndexSettings().getSettings(); | ||
| BuilderContext builderContext = new BuilderContext(indexSettings, new ContentPath(1)); | ||
| dummyBuilder.build(builderContext); | ||
| dynamicTemplateInvalid = false; | ||
| break; | ||
| } else { | ||
| lastError = new IllegalArgumentException("Unused mapping attributes [" + fieldTypeConfig + "]"); | ||
| } | ||
| } catch (Exception e) { | ||
| lastError = e; | ||
| } | ||
| } | ||
|
|
||
| final boolean failInvalidDynamicTemplates = parserContext.indexVersionCreated().onOrAfter(Version.V_8_0_0); | ||
| if (dynamicTemplateInvalid) { | ||
| String message = String.format(Locale.ROOT, "dynamic template [%s] has invalid content [%s]", | ||
| dynamicTemplate.getName(), Strings.toString(dynamicTemplate)); | ||
| if (failInvalidDynamicTemplates) { | ||
| throw new IllegalArgumentException(message, lastError); | ||
| } else { | ||
| final String deprecationMessage; | ||
| if (lastError != null) { | ||
| deprecationMessage = String.format(Locale.ROOT, "%s, caused by [%s]", message, lastError.getMessage()); | ||
| } else { | ||
| deprecationMessage = message; | ||
| } | ||
| DEPRECATION_LOGGER.deprecatedAndMaybeLog("invalid_dynamic_template", deprecationMessage); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private static boolean containsSnippet(Map<?, ?> map, String snippet) { | ||
| for (Map.Entry<?, ?> entry : map.entrySet()) { | ||
| String key = entry.getKey().toString(); | ||
| if (key.contains(snippet)) { | ||
| return true; | ||
| } | ||
|
|
||
| Object value = entry.getValue(); | ||
| if (value instanceof Map) { | ||
| if (containsSnippet((Map<?, ?>) value, snippet)) { | ||
| return true; | ||
| } | ||
| } else if (value instanceof List) { | ||
| if (containsSnippet((List<?>) value, snippet)) { | ||
| return true; | ||
| } | ||
| } else if (value instanceof String) { | ||
| String valueString = (String) value; | ||
| if (valueString.contains(snippet)) { | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| private static boolean containsSnippet(List<?> list, String snippet) { | ||
| for (Object value : list) { | ||
| if (value instanceof Map) { | ||
| if (containsSnippet((Map<?, ?>) value, snippet)) { | ||
| return true; | ||
| } | ||
| } else if (value instanceof List) { | ||
| if (containsSnippet((List<?>) value, snippet)) { | ||
| return true; | ||
| } | ||
| } else if (value instanceof String) { | ||
| String valueString = (String) value; | ||
| if (valueString.contains(snippet)) { | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to check
{dynamic_type}too?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only
{name}, because if the type hasn't specifically configured then line 360 ensures that all possible dynamic types are checked.