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.
This commit is contained in:
David Pilato 2014-01-28 16:52:09 +01:00
parent dd389d1cc5
commit d621ab9958
6 changed files with 30 additions and 64 deletions

View File

@ -24,12 +24,11 @@ import org.elasticsearch.action.get.MultiGetRequest;
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 class RestMultiGetAction extends BaseRestHandler {
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);

View File

@ -24,12 +24,11 @@ import org.elasticsearch.action.percolate.MultiPercolateResponse;
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 class RestMultiPercolateAction extends BaseRestHandler {
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));

View File

@ -26,8 +26,6 @@ import org.elasticsearch.action.support.IndicesOptions;
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 @@ public class RestPercolateAction extends BaseRestHandler {
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);

View File

@ -25,12 +25,11 @@ import org.elasticsearch.action.search.MultiSearchResponse;
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 class RestMultiSearchAction extends BaseRestHandler {
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);

View File

@ -23,6 +23,8 @@ import org.elasticsearch.ElasticsearchIllegalArgumentException;
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 class RestActions {
}
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;
}
}

View File

@ -25,12 +25,11 @@ import org.elasticsearch.action.termvector.MultiTermVectorsResponse;
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 class RestMultiTermVectorsAction extends BaseRestHandler {
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));