Skip to content

Null completion field should not throw IAE #33268

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.

Sign up for GitHub

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

Merged
merged 12 commits into from
Sep 3, 2018
Merged

Null completion field should not throw IAE #33268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1c5da09
[Issue #33200] null completion values should not throw IAE and be add…
tony-dillon Aug 28, 2018
9d96f19
[Issue #33200] fixed test
tony-dillon Aug 30, 2018
681b868
[Issue #33200] updated test to use Token.NULL_VALUE
tony-dillon Aug 30, 2018
47513b9
[Issue #33200] stopped adding null fields to ignored field list. rest…
tony-dillon Aug 30, 2018
886e18f
Merge branch 'master' into null_completion
tony-dillon Aug 30, 2018
113245b
[Issue #33200] fixed error where wrong document was being asserted on
tony-dillon Aug 30, 2018
94790b1
[Issue #33200] set correct null field value on test document
tony-dillon Aug 30, 2018
576dc7b
[Issue #33200] set nullField on correct document
tony-dillon Aug 30, 2018
563de3f
[Issue #33200] remove unused use statement
tony-dillon Aug 30, 2018
27adc3f
remote IT for null completion field value
jimczi Sep 3, 2018
7e0ad6e
Merge branch 'master' into pr/33268
jimczi Sep 3, 2018
75db758
add local change
jimczi Sep 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,8 +435,10 @@ public Mapper parse(ParseContext context) throws IOException {
XContentParser parser = context.parser();
Token token = parser.currentToken();
Map<String, CompletionInputMetaData> inputMap = new HashMap<>(1);

// ignore null values
if (token == Token.VALUE_NULL) {
throw new MapperParsingException("completion field [" + fieldType().name() + "] does not support null values");
return null;
} else if (token == Token.START_ARRAY) {
while ((token = parser.nextToken()) != Token.END_ARRAY) {
parse(context, token, parser, inputMap);
Expand Down
11 changes: 11 additions & 0 deletions server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,17 @@ public void testFieldValueValidation() throws Exception {
assertNotNull(doc.docs().get(0).getField("_ignored"));
IndexableField ignoredFields = doc.docs().get(0).getField("_ignored");
assertThat(ignoredFields.stringValue(), equalTo("completion"));

// null inputs are ignored
ParsedDocument nullDoc = defaultMapper.parse(SourceToParse.source("test", "type1", "1", BytesReference
.bytes(XContentFactory.jsonBuilder()
.startObject()
.nullField("completion")
.endObject()),
XContentType.JSON));
assertThat(nullDoc.docs().size(), equalTo(1));
assertNull(nullDoc.docs().get(0).get("completion"));
assertNull(nullDoc.docs().get(0).getField("_ignored"));
}

public void testPrefixQueryType() throws Exception {
Expand Down
31 changes: 0 additions & 31 deletions server/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,10 @@
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.support.master.AcknowledgedResponse;
import org.elasticsearch.common.FieldMemoryStats;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.Fuzziness;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.mapper.MapperParsingException;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.search.aggregations.AggregationBuilders;
Expand Down Expand Up @@ -1111,35 +1109,6 @@ public void testIssue5930() throws IOException {
}
}

// see issue #6399
public void testIndexingUnrelatedNullValue() throws Exception {
String mapping = Strings
.toString(jsonBuilder()
.startObject()
.startObject(TYPE)
.startObject("properties")
.startObject(FIELD)
.field("type", "completion")
.endObject()
.endObject()
.endObject()
.endObject());

assertAcked(client().admin().indices().prepareCreate(INDEX).addMapping(TYPE, mapping, XContentType.JSON).get());
ensureGreen();

client().prepareIndex(INDEX, TYPE, "1").setSource(FIELD, "strings make me happy", FIELD + "_1", "nulls make me sad")
.setRefreshPolicy(IMMEDIATE).get();

try {
client().prepareIndex(INDEX, TYPE, "2").setSource(FIELD, null, FIELD + "_1", "nulls make me sad").get();
fail("Expected MapperParsingException for null value");
} catch (MapperParsingException e) {
// make sure that the exception has the name of the field causing the error
assertTrue(e.getDetailedMessage().contains(FIELD));
}
}

public void testMultiDocSuggestions() throws Exception {
final CompletionMappingBuilder mapping = new CompletionMappingBuilder();
createIndexAndMapping(mapping);
Expand Down