Merges PR #18957

This commit fixes several NPEs caused by implicitly performing a get request for a document that exists with its _source disabled and then trying to access the source. Instead of causing an NPE the following queries will throw an exception with a "source disabled" message (similar behavior as if the document does not exist).:
- GeoShape query for pre-indexed shape (throws IllegalArgumentException)
- Percolate query for an existing document (throws IllegalArgumentException)

A Terms query with a lookup will ignore the document if the source does not exist (same as if the document does not exist).

GET and HEAD requests for the document _source will return a 404 if the source is disabled (even if the document exists).

core/src/main/java/org/elasticsearch/common/network/NetworkModule.java

@@ -238,7 +238,8 @@ public class NetworkModule extends AbstractModule {
         RestIndexAction.class,
         RestGetAction.class,
         RestGetSourceAction.class,
-        RestHeadAction.class,
+        RestHeadAction.Document.class,
+        RestHeadAction.Source.class,
         RestMultiGetAction.class,
         RestDeleteAction.class,
         org.elasticsearch.rest.action.count.RestCountAction.class,

core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java

@@ -378,6 +378,10 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuil
         if (!response.isExists()) {
             throw new IllegalArgumentException("Shape with ID [" + getRequest.id() + "] in type [" + getRequest.type() + "] not found");
         }
+        if (response.isSourceEmpty()) {
+            throw new IllegalArgumentException("Shape with ID [" + getRequest.id() + "] in type [" + getRequest.type() +
+                    "] source disabled");
+        }
 
         String[] pathElements = path.split("\\.");
         int currentPathSlot = 0;

core/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java

@@ -328,7 +328,7 @@ public class TermsQueryBuilder extends AbstractQueryBuilder<TermsQueryBuilder> {
         GetRequest getRequest = new GetRequest(termsLookup.index(), termsLookup.type(), termsLookup.id())
                 .preference("_local").routing(termsLookup.routing());
         final GetResponse getResponse = client.get(getRequest).actionGet();
-        if (getResponse.isExists()) {
+        if (getResponse.isSourceEmpty() == false) { // extract terms only if the doc source exists
             List<Object> extractedValues = XContentMapValues.extractRawValues(termsLookup.path(), getResponse.getSourceAsMap());
             terms.addAll(extractedValues);
         }

core/src/main/java/org/elasticsearch/rest/action/get/RestGetSourceAction.java

@@ -78,7 +78,7 @@ public class RestGetSourceAction extends BaseRestHandler {
             @Override
             public RestResponse buildResponse(GetResponse response) throws Exception {
                 XContentBuilder builder = channel.newBuilder(response.getSourceInternal(), false);
-                if (!response.isExists()) {
+                if (response.isSourceEmpty()) { // check if doc source (or doc itself) is missing
                     return new BytesRestResponse(NOT_FOUND, builder);
                 } else {
                     builder.rawValue(response.getSourceInternal());

core/src/main/java/org/elasticsearch/rest/action/get/RestHeadAction.java

@@ -39,15 +39,47 @@ import static org.elasticsearch.rest.RestStatus.NOT_FOUND;
 import static org.elasticsearch.rest.RestStatus.OK;
 
 /**
- *
+ * Base class for {@code HEAD} request handlers for a single document.
  */
-public class RestHeadAction extends BaseRestHandler {
+public abstract class RestHeadAction extends BaseRestHandler {
+
+    /**
+     * Handler to check for document existence.
+     */
+    public static class Document extends RestHeadAction {
+
+        @Inject
+        public Document(Settings settings, RestController controller, Client client) {
+            super(settings, client, false);
+            controller.registerHandler(HEAD, "/{index}/{type}/{id}", this);
+        }
+    }
+
+    /**
+     * Handler to check for document source existence (may be disabled in the mapping).
+     */
+    public static class Source extends RestHeadAction {
 
-    @Inject
-    public RestHeadAction(Settings settings, RestController controller, Client client) {
+        @Inject
+        public Source(Settings settings, RestController controller, Client client) {
+            super(settings, client, true);
+            controller.registerHandler(HEAD, "/{index}/{type}/{id}/_source", this);
+        }
+    }
+
+    private final boolean source;
+
+    /**
+     * All subclasses must be registered in {@link org.elasticsearch.common.network.NetworkModule}.
+     *
+     * @param settings injected settings
+     * @param client   injected client
+     * @param source   {@code false} to check for {@link GetResponse#isExists()}.
+     *                 {@code true} to also check for {@link GetResponse#isSourceEmpty()}.
+     */
+    public RestHeadAction(Settings settings, Client client, boolean source) {
         super(settings, client);
-        controller.registerHandler(HEAD, "/{index}/{type}/{id}", this);
-        controller.registerHandler(HEAD, "/{index}/{type}/{id}/_source", this);
+        this.source = source;
     }
 
     @Override
@@ -68,6 +100,8 @@ public class RestHeadAction extends BaseRestHandler {
             public RestResponse buildResponse(GetResponse response) {
                 if (!response.isExists()) {
                     return new BytesRestResponse(NOT_FOUND, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY);
+                } else if (source && response.isSourceEmpty()) { // doc exists, but source might not (disabled in the mapping)
+                    return new BytesRestResponse(NOT_FOUND, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY);
                 } else {
                     return new BytesRestResponse(OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY);
                 }

core/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java

@@ -19,6 +19,8 @@
 
 package org.elasticsearch.search.geo;
 
+import org.elasticsearch.ElasticsearchException;
+import org.elasticsearch.common.settings.Settings;
 import org.locationtech.spatial4j.shape.Rectangle;
 import com.vividsolutions.jts.geom.Coordinate;
 
@@ -54,6 +56,7 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSear
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
+import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.nullValue;
 
 public class GeoShapeQueryTests extends ESSingleNodeTestCase {
@@ -197,6 +200,30 @@ public class GeoShapeQueryTests extends ESSingleNodeTestCase {
         assertThat(searchResponse.getHits().getAt(0).id(), equalTo("1"));
     }
 
+    public void testIndexedShapeReferenceSourceDisabled() throws Exception {
+        XContentBuilder mapping = XContentFactory.jsonBuilder().startObject()
+                .startObject("properties")
+                    .startObject("location")
+                        .field("type", "geo_shape")
+                        .field("tree", "quadtree")
+                    .endObject()
+                .endObject()
+            .endObject();
+        client().admin().indices().prepareCreate("test").addMapping("type1", mapping).get();
+        createIndex("shapes", Settings.EMPTY, "shape_type", "_source", "enabled=false");
+        ensureGreen();
+
+        ShapeBuilder shape = ShapeBuilders.newEnvelope(new Coordinate(-45, 45), new Coordinate(45, -45));
+
+        client().prepareIndex("shapes", "shape_type", "Big_Rectangle").setSource(jsonBuilder().startObject()
+            .field("shape", shape).endObject()).setRefreshPolicy(IMMEDIATE).get();
+
+        ElasticsearchException e = expectThrows(ElasticsearchException.class, () -> client().prepareSearch("test").setTypes("type1")
+            .setQuery(geoIntersectionQuery("location", "Big_Rectangle", "shape_type")).get());
+        assertThat(e.getRootCause(), instanceOf(IllegalArgumentException.class));
+        assertThat(e.getRootCause().getMessage(), containsString("source disabled"));
+    }
+
     public void testReusableBuilder() throws IOException {
         ShapeBuilder polygon = ShapeBuilders.newPolygon(new CoordinatesBuilder()
                 .coordinate(170, -10).coordinate(190, -10).coordinate(190, 10).coordinate(170, 10).close())

core/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java

@@ -1148,6 +1148,7 @@ public class SearchQueryIT extends ESIntegTestCase {
                 jsonBuilder().startObject().startObject("type").startObject("properties")
                         .startObject("arr").startObject("properties").startObject("term").field("type", "text")
                         .endObject().endObject().endObject().endObject().endObject().endObject()));
+        assertAcked(prepareCreate("lookup3").addMapping("type", "_source", "enabled=false", "terms","type=text"));
         assertAcked(prepareCreate("test").addMapping("type", "term", "type=text"));
 
         indexRandom(true,
@@ -1172,6 +1173,7 @@ public class SearchQueryIT extends ESIntegTestCase {
                         .startObject().field("term", "4").endObject()
                         .endArray()
                         .endObject()),
+                client().prepareIndex("lookup3", "type", "1").setSource("terms", new String[]{"1", "3"}),
                 client().prepareIndex("test", "type", "1").setSource("term", "1"),
                 client().prepareIndex("test", "type", "2").setSource("term", "2"),
                 client().prepareIndex("test", "type", "3").setSource("term", "3"),
@@ -1227,6 +1229,16 @@ public class SearchQueryIT extends ESIntegTestCase {
         searchResponse = client().prepareSearch("test")
                 .setQuery(termsLookupQuery("not_exists", new TermsLookup("lookup2", "type", "3", "arr.term"))).get();
         assertHitCount(searchResponse, 0L);
+
+        // index "lookup" type "type" id "missing" document does not exist: ignore the lookup terms
+        searchResponse = client().prepareSearch("test")
+            .setQuery(termsLookupQuery("term" , new TermsLookup("lookup", "type", "missing", "terms"))).get();
+        assertHitCount(searchResponse, 0L);
+
+        // index "lookup3" type "type" has the source disabled: ignore the lookup terms
+        searchResponse = client().prepareSearch("test")
+            .setQuery(termsLookupQuery("term" , new TermsLookup("lookup3", "type", "1", "terms"))).get();
+        assertHitCount(searchResponse, 0L);
     }
 
     public void testBasicQueryById() throws Exception {

docs/reference/docs/get.asciidoc

@@ -146,7 +146,8 @@ You can also use the same source filtering parameters to control which parts of
 curl -XGET 'http://localhost:9200/twitter/tweet/1/_source?_source_include=*.id&_source_exclude=entities'
 --------------------------------------------------
 
-Note, there is also a HEAD variant for the _source endpoint to efficiently test for document existence.
+Note, there is also a HEAD variant for the _source endpoint to efficiently test for document _source existence.
+An existing document will not have a _source if it is disabled in the <<mapping-source-field,mapping>>.
 Curl example:
 
 [source,js]

modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java

@@ -359,6 +359,11 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder<PercolateQueryBu
                     "indexed document [{}/{}/{}] couldn't be found", indexedDocumentIndex, indexedDocumentType, indexedDocumentId
             );
         }
+        if(getResponse.isSourceEmpty()) {
+            throw new IllegalArgumentException(
+                "indexed document [" + indexedDocumentIndex + "/" + indexedDocumentType + "/" + indexedDocumentId + "] source disabled"
+            );
+        }
         return new PercolateQueryBuilder(field, documentType, getResponse.getSourceAsBytesRef());
     }
 
@@ -369,7 +374,7 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder<PercolateQueryBu
         }
 
         if (document == null) {
-            throw new IllegalStateException("nothing to percolator");
+            throw new IllegalStateException("no document to percolate");
         }
 
         MapperService mapperService = context.getMapperService();

modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java

@@ -19,6 +19,7 @@
 package org.elasticsearch.percolator;
 
 import org.apache.lucene.search.join.ScoreMode;
+import org.elasticsearch.action.search.SearchPhaseExecutionException;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
@@ -37,7 +38,6 @@ import org.elasticsearch.test.ESSingleNodeTestCase;
 import java.util.Collection;
 import java.util.Collections;
 
-import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE;
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.elasticsearch.index.query.QueryBuilders.boolQuery;
 import static org.elasticsearch.index.query.QueryBuilders.commonTermsQuery;
@@ -49,6 +49,7 @@ import static org.elasticsearch.index.query.QueryBuilders.spanNotQuery;
 import static org.elasticsearch.index.query.QueryBuilders.spanTermQuery;
 import static org.elasticsearch.index.query.QueryBuilders.termQuery;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 
@@ -158,6 +159,29 @@ public class PercolatorQuerySearchIT extends ESSingleNodeTestCase {
         assertThat(response.getHits().getAt(2).getId(), equalTo("3"));
     }
 
+    public void testPercolatorQueryExistingDocumentSourceDisabled() throws Exception {
+        createIndex("test", client().admin().indices().prepareCreate("test")
+            .addMapping("type", "_source", "enabled=false", "field1", "type=keyword")
+            .addMapping("queries", "query", "type=percolator")
+        );
+
+        client().prepareIndex("test", "queries", "1")
+            .setSource(jsonBuilder().startObject().field("query", matchAllQuery()).endObject())
+            .get();
+
+        client().prepareIndex("test", "type", "1").setSource("{}").get();
+        client().admin().indices().prepareRefresh().get();
+
+        logger.info("percolating empty doc with source disabled");
+        Throwable e = expectThrows(SearchPhaseExecutionException.class, () -> {
+            client().prepareSearch()
+                .setQuery(new PercolateQueryBuilder("query", "type", "test", "type", "1", null, null, null))
+                .get();
+        }).getRootCause();
+        assertThat(e, instanceOf(IllegalArgumentException.class));
+        assertThat(e.getMessage(), containsString("source disabled"));
+    }
+
     public void testPercolatorSpecificQueries()  throws Exception {
         createIndex("test", client().admin().indices().prepareCreate("test")
                 .addMapping("type", "field1", "type=text", "field2", "type=text")

rest-api-spec/src/main/resources/rest-api-spec/test/get_source/85_source_missing.yaml

@@ -0,0 +1,40 @@
+---
+setup:
+ - do:
+      indices.create:
+          index: test_1
+          body:
+            mappings:
+              test:
+                _source: { enabled: false }
+ - do:
+      cluster.health:
+          wait_for_status: yellow
+
+ - do:
+      index:
+          index:   test_1
+          type:    test
+          id:      1
+          body:    { foo: bar }
+
+
+---
+"Missing document source with catch":
+
+  - do:
+      catch:   missing
+      get_source:
+        index: test_1
+        type:  test
+        id:    1
+
+---
+"Missing document source with ignore":
+
+  - do:
+      get_source:
+        index:  test_1
+        type:   test
+        id:     1
+        ignore: 404