Skip to content
Merged
4 changes: 4 additions & 0 deletions docs/reference/mapping/dynamic/templates.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ Dynamic templates are specified as an array of named objects:
<2> The match conditions can include any of : `match_mapping_type`, `match`, `match_pattern`, `unmatch`, `path_match`, `path_unmatch`.
<3> The mapping that the matched field should use.

If an provided mapping contains an invalid mapping snippet then that results in
a validation error when updating a mapping. This is to ensure that if an unmapped
field is mapped via dynamic templates then this will result in a valid mapping update
and not fail the index or update request with a document that contains an invalid field.

Templates are processed in order -- the first matching template wins. When
putting new dynamic templates through the <<indices-put-mapping, put mapping>> API,
Expand Down
7 changes: 7 additions & 0 deletions docs/reference/release-notes/8.0.0-alpha1.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,10 @@ The changes listed below have been released for the first time in {es}

coming[8.0.0]

[[breaking-8.0.0-alpha1]]
[float]
=== Breaking changes

Mapping::
* Dynamic mappings in indices created on 8.0 and later no longer accept invalid mapping snippets
(e.g. incorrect analyzer settings or unknown field types). {pull}51233[#51233]
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,18 @@ private List processList(List list, String name, String dynamicType) {
return processedList;
}

String getName() {
return name;
}

XContentFieldType getXContentFieldType() {
return xcontentFieldType;
}

Map<String, Object> getMapping() {
return mapping;
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -716,4 +716,5 @@ public synchronized List<String> reloadSearchAnalyzers(AnalysisRegistry registry
}
return reloadedAnalyzers;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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[]{
Expand Down Expand Up @@ -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();
}
}
Expand All @@ -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<>();
Expand All @@ -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" }
}
}
]
Expand All @@ -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);
}
}
Expand Down Expand Up @@ -333,4 +342,111 @@ 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}")) {
Copy link
Contributor

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?

Copy link
Member Author

@martijnvg martijnvg Feb 10, 2020

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.

// 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);
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I will try to do this here.

if (fieldTypeConfig.isEmpty()) {
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;
}
}
Loading