Skip to content

Commit

Permalink
Make dynamic template parsing less lenient. #17249
Browse files Browse the repository at this point in the history 
Today unknown parameters are ignored yet carried through serialization.
  • Loading branch information
@jpountz
jpountz committed Mar 22, 2016
1 parent 2064466 commit d514977
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 62 deletions.
111 changes: 54 additions & 57 deletions core/src/main/java/org/elasticsearch/index/mapper/object/DynamicTemplate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,15 @@

package org.elasticsearch.index.mapper.object;

import org.elasticsearch.Version;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.index.mapper.ContentPath;
import org.elasticsearch.index.mapper.MapperParsingException;

import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
Expand All @@ -33,30 +37,41 @@
/**
*
*/
public class DynamicTemplate {
public class DynamicTemplate implements ToXContent {

public static enum MatchType {
SIMPLE,
REGEX;
SIMPLE {
@Override
public String toString() {
return "simple";
}
},
REGEX {
@Override
public String toString() {
return "regex";
}
};

public static MatchType fromString(String value) {
if ("simple".equals(value)) {
return SIMPLE;
} else if ("regex".equals(value)) {
return REGEX;
for (MatchType v : values()) {
if (v.toString().equals(value)) {
return v;
}
}
throw new IllegalArgumentException("No matching pattern matched on [" + value + "]");
}
}

public static DynamicTemplate parse(String name, Map<String, Object> conf) throws MapperParsingException {
public static DynamicTemplate parse(String name, Map<String, Object> conf,
Version indexVersionCreated) throws MapperParsingException {
String match = null;
String pathMatch = null;
String unmatch = null;
String pathUnmatch = null;
Map<String, Object> mapping = null;
String matchMappingType = null;
String matchPattern = "simple";
String matchPattern = MatchType.SIMPLE.toString();

for (Map.Entry<String, Object> entry : conf.entrySet()) {
String propName = Strings.toUnderscoreCase(entry.getKey());
Expand All @@ -74,22 +89,18 @@ public static DynamicTemplate parse(String name, Map<String, Object> conf) throw
matchPattern = entry.getValue().toString();
} else if ("mapping".equals(propName)) {
mapping = (Map<String, Object>) entry.getValue();
} else if (indexVersionCreated.onOrAfter(Version.V_5_0_0)) {
// unknown parameters were ignored before but still carried through serialization
// so we need to ignore them at parsing time for old indices
throw new IllegalArgumentException("Illegal dynamic template parameter: [" + propName + "]");
}
}

if (match == null && pathMatch == null && matchMappingType == null) {
throw new MapperParsingException("template must have match, path_match or match_mapping_type set");
}
if (mapping == null) {
throw new MapperParsingException("template must have mapping set");
}
return new DynamicTemplate(name, conf, pathMatch, pathUnmatch, match, unmatch, matchMappingType, MatchType.fromString(matchPattern), mapping);
return new DynamicTemplate(name, pathMatch, pathUnmatch, match, unmatch, matchMappingType, MatchType.fromString(matchPattern), mapping);
}

private final String name;

private final Map<String, Object> conf;

private final String pathMatch;

private final String pathUnmatch;
Expand All @@ -104,9 +115,14 @@ public static DynamicTemplate parse(String name, Map<String, Object> conf) throw

private final Map<String, Object> mapping;

public DynamicTemplate(String name, Map<String, Object> conf, String pathMatch, String pathUnmatch, String match, String unmatch, String matchMappingType, MatchType matchType, Map<String, Object> mapping) {
public DynamicTemplate(String name, String pathMatch, String pathUnmatch, String match, String unmatch, String matchMappingType, MatchType matchType, Map<String, Object> mapping) {
if (match == null && pathMatch == null && matchMappingType == null) {
throw new MapperParsingException("template must have match, path_match or match_mapping_type set");
}
if (mapping == null) {
throw new MapperParsingException("template must have mapping set");
}
this.name = name;
this.conf = new TreeMap<>(conf);
this.pathMatch = pathMatch;
this.pathUnmatch = pathUnmatch;
this.match = match;
Expand All @@ -120,10 +136,6 @@ public String name() {
return this.name;
}

public Map<String, Object> conf() {
return this.conf;
}

public boolean match(ContentPath path, String name, String dynamicType) {
if (pathMatch != null && !patternMatch(pathMatch, path.pathAsText(name))) {
return false;
Expand All @@ -148,10 +160,6 @@ public boolean match(ContentPath path, String name, String dynamicType) {
return true;
}

public boolean hasType() {
return mapping.containsKey("type");
}

public String mappingType(String dynamicType) {
return mapping.containsKey("type") ? mapping.get("type").toString().replace("{dynamic_type}", dynamicType).replace("{dynamicType}", dynamicType) : dynamicType;
}
Expand Down Expand Up @@ -200,40 +208,29 @@ private List processList(List list, String name, String dynamicType) {
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
if (match != null) {
builder.field("match", match);
}
if (o == null || getClass() != o.getClass()) {
return false;
if (pathMatch != null) {
builder.field("path_match", pathMatch);
}

DynamicTemplate that = (DynamicTemplate) o;

// check if same matching, if so, replace the mapping
if (match != null ? !match.equals(that.match) : that.match != null) {
return false;
if (unmatch != null) {
builder.field("unmatch", unmatch);
}
if (matchMappingType != null ? !matchMappingType.equals(that.matchMappingType) : that.matchMappingType != null) {
return false;
if (pathUnmatch != null) {
builder.field("path_unmatch", pathUnmatch);
}
if (matchType != that.matchType) {
return false;
if (matchMappingType != null) {
builder.field("match_mapping_type", matchMappingType);
}
if (unmatch != null ? !unmatch.equals(that.unmatch) : that.unmatch != null) {
return false;
if (matchType != MatchType.SIMPLE) {
builder.field("match_pattern", matchType);
}

return true;
}

@Override
public int hashCode() {
// check if same matching, if so, replace the mapping
int result = match != null ? match.hashCode() : 0;
result = 31 * result + (unmatch != null ? unmatch.hashCode() : 0);
result = 31 * result + (matchType != null ? matchType.hashCode() : 0);
result = 31 * result + (matchMappingType != null ? matchMappingType.hashCode() : 0);
return result;
// use a sorted map for consistent serialization
builder.field("mapping", new TreeMap<>(mapping));
builder.endObject();
return builder;
}
}
14 changes: 9 additions & 5 deletions core/src/main/java/org/elasticsearch/index/mapper/object/RootObjectMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.index.mapper.object;

import org.elasticsearch.Version;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.joda.FormatDateTimeFormatter;
Expand Down Expand Up @@ -140,14 +141,15 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext
String fieldName = Strings.toUnderscoreCase(entry.getKey());
Object fieldNode = entry.getValue();
if (parseObjectOrDocumentTypeProperties(fieldName, fieldNode, parserContext, builder)
|| processField(builder, fieldName, fieldNode)) {
|| processField(builder, fieldName, fieldNode, parserContext.indexVersionCreated())) {
iterator.remove();
}
}
return builder;
}

protected boolean processField(ObjectMapper.Builder builder, String fieldName, Object fieldNode) {
protected boolean processField(ObjectMapper.Builder builder, String fieldName, Object fieldNode,
Version indexVersionCreated) {
if (fieldName.equals("date_formats") || fieldName.equals("dynamic_date_formats")) {
List<FormatDateTimeFormatter> dateTimeFormatters = new ArrayList<>();
if (fieldNode instanceof List) {
Expand Down Expand Up @@ -185,7 +187,10 @@ protected boolean processField(ObjectMapper.Builder builder, String fieldName, O
throw new MapperParsingException("A dynamic template must be defined with a name");
}
Map.Entry<String, Object> entry = tmpl.entrySet().iterator().next();
((Builder) builder).add(DynamicTemplate.parse(entry.getKey(), (Map<String, Object>) entry.getValue()));
String templateName = entry.getKey();
Map<String, Object> templateParams = (Map<String, Object>) entry.getValue();
DynamicTemplate template = DynamicTemplate.parse(templateName, templateParams, indexVersionCreated);
((Builder) builder).add(template);
}
return true;
} else if (fieldName.equals("date_detection")) {
Expand Down Expand Up @@ -329,8 +334,7 @@ protected void doXContent(XContentBuilder builder, ToXContent.Params params) thr
builder.startArray("dynamic_templates");
for (DynamicTemplate dynamicTemplate : dynamicTemplates) {
builder.startObject();
builder.field(dynamicTemplate.name());
builder.map(dynamicTemplate.conf());
builder.field(dynamicTemplate.name(), dynamicTemplate);
builder.endObject();
}
builder.endArray();
Expand Down
93 changes: 93 additions & 0 deletions core/src/test/java/org/elasticsearch/index/mapper/DynamicTemplateTests.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/*
* 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.index.mapper;

import org.elasticsearch.Version;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.index.mapper.object.DynamicTemplate;
import org.elasticsearch.test.ESTestCase;

import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

public class DynamicTemplateTests extends ESTestCase {

public void testParseUnknownParam() throws Exception {
Map<String, Object> templateDef = new HashMap<>();
templateDef.put("match_mapping_type", "string");
templateDef.put("mapping", Collections.singletonMap("store", true));
templateDef.put("random_param", "random_value");

IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> DynamicTemplate.parse("my_template", templateDef, Version.V_5_0_0));
assertEquals("Illegal dynamic template parameter: [random_param]", e.getMessage());

// but no issues on 2.x for bw compat
DynamicTemplate template = DynamicTemplate.parse("my_template", templateDef, Version.V_2_3_0);
XContentBuilder builder = JsonXContent.contentBuilder();
template.toXContent(builder, ToXContent.EMPTY_PARAMS);
assertEquals("{\"match_mapping_type\":\"string\",\"mapping\":{\"store\":true}}", builder.string());
}

public void testSerialization() throws Exception {
// type-based template
Map<String, Object> templateDef = new HashMap<>();
templateDef.put("match_mapping_type", "string");
templateDef.put("mapping", Collections.singletonMap("store", true));
DynamicTemplate template = DynamicTemplate.parse("my_template", templateDef, Version.V_5_0_0);
XContentBuilder builder = JsonXContent.contentBuilder();
template.toXContent(builder, ToXContent.EMPTY_PARAMS);
assertEquals("{\"match_mapping_type\":\"string\",\"mapping\":{\"store\":true}}", builder.string());

// name-based template
templateDef = new HashMap<>();
templateDef.put("match", "*name");
templateDef.put("unmatch", "first_name");
templateDef.put("mapping", Collections.singletonMap("store", true));
template = DynamicTemplate.parse("my_template", templateDef, Version.V_5_0_0);
builder = JsonXContent.contentBuilder();
template.toXContent(builder, ToXContent.EMPTY_PARAMS);
assertEquals("{\"match\":\"*name\",\"unmatch\":\"first_name\",\"mapping\":{\"store\":true}}", builder.string());

// path-based template
templateDef = new HashMap<>();
templateDef.put("path_match", "*name");
templateDef.put("path_unmatch", "first_name");
templateDef.put("mapping", Collections.singletonMap("store", true));
template = DynamicTemplate.parse("my_template", templateDef, Version.V_5_0_0);
builder = JsonXContent.contentBuilder();
template.toXContent(builder, ToXContent.EMPTY_PARAMS);
assertEquals("{\"path_match\":\"*name\",\"path_unmatch\":\"first_name\",\"mapping\":{\"store\":true}}",
builder.string());

// regex matching
templateDef = new HashMap<>();
templateDef.put("match", "^a$");
templateDef.put("match_pattern", "regex");
templateDef.put("mapping", Collections.singletonMap("store", true));
template = DynamicTemplate.parse("my_template", templateDef, Version.V_5_0_0);
builder = JsonXContent.contentBuilder();
template.toXContent(builder, ToXContent.EMPTY_PARAMS);
assertEquals("{\"match\":\"^a$\",\"match_pattern\":\"regex\",\"mapping\":{\"store\":true}}", builder.string());
}
}

0 comments on commit d514977

Please sign in to comment.