From 9172653211fb35b1b7dee491650b913d5c2282a5 Mon Sep 17 00:00:00 2001 From: qwerty4030 Date: Sat, 27 Aug 2016 04:24:45 -0700 Subject: [PATCH] Fix NPE during search with source filtering if the source is disabled. (#20093) * Fix NPE during search with source filtering if the source is disabled. Instead of throwing an NPE, a search response with source filtering will not contain the source if it is disabled in the mapping. Closes #7758 * Created unit tests for FetchSourceSubPhase. Tests similar to SourceFetchingIT. Removed SourceFetchingIT#testSourceDisabled (now covered via unit test FetchSourceSubPhaseTests#testSourceDisabled). * Updated FetchSouceSubPhase unit tests per comments. Renamed main unit test method. Use assertEquals and assertNull instead of assertThat (less code). --- .../fetch/subphase/FetchSourceSubPhase.java | 9 +- .../subphase/FetchSourceSubPhaseTests.java | 134 ++++++++++++++++++ 2 files changed, 140 insertions(+), 3 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/search/fetch/subphase/FetchSourceSubPhaseTests.java diff --git a/core/src/main/java/org/elasticsearch/search/fetch/subphase/FetchSourceSubPhase.java b/core/src/main/java/org/elasticsearch/search/fetch/subphase/FetchSourceSubPhase.java index 7ba24442a7f..c67b96c7af5 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/subphase/FetchSourceSubPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/subphase/FetchSourceSubPhase.java @@ -35,19 +35,22 @@ public final class FetchSourceSubPhase implements FetchSubPhase { if (context.sourceRequested() == false) { return; } + SourceLookup source = context.lookup().source(); + if (source.internalSourceRef() == null) { + return; // source disabled in the mapping + } FetchSourceContext fetchSourceContext = context.fetchSourceContext(); assert fetchSourceContext.fetchSource(); if (fetchSourceContext.includes().length == 0 && fetchSourceContext.excludes().length == 0) { - hitContext.hit().sourceRef(context.lookup().source().internalSourceRef()); + hitContext.hit().sourceRef(source.internalSourceRef()); return; } - SourceLookup source = context.lookup().source(); Object value = source.filter(fetchSourceContext.includes(), fetchSourceContext.excludes()); try { final int initialCapacity = Math.min(1024, source.internalSourceRef().length()); BytesStreamOutput streamOutput = new BytesStreamOutput(initialCapacity); - XContentBuilder builder = new XContentBuilder(context.lookup().source().sourceContentType().xContent(), streamOutput); + XContentBuilder builder = new XContentBuilder(source.sourceContentType().xContent(), streamOutput); builder.value(value); hitContext.hit().sourceRef(builder.bytes()); } catch (IOException e) { diff --git a/core/src/test/java/org/elasticsearch/search/fetch/subphase/FetchSourceSubPhaseTests.java b/core/src/test/java/org/elasticsearch/search/fetch/subphase/FetchSourceSubPhaseTests.java new file mode 100644 index 00000000000..43461929bd9 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/search/fetch/subphase/FetchSourceSubPhaseTests.java @@ -0,0 +1,134 @@ +/* + * 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.search.fetch.subphase; + +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.search.fetch.FetchSubPhase; +import org.elasticsearch.search.internal.InternalSearchHit; +import org.elasticsearch.search.internal.SearchContext; +import org.elasticsearch.search.lookup.SearchLookup; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.TestSearchContext; + +import java.io.IOException; +import java.util.Collections; + +public class FetchSourceSubPhaseTests extends ESTestCase { + + static class FetchSourceSubPhaseTestSearchContext extends TestSearchContext { + + FetchSourceContext context; + BytesReference source; + + FetchSourceSubPhaseTestSearchContext(FetchSourceContext context, BytesReference source) { + super(null); + this.context = context; + this.source = source; + } + + @Override + public boolean sourceRequested() { + return context != null && context.fetchSource(); + } + + @Override + public FetchSourceContext fetchSourceContext() { + return context; + } + + @Override + public SearchLookup lookup() { + SearchLookup lookup = super.lookup(); + lookup.source().setSource(source); + return lookup; + } + } + + public void testFetchSource() throws IOException { + XContentBuilder source = XContentFactory.jsonBuilder().startObject() + .field("field", "value") + .endObject(); + FetchSubPhase.HitContext hitContext = hitExecute(source, true, null, null); + assertEquals(Collections.singletonMap("field","value"), hitContext.hit().sourceAsMap()); + } + + public void testBasicFiltering() throws IOException { + XContentBuilder source = XContentFactory.jsonBuilder().startObject() + .field("field1", "value") + .field("field2", "value2") + .endObject(); + FetchSubPhase.HitContext hitContext = hitExecute(source, false, null, null); + assertNull(hitContext.hit().sourceAsMap()); + + hitContext = hitExecute(source, true, "field1", null); + assertEquals(Collections.singletonMap("field1","value"), hitContext.hit().sourceAsMap()); + + hitContext = hitExecute(source, true, "hello", null); + assertEquals(Collections.emptyMap(), hitContext.hit().sourceAsMap()); + + hitContext = hitExecute(source, true, "*", "field2"); + assertEquals(Collections.singletonMap("field1","value"), hitContext.hit().sourceAsMap()); + } + + public void testMultipleFiltering() throws IOException { + XContentBuilder source = XContentFactory.jsonBuilder().startObject() + .field("field", "value") + .field("field2", "value2") + .endObject(); + FetchSubPhase.HitContext hitContext = hitExecuteMultiple(source, true, new String[]{"*.notexisting", "field"}, null); + assertEquals(Collections.singletonMap("field","value"), hitContext.hit().sourceAsMap()); + + hitContext = hitExecuteMultiple(source, true, new String[]{"field.notexisting.*", "field"}, null); + assertEquals(Collections.singletonMap("field","value"), hitContext.hit().sourceAsMap()); + } + + public void testSourceDisabled() throws IOException { + FetchSubPhase.HitContext hitContext = hitExecute(null, true, null, null); + assertNull(hitContext.hit().sourceAsMap()); + + hitContext = hitExecute(null, false, null, null); + assertNull(hitContext.hit().sourceAsMap()); + + hitContext = hitExecute(null, true, "field1", null); + assertNull(hitContext.hit().sourceAsMap()); + + hitContext = hitExecuteMultiple(null, true, new String[]{"*"}, new String[]{"field2"}); + assertNull(hitContext.hit().sourceAsMap()); + } + + private FetchSubPhase.HitContext hitExecute(XContentBuilder source, boolean fetchSource, String include, String exclude) { + return hitExecuteMultiple(source, fetchSource, + include == null ? Strings.EMPTY_ARRAY : new String[]{include}, + exclude == null ? Strings.EMPTY_ARRAY : new String[]{exclude}); + } + + private FetchSubPhase.HitContext hitExecuteMultiple(XContentBuilder source, boolean fetchSource, String[] includes, String[] excludes) { + FetchSourceContext fetchSourceContext = new FetchSourceContext(fetchSource, includes, excludes); + SearchContext searchContext = new FetchSourceSubPhaseTestSearchContext(fetchSourceContext, source == null ? null : source.bytes()); + FetchSubPhase.HitContext hitContext = new FetchSubPhase.HitContext(); + hitContext.reset(new InternalSearchHit(1, null, null, null), null, 1, null); + FetchSourceSubPhase phase = new FetchSourceSubPhase(); + phase.hitExecute(searchContext, hitContext); + return hitContext; + } +}