Fix potential NPE when no source and no body

In recent changes, we added missing support for `source` parameter in some REST APIs: * #4892 : mget * #4900 : mpercolate * #4901 : msearch * #4902 : mtermvectors * #4903 : percolate ```java BytesReference content = null; if (request.hasContent()) { content = request.content(); } else { String source = request.param("source"); if (source != null) { content = new BytesArray(source); } } ``` It's definitely better to have: ```java BytesReference content = request.content(); if (!request.hasContent()) { String source = request.param("source"); if (source != null) { content = new BytesArray(source); } } ``` That said, it could be nice to have a single method to manage it for various REST actions. Closes #4924.
elastic · Jan 28, 2014 · 4e1a947 · 4e1a947
1 parent 80813b4
commit 4e1a947
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 64 deletions.
diff --git a/src/main/java/org/elasticsearch/rest/action/get/RestMultiGetAction.java b/src/main/java/org/elasticsearch/rest/action/get/RestMultiGetAction.java
@@ -24,12 +24,11 @@
 import org.elasticsearch.action.get.MultiGetResponse;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.common.Strings;
-import org.elasticsearch.common.bytes.BytesArray;
-import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.rest.*;
+import org.elasticsearch.rest.action.support.RestActions;
 import org.elasticsearch.search.fetch.source.FetchSourceContext;
 
 import java.io.IOException;
@@ -73,18 +72,8 @@ public void handleRequest(final RestRequest request, final RestChannel channel)
 
         FetchSourceContext defaultFetchSource = FetchSourceContext.parseFromRestRequest(request);
 
-        BytesReference content = null;
-        if (request.hasContent()) {
-            content = request.content();
-        } else {
-            String source = request.param("source");
-            if (source != null) {
-                content = new BytesArray(source);
-            }
-        }
-
         try {
-            multiGetRequest.add(request.param("index"), request.param("type"), sFields, defaultFetchSource, request.param("routing"), content, allowExplicitIndex);
+            multiGetRequest.add(request.param("index"), request.param("type"), sFields, defaultFetchSource, request.param("routing"), RestActions.getRestContent(request), allowExplicitIndex);
         } catch (Exception e) {
             try {
                 XContentBuilder builder = restContentBuilder(request);

diff --git a/src/main/java/org/elasticsearch/rest/action/percolate/RestMultiPercolateAction.java b/src/main/java/org/elasticsearch/rest/action/percolate/RestMultiPercolateAction.java
@@ -24,12 +24,11 @@
 import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.common.Strings;
-import org.elasticsearch.common.bytes.BytesArray;
-import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.rest.*;
+import org.elasticsearch.rest.action.support.RestActions;
 
 import java.io.IOException;
 
@@ -66,18 +65,8 @@ public void handleRequest(final RestRequest restRequest, final RestChannel restC
         multiPercolateRequest.indices(Strings.splitStringByCommaToArray(restRequest.param("index")));
         multiPercolateRequest.documentType(restRequest.param("type"));
 
-        BytesReference content = null;
-        if (restRequest.hasContent()) {
-            content = restRequest.content();
-        } else {
-            String source = restRequest.param("source");
-            if (source != null) {
-                content = new BytesArray(source);
-            }
-        }
-
         try {
-            multiPercolateRequest.add(content, restRequest.contentUnsafe(), allowExplicitIndex);
+            multiPercolateRequest.add(RestActions.getRestContent(restRequest), restRequest.contentUnsafe(), allowExplicitIndex);
         } catch (Exception e) {
             try {
                 restChannel.sendResponse(new XContentThrowableRestResponse(restRequest, e));

diff --git a/src/main/java/org/elasticsearch/rest/action/percolate/RestPercolateAction.java b/src/main/java/org/elasticsearch/rest/action/percolate/RestPercolateAction.java
@@ -26,8 +26,6 @@
 import org.elasticsearch.action.support.broadcast.BroadcastOperationThreading;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.common.Strings;
-import org.elasticsearch.common.bytes.BytesArray;
-import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -71,18 +69,7 @@ void parseDocPercolate(PercolateRequest percolateRequest, RestRequest restReques
         percolateRequest.documentType(restRequest.param("type"));
         percolateRequest.routing(restRequest.param("routing"));
         percolateRequest.preference(restRequest.param("preference"));
-
-        BytesReference content = null;
-        if (restRequest.hasContent()) {
-            content = restRequest.content();
-        } else {
-            String source = restRequest.param("source");
-            if (source != null) {
-                content = new BytesArray(source);
-            }
-        }
-
-        percolateRequest.source(content, restRequest.contentUnsafe());
+        percolateRequest.source(RestActions.getRestContent(restRequest), restRequest.contentUnsafe());
 
         percolateRequest.indicesOptions(IndicesOptions.fromRequest(restRequest, percolateRequest.indicesOptions()));
         executePercolate(percolateRequest, restRequest, restChannel);

diff --git a/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java b/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java
@@ -25,12 +25,11 @@
 import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.common.Strings;
-import org.elasticsearch.common.bytes.BytesArray;
-import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.rest.*;
+import org.elasticsearch.rest.action.support.RestActions;
 
 import java.io.IOException;
 
@@ -69,18 +68,8 @@ public void handleRequest(final RestRequest request, final RestChannel channel)
         String[] types = Strings.splitStringByCommaToArray(request.param("type"));
         IndicesOptions indicesOptions = IndicesOptions.fromRequest(request, multiSearchRequest.indicesOptions());
 
-        BytesReference content = null;
-        if (request.hasContent()) {
-            content = request.content();
-        } else {
-            String source = request.param("source");
-            if (source != null) {
-                content = new BytesArray(source);
-            }
-        }
-
         try {
-            multiSearchRequest.add(content, request.contentUnsafe(), indices, types, request.param("search_type"), request.param("routing"), indicesOptions, allowExplicitIndex);
+            multiSearchRequest.add(RestActions.getRestContent(request), request.contentUnsafe(), indices, types, request.param("search_type"), request.param("routing"), indicesOptions, allowExplicitIndex);
         } catch (Exception e) {
             try {
                 XContentBuilder builder = restContentBuilder(request);

diff --git a/src/main/java/org/elasticsearch/rest/action/support/RestActions.java b/src/main/java/org/elasticsearch/rest/action/support/RestActions.java
@@ -23,6 +23,8 @@
 import org.elasticsearch.action.ShardOperationFailedException;
 import org.elasticsearch.action.support.QuerySourceBuilder;
 import org.elasticsearch.action.support.broadcast.BroadcastOperationResponse;
+import org.elasticsearch.common.bytes.BytesArray;
+import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentBuilderString;
 import org.elasticsearch.index.query.QueryBuilders;
@@ -101,4 +103,23 @@ public static QuerySourceBuilder parseQuerySource(RestRequest request) {
         }
         return new QuerySourceBuilder().setQuery(queryBuilder);
     }
+
+    /**
+     * Get Rest content from either payload or source parameter
+     * @param request Rest request
+     * @return rest content
+     */
+    public static BytesReference getRestContent(RestRequest request) {
+        assert request != null;
+
+        BytesReference content = request.content();
+        if (!request.hasContent()) {
+            String source = request.param("source");
+            if (source != null) {
+                content = new BytesArray(source);
+            }
+        }
+
+        return content;
+    }
 }
diff --git a/src/main/java/org/elasticsearch/rest/action/termvector/RestMultiTermVectorsAction.java b/src/main/java/org/elasticsearch/rest/action/termvector/RestMultiTermVectorsAction.java
@@ -25,12 +25,11 @@
 import org.elasticsearch.action.termvector.TermVectorRequest;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.common.Strings;
-import org.elasticsearch.common.bytes.BytesArray;
-import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.rest.*;
+import org.elasticsearch.rest.action.support.RestActions;
 
 import static org.elasticsearch.rest.RestRequest.Method.GET;
 import static org.elasticsearch.rest.RestRequest.Method.POST;
@@ -61,16 +60,8 @@ public void handleRequest(final RestRequest request, final RestChannel channel)
         RestTermVectorAction.readURIParameters(template, request);
         multiTermVectorsRequest.ids(Strings.commaDelimitedListToStringArray(request.param("ids")));
 
-        BytesReference content = request.content();
-        if (!request.hasContent()) {
-            String source = request.param("source");
-            if (source != null) {
-                content = new BytesArray(source);
-            }
-        }
-
         try {
-            multiTermVectorsRequest.add(template, content);
+            multiTermVectorsRequest.add(template, RestActions.getRestContent(request));
         } catch (Throwable t) {
             try {
                 channel.sendResponse(new XContentThrowableRestResponse(request, t));