Skip to content

SOLR-18318: Fix NPE in GetNodeSystemInfo when nodes param is set - #4661

Open
Yashgoswami-ds wants to merge 3 commits into
apache:mainfrom
Yashgoswami-ds:SOLR-18318-fix-node-system-npe
Open

SOLR-18318: Fix NPE in GetNodeSystemInfo when nodes param is set#4661
Yashgoswami-ds wants to merge 3 commits into
apache:mainfrom
Yashgoswami-ds:SOLR-18318-fix-node-system-npe

Conversation

@Yashgoswami-ds

Copy link
Copy Markdown

Description

Fixes an issue where the v2 /api/node/system endpoint throws a NullPointerException when the nodes parameter is provided in SolrCloud mode.

The issue occurs when GetNodeSystemInfo proxies requests to other nodes and tries to store the proxied responses in NodeSystemResponse.remoteNodeData. Since remoteNodeData was not initialized, adding node responses caused an NPE and returned an HTTP 500 error instead of the expected aggregated node information.

Solution

Initialized NodeSystemResponse.remoteNodeData with a LinkedHashMap so that proxied node responses can be safely stored.

Added a new SolrCloud test GetNodeSystemInfoSolrCloudTest that covers the failing scenario by requesting node system information with the nodes parameter and verifies that the response contains the expected node data.

Tests

Ran:

./gradlew :solr:core:test --tests GetNodeSystemInfoSolrCloudTest

@epugh

epugh commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

triggered CI. any chance you can look at the comment as well?? Be nice to know.

 // TODO The typing here is kindof wonky - can I tighten 'Object' here to be NodeSystemResponse or
  // will Jackson choke on that?

@Yashgoswami-ds

Copy link
Copy Markdown
Author

Thanks for the suggestion. I’ll take a look at the remoteNodeData usage and verify whether we can safely tighten the type to NodeSystemResponse without affecting Jackson serialization. I’ll update it if it is safe.

@epugh

epugh commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

looks like some precommit faiolures... I believe since this is a bugfix we need a changelog too.

@Yashgoswami-ds

Copy link
Copy Markdown
Author

triggered CI. any chance you can look at the comment as well?? Be nice to know.

 // TODO The typing here is kindof wonky - can I tighten 'Object' here to be NodeSystemResponse or
  // will Jackson choke on that?

Thanks for pointing this out. I looked into the usage of remoteNodeData.

The proxied response that we add in GetNodeSystemInfo is indeed a NodeSystemResponse, so I checked whether we could tighten the type to Map<String, NodeSystemResponse>. However, remoteNodeData is also populated through @JsonAnySetter, where Jackson provides dynamic fields with Object values.

Changing the type would require changing the @JsonAnySetter handling as well, and it could affect the current JSON serialization/deserialization behavior of this response model. Since this PR is focused on fixing the NPE caused by remoteNodeData being uninitialized, I have kept the existing Object type and only initialized the map with a LinkedHashMap so proxied node responses can be safely stored.

Thanks again for the suggestion.

@Yashgoswami-ds

Copy link
Copy Markdown
Author

please review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants