Skip to content

Commit

Permalink
Fix error applying ignore_malformed to boolean values (elastic#41261)
Browse files Browse the repository at this point in the history 
The `ignore_malformed` option currently works on numeric fields only when the
bad value isn't a string value but not if it is a boolean. In this case we get a
parsing error from the xContent parser which we need to catch in addition to the
field mapper.

Closes elastic#11498
  • Loading branch information
Christoph Büscher authored and Gurkan Kaymak committed May 27, 2019
1 parent 27ed862 commit f88c5d4
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 33 deletions.
6 changes: 4 additions & 2 deletions server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

package org.elasticsearch.index.mapper;

import com.fasterxml.jackson.core.JsonParseException;

import org.apache.lucene.document.DoublePoint;
import org.apache.lucene.document.Field;
import org.apache.lucene.document.FloatPoint;
Expand Down Expand Up @@ -1042,8 +1044,8 @@ protected void parseCreateField(ParseContext context, List<IndexableField> field
} else {
try {
numericValue = fieldType().type.parse(parser, coerce.value());
} catch (IllegalArgumentException e) {
if (ignoreMalformed.value()) {
} catch (IllegalArgumentException | JsonParseException e) {
if (ignoreMalformed.value() && parser.currentToken().isValue()) {
context.addIgnoredField(fieldType.name());
return;
} else {
Expand Down
86 changes: 55 additions & 31 deletions server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,14 @@
package org.elasticsearch.index.mapper;

import com.carrotsearch.randomizedtesting.annotations.Timeout;

import org.apache.lucene.index.DocValuesType;
import org.apache.lucene.index.IndexableField;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.mapper.NumberFieldMapper.NumberType;
Expand All @@ -37,6 +40,7 @@
import java.util.HashSet;
import java.util.List;

import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.hamcrest.Matchers.containsString;

public class NumberFieldMapperTests extends AbstractNumericFieldMapperTestCase {
Expand Down Expand Up @@ -218,45 +222,65 @@ protected void doTestDecimalCoerce(String type) throws IOException {

public void testIgnoreMalformed() throws Exception {
for (String type : TYPES) {
doTestIgnoreMalformed(type);
}
}
for (Object malformedValue : new Object[] { "a", Boolean.FALSE }) {
String mapping = Strings.toString(jsonBuilder().startObject().startObject("type").startObject("properties")
.startObject("field").field("type", type).endObject().endObject().endObject().endObject());

private void doTestIgnoreMalformed(String type) throws IOException {
String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("properties").startObject("field").field("type", type).endObject().endObject()
.endObject().endObject());
DocumentMapper mapper = parser.parse("type", new CompressedXContent(mapping));

DocumentMapper mapper = parser.parse("type", new CompressedXContent(mapping));
assertEquals(mapping, mapper.mappingSource().toString());

assertEquals(mapping, mapper.mappingSource().toString());
ThrowingRunnable runnable = () -> mapper.parse(new SourceToParse("test", "type", "1",
BytesReference.bytes(jsonBuilder().startObject().field("field", malformedValue).endObject()), XContentType.JSON));
MapperParsingException e = expectThrows(MapperParsingException.class, runnable);
if (malformedValue instanceof String) {
assertThat(e.getCause().getMessage(), containsString("For input string: \"a\""));
} else {
assertThat(e.getCause().getMessage(), containsString("Current token"));
assertThat(e.getCause().getMessage(), containsString("not numeric, can not use numeric value accessors"));
}

ThrowingRunnable runnable = () -> mapper.parse(new SourceToParse("test", "type", "1", BytesReference
.bytes(XContentFactory.jsonBuilder()
.startObject()
.field("field", "a")
.endObject()),
XContentType.JSON));
MapperParsingException e = expectThrows(MapperParsingException.class, runnable);

assertThat(e.getCause().getMessage(), containsString("For input string: \"a\""));
mapping = Strings.toString(jsonBuilder().startObject().startObject("type").startObject("properties").startObject("field")
.field("type", type).field("ignore_malformed", true).endObject().endObject().endObject().endObject());

mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("properties").startObject("field").field("type", type).field("ignore_malformed", true).endObject().endObject()
.endObject().endObject());
DocumentMapper mapper2 = parser.parse("type", new CompressedXContent(mapping));

DocumentMapper mapper2 = parser.parse("type", new CompressedXContent(mapping));
ParsedDocument doc = mapper2.parse(new SourceToParse("test", "type", "1",
BytesReference.bytes(jsonBuilder().startObject().field("field", malformedValue).endObject()), XContentType.JSON));

ParsedDocument doc = mapper2.parse(new SourceToParse("test", "type", "1", BytesReference
.bytes(XContentFactory.jsonBuilder()
.startObject()
.field("field", "a")
.endObject()),
XContentType.JSON));
IndexableField[] fields = doc.rootDoc().getFields("field");
assertEquals(0, fields.length);
assertArrayEquals(new String[] { "field" }, doc.rootDoc().getValues("_ignored"));
}
}
}

IndexableField[] fields = doc.rootDoc().getFields("field");
assertEquals(0, fields.length);
assertArrayEquals(new String[] { "field" }, doc.rootDoc().getValues("_ignored"));
/**
* Test that in case the malformed value is an xContent object we throw error regardless of `ignore_malformed`
*/
public void testIgnoreMalformedWithObject() throws Exception {
for (String type : TYPES) {
Object malformedValue = new ToXContentObject() {
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
return builder.startObject().field("foo", "bar").endObject();
}
};
for (Boolean ignoreMalformed : new Boolean[] { true, false }) {
String mapping = Strings.toString(
jsonBuilder().startObject().startObject("type").startObject("properties").startObject("field").field("type", type)
.field("ignore_malformed", ignoreMalformed).endObject().endObject().endObject().endObject());
DocumentMapper mapper = parser.parse("type", new CompressedXContent(mapping));
assertEquals(mapping, mapper.mappingSource().toString());

MapperParsingException e = expectThrows(MapperParsingException.class,
() -> mapper.parse(new SourceToParse("test", "type", "1",
BytesReference.bytes(jsonBuilder().startObject().field("field", malformedValue).endObject()),
XContentType.JSON)));
assertThat(e.getCause().getMessage(), containsString("Current token"));
assertThat(e.getCause().getMessage(), containsString("not numeric, can not use numeric value accessors"));
}
}
}

public void testRejectNorms() throws IOException {
Expand Down

0 comments on commit f88c5d4

Please sign in to comment.