Skip to content

Commit de1f6c2

Browse files
committed
Handle Null in FetchSourceContext#fetchSource (#36839)
This change ensures that when null is passed as the value to one of UpdateRequest#fetchSource @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 explicitly for Null and not for arrays of Null objects. In master, the problematic behavior was corrected as part of #29293
1 parent 2abb8b5 commit de1f6c2

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
@@ -403,7 +403,9 @@ public UpdateRequest fields(String... fields) {
403403
*/
404404
public UpdateRequest fetchSource(@Nullable String include, @Nullable String exclude) {
405405
FetchSourceContext context = this.fetchSourceContext == null ? FetchSourceContext.FETCH_SOURCE : this.fetchSourceContext;
406-
this.fetchSourceContext = new FetchSourceContext(context.fetchSource(), new String[] {include}, new String[]{exclude});
406+
String[] includes = include == null ? Strings.EMPTY_ARRAY : new String[]{include};
407+
String[] excludes = exclude == null ? Strings.EMPTY_ARRAY : new String[]{exclude};
408+
this.fetchSourceContext = new FetchSourceContext(context.fetchSource(), includes, excludes);
407409
return this;
408410
}
409411

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)