From c31a5b1138588846091252574b8a89c6570f8a6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Wed, 17 Apr 2019 18:44:07 +0200 Subject: [PATCH] Fix error applying `ignore_malformed` to boolean values (#41261) 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 #11498 --- .../index/mapper/NumberFieldMapper.java | 6 +- .../index/mapper/NumberFieldMapperTests.java | 86 ++++++++++++------- 2 files changed, 59 insertions(+), 33 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index 06e12ca8b5e4c..927bce5d9d6dd 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -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; @@ -1042,8 +1044,8 @@ protected void parseCreateField(ParseContext context, List 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 { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java index b4b9242daa456..77953c0903fd2 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -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; @@ -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 { @@ -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 {