Skip to content

Fix NPE in _listAddAll when server returns JSON null for a list field - #2041

Open
gingeekrishna wants to merge 5 commits into
opensearch-project:mainfrom
gingeekrishna:fix/1813-listaddall-null-values
Open

Fix NPE in _listAddAll when server returns JSON null for a list field#2041
gingeekrishna wants to merge 5 commits into
opensearch-project:mainfrom
gingeekrishna:fix/1813-listaddall-null-values

Conversation

@gingeekrishna

Copy link
Copy Markdown

Fixes #1813

_listAddAll(list, values) threw NullPointerException via Objects.requireNonNull(values) when values was null. This happens when OpenSearch returns JSON null for an optional list field (e.g. ism_template in ISM Policy), causing deserialization to fail:

NullPointerException at ObjectBuilderBase._listAddAll
  at Policy$Builder.ismTemplate(Policy.java:379)

Fix: Guard against null values by returning the existing list unchanged when values is null. Same logic applied to _mapPutAll would follow the same pattern if similar issues arise for maps.

Copilot AI review requested due to automatic review settings July 5, 2026 08:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

gingeekrishna added a commit to gingeekrishna/opensearch-java that referenced this pull request Jul 5, 2026
Signed-off-by: Radhakrishnan Pachyappan <gingeekrishna@gmail.com>
$ref: '#/components/schemas/_core.search___FetchProfile'
type: array
items:
$ref: '#/components/schemas/_core.search___FetchProfile'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, this is the way we generate Java client, thank you @Hailong-am

gingeekrishna added a commit to gingeekrishna/opensearch-java that referenced this pull request Aug 16, 2026
Signed-off-by: Radhakrishnan Pachyappan <gingeekrishna@gmail.com>
@gingeekrishna
gingeekrishna force-pushed the fix/1813-listaddall-null-values branch from dc9939a to 2abeeb0 Compare August 16, 2026 16:15
gingeekrishna added a commit to gingeekrishna/opensearch-java that referenced this pull request Aug 30, 2026
Signed-off-by: Radhakrishnan Pachyappan <gingeekrishna@gmail.com>
@gingeekrishna
gingeekrishna force-pushed the fix/1813-listaddall-null-values branch from 2abeeb0 to 8cad090 Compare August 30, 2026 06:28
gingeekrishna and others added 4 commits September 4, 2026 11:26
…etchProfile>

OpenSearch returns fetch as a JSON array in profile responses but the
spec defined it as a single FetchProfile object. This caused:
  UnexpectedJsonEventException: Unexpected JSON event 'START_ARRAY'
  instead of '[START_OBJECT, KEY_NAME]'

Change the spec to type: array / items: FetchProfile and update the
generated ShardProfile.java to use List<FetchProfile> with array
serialization and arrayDeserializer, matching the pattern already
used by aggregations and searches in the same class.

Fixes opensearch-project#1965

Signed-off-by: Radhakrishnan Pachyappan <gingeekrishna@gmail.com>
Signed-off-by: Radhakrishnan Pachyappan <gingeekrishna@gmail.com>
_listAddAll(list, values) threw NullPointerException via
Objects.requireNonNull(values) when the incoming values list was null.
This happens when OpenSearch returns JSON null for an optional list
field (e.g. ism_template in ISM Policy), causing deserialization to
fail with an NPE inside the builder.

Guard against null values by returning the existing list unchanged
when values is null, consistent with the intent of treating a missing
or null list as equivalent to an empty one.

Fixes opensearch-project#1813

Signed-off-by: Radhakrishnan Pachyappan <gingeekrishnan@gmail.com>
Signed-off-by: Radhakrishnan Pachyappan <gingeekrishna@gmail.com>
@gingeekrishna
gingeekrishna force-pushed the fix/1813-listaddall-null-values branch from 8cad090 to af9cdc0 Compare September 4, 2026 05:57
The previous commit made _listAddAll tolerate a null values list by
returning the existing list unchanged. That was too broad: _listAddAll
is the same helper generated builder setters use for direct public API
calls (e.g. b.storedFields(nullFields)), and ClassStructureTest.
testListSetters explicitly asserts that passing null to a list setter
throws NullPointerException - a real, tested API contract this broke.

CI caught it: testListSetters[json-b] and testListSetters[jackson]
both failed after the previous push.

The actual bug (opensearch-project#1813) is specific to deserialization: when the server
sends JSON null for an array field, JsonpDeserializer's default
deserialize(parser, mapper) short-circuits to Java null for any
deserializer that doesn't accept Event.VALUE_NULL - which is exactly
what ArrayDeserializer.acceptedEvents() didn't do. That Java null then
gets passed straight into the generated field setter (the same setter
public callers use), which passes it to _listAddAll, which throws.

Fix it at the deserializer instead: ArrayDeserializer now accepts
Event.VALUE_NULL and returns an empty list for it, so a null array in
server JSON becomes an empty list before it ever reaches the setter -
no different from an absent field. This is the correct, narrowly-
scoped fix: it only changes what array deserialization does with a
JSON null, leaving _listAddAll (and the public builder API contract
tested by testListSetters) exactly as it always was. Reverts the
_listAddAll change from the previous commit.

Adds JsonpDeserializerBaseTest#testNullArray covering the
top-level-null case, alongside the existing testNullArrayItem which
covers null items within an array.

Signed-off-by: Radhakrishnan P <gingeekrishna@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] required spec not respected for maps and lists

4 participants