Skip to content

Commit 5148f1f

Browse files
committed
Handle null in fetchSource includes/excludes
This change ensures that when null is passed as the value to one of its Nullable parameters, it is not wrapped in a String array. That would in turn cause a NPE when attempting to serialize FetchSourceContext as its constructor checks for null values only. In master, the problematic behavior was corrected as part of elastic#29293
1 parent b54783d commit 5148f1f

File tree

2 files changed

+31
-1
lines changed

2 files changed

+31
-1
lines changed

server/src/main/java/org/elasticsearch/action/update/UpdateRequest.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,9 @@ public UpdateRequest fields(String... fields) {
410410
*/
411411
public UpdateRequest fetchSource(@Nullable String include, @Nullable String exclude) {
412412
FetchSourceContext context = this.fetchSourceContext == null ? FetchSourceContext.FETCH_SOURCE : this.fetchSourceContext;
413-
this.fetchSourceContext = new FetchSourceContext(context.fetchSource(), new String[] {include}, new String[]{exclude});
413+
String[] includes = include == null ? Strings.EMPTY_ARRAY : new String[]{include};
414+
String[] excludes = exclude == null ? Strings.EMPTY_ARRAY : new String[]{exclude};
415+
this.fetchSourceContext = new FetchSourceContext(context.fetchSource(), includes, excludes);
414416
return this;
415417
}
416418

server/src/test/java/org/elasticsearch/update/UpdateIT.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,34 @@ public void testUpdate() throws Exception {
379379
assertThat(updateResponse.getGetResult().sourceAsMap().size(), equalTo(1));
380380
assertThat(updateResponse.getGetResult().sourceAsMap().get("field1"), equalTo(2));
381381

382+
// check updates with null excludes
383+
client().prepareIndex("test", "type1", "1").setSource("field1", 1, "field2", 2).execute().actionGet();
384+
updateResponse = client().prepareUpdate(indexOrAlias(), "type1", "1")
385+
.setScript(new Script(ScriptType.INLINE, UPDATE_SCRIPTS, FIELD_INC_SCRIPT, Collections.singletonMap("field", "field1")))
386+
.setFetchSource("field1", null)
387+
.get();
388+
assertThat(updateResponse.getIndex(), equalTo("test"));
389+
assertThat(updateResponse.getGetResult(), notNullValue());
390+
assertThat(updateResponse.getGetResult().getIndex(), equalTo("test"));
391+
assertThat(updateResponse.getGetResult().sourceRef(), notNullValue());
392+
assertThat(updateResponse.getGetResult().field("field1"), nullValue());
393+
assertThat(updateResponse.getGetResult().sourceAsMap().size(), equalTo(1));
394+
assertThat(updateResponse.getGetResult().sourceAsMap().get("field1"), equalTo(2));
395+
396+
// check updates with null includes
397+
client().prepareIndex("test", "type1", "1").setSource("field1", 1, "field2", 2).execute().actionGet();
398+
updateResponse = client().prepareUpdate(indexOrAlias(), "type1", "1")
399+
.setScript(new Script(ScriptType.INLINE, UPDATE_SCRIPTS, FIELD_INC_SCRIPT, Collections.singletonMap("field", "field1")))
400+
.setFetchSource(null, "field1")
401+
.get();
402+
assertThat(updateResponse.getIndex(), equalTo("test"));
403+
assertThat(updateResponse.getGetResult(), notNullValue());
404+
assertThat(updateResponse.getGetResult().getIndex(), equalTo("test"));
405+
assertThat(updateResponse.getGetResult().sourceRef(), notNullValue());
406+
assertThat(updateResponse.getGetResult().field("field2"), nullValue());
407+
assertThat(updateResponse.getGetResult().sourceAsMap().size(), equalTo(1));
408+
assertThat(updateResponse.getGetResult().sourceAsMap().get("field2"), equalTo(2));
409+
382410
// check updates without script
383411
// add new field
384412
client().prepareIndex("test", "type1", "1").setSource("field", 1).execute().actionGet();

0 commit comments

Comments
 (0)