Fix nested _source retrieval with includes/excludes

If an exclude or an include clause removes an entry to a nested field in the original source at query time, the creation of nested hits fails with an NPE. This change fixes this exception and replaces the nested document source with an empty map. Closes elastic#33163 Closes elastic#33170
jimczi · Aug 27, 2018 · 0db3524 · 0db3524
1 parent 309fb22
commit 0db3524
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 2 deletions.
diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchSourceSubPhase.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchSourceSubPhase.java
@@ -29,6 +29,7 @@
 import org.elasticsearch.search.lookup.SourceLookup;
 
 import java.io.IOException;
+import java.util.HashMap;
 import java.util.Map;
 
 public final class FetchSourceSubPhase implements FetchSubPhase {
@@ -57,6 +58,7 @@ public void hitExecute(SearchContext context, HitContext hitContext) {
         if (nestedHit) {
             value = getNestedSource((Map<String, Object>) value, hitContext);
         }
+
         try {
             final int initialCapacity = nestedHit ? 1024 : Math.min(1024, source.internalSourceRef().length());
             BytesStreamOutput streamOutput = new BytesStreamOutput(initialCapacity);
@@ -81,6 +83,9 @@ public void hitExecute(SearchContext context, HitContext hitContext) {
     private Map<String, Object> getNestedSource(Map<String, Object> sourceAsMap, HitContext hitContext) {
         for (SearchHit.NestedIdentity o = hitContext.hit().getNestedIdentity(); o != null; o = o.getChild()) {
             sourceAsMap = (Map<String, Object>) sourceAsMap.get(o.getField().string());
+            if (sourceAsMap == null) {
+                return null;
+            }
         }
         return sourceAsMap;
     }

diff --git a/server/src/test/java/org/elasticsearch/search/fetch/subphase/FetchSourceSubPhaseTests.java b/server/src/test/java/org/elasticsearch/search/fetch/subphase/FetchSourceSubPhaseTests.java
@@ -34,6 +34,7 @@
 
 import java.io.IOException;
 import java.util.Collections;
+import java.util.Map;
 
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -78,6 +79,29 @@ public void testMultipleFiltering() throws IOException {
         assertEquals(Collections.singletonMap("field","value"), hitContext.hit().getSourceAsMap());
     }
 
+    public void testNestedSource() throws IOException {
+        Map<String, Object> expectedNested = Collections.singletonMap("nested2", Collections.singletonMap("field", "value0"));
+        XContentBuilder source = XContentFactory.jsonBuilder().startObject()
+            .field("field", "value")
+            .field("field2", "value2")
+            .field("nested1", expectedNested)
+            .endObject();
+        FetchSubPhase.HitContext hitContext = hitExecuteMultiple(source, true, null, null,
+            new SearchHit.NestedIdentity("nested1", 0,null));
+        assertEquals(expectedNested, hitContext.hit().getSourceAsMap());
+        hitContext = hitExecuteMultiple(source, true, new String[]{"invalid"}, null,
+            new SearchHit.NestedIdentity("nested1", 0,null));
+        assertEquals(Collections.emptyMap(), hitContext.hit().getSourceAsMap());
+
+        hitContext = hitExecuteMultiple(source, true, null, null,
+            new SearchHit.NestedIdentity("nested1", 0, new SearchHit.NestedIdentity("nested2", 0, null)));
+        assertEquals(Collections.singletonMap("field", "value0"), hitContext.hit().getSourceAsMap());
+
+        hitContext = hitExecuteMultiple(source, true, new String[]{"invalid"}, null,
+            new SearchHit.NestedIdentity("nested1", 0, new SearchHit.NestedIdentity("nested2", 0, null)));
+        assertEquals(Collections.emptyMap(), hitContext.hit().getSourceAsMap());
+    }
+
     public void testSourceDisabled() throws IOException {
         FetchSubPhase.HitContext hitContext = hitExecute(null, true, null, null);
         assertNull(hitContext.hit().getSourceAsMap());
@@ -96,17 +120,29 @@ public void testSourceDisabled() throws IOException {
     }
 
     private FetchSubPhase.HitContext hitExecute(XContentBuilder source, boolean fetchSource, String include, String exclude) {
+        return hitExecute(source, fetchSource, include, exclude, null);
+    }
+
+
+    private FetchSubPhase.HitContext hitExecute(XContentBuilder source, boolean fetchSource, String include, String exclude,
+                                                    SearchHit.NestedIdentity nestedIdentity) {
         return hitExecuteMultiple(source, fetchSource,
             include == null ? Strings.EMPTY_ARRAY : new String[]{include},
-            exclude == null ? Strings.EMPTY_ARRAY : new String[]{exclude});
+            exclude == null ? Strings.EMPTY_ARRAY : new String[]{exclude}, nestedIdentity);
     }
 
     private FetchSubPhase.HitContext hitExecuteMultiple(XContentBuilder source, boolean fetchSource, String[] includes, String[] excludes) {
+        return hitExecuteMultiple(source, fetchSource, includes, excludes, null);
+    }
+
+    private FetchSubPhase.HitContext hitExecuteMultiple(XContentBuilder source, boolean fetchSource, String[] includes, String[] excludes,
+                                                            SearchHit.NestedIdentity nestedIdentity) {
         FetchSourceContext fetchSourceContext = new FetchSourceContext(fetchSource, includes, excludes);
         SearchContext searchContext = new FetchSourceSubPhaseTestSearchContext(fetchSourceContext,
                 source == null ? null : BytesReference.bytes(source));
         FetchSubPhase.HitContext hitContext = new FetchSubPhase.HitContext();
-        hitContext.reset(new SearchHit(1, null, null, null), null, 1, null);
+        final SearchHit searchHit = new SearchHit(1, null, null, nestedIdentity, null);
+        hitContext.reset(searchHit, null, 1, null);
         FetchSourceSubPhase phase = new FetchSourceSubPhase();
         phase.hitExecute(searchContext, hitContext);
         return hitContext;