Skip to content

Fix NPE in bitmap terms lookup when stored field is missing#22208

Open
kshitizj03 wants to merge 1 commit into
opensearch-project:mainfrom
kshitizj03:fix/21995-bitmap-lookup-missing-field-npe
Open

Fix NPE in bitmap terms lookup when stored field is missing#22208
kshitizj03 wants to merge 1 commit into
opensearch-project:mainfrom
kshitizj03:fix/21995-bitmap-lookup-missing-field-npe

Conversation

@kshitizj03

Copy link
Copy Markdown

Description

A terms query using terms lookup with value_type: bitmap and store: true throws a
NullPointerException when the looked-up document exists but the stored field at path is
missing.

Root cause: in TermsQueryBuilder#fetch, getResponse.getField(termsLookup.path()) returns
null for an absent stored field, and getValues() was called on it unconditionally.

This change guards the null DocumentField and treats a missing stored field as no terms
(no matches), which is consistent with how a missing source field is already handled. Existing
single-value bitmap validation is preserved for the present-field case.

Added a REST yaml test in search/380_bitmap_filtering.yml: a lookup document missing the stored
bitmap field now returns hits.total: 0 instead of erroring.

Related Issues

Resolves #21995

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

A terms query using terms lookup with value_type: bitmap and store: true
threw a NullPointerException when the looked-up document existed but the
stored field at `path` was absent. GetResponse.getField(path) returns null
for an absent stored field, and getValues() was called on it unconditionally.

Guard the null DocumentField and treat a missing stored field as no terms
(no matches), which matches the behavior for a missing source field.

Fixes opensearch-project#21995

Signed-off-by: Kshitiz Jain <kshitizj@gmail.com>
@github-actions

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@github-actions

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Add null check for values list

Add a null check for storedField.getValues() before accessing it. Even when
storedField is not null, getValues() could potentially return null, which would
cause a NullPointerException when calling size().

server/src/main/java/org/opensearch/index/query/TermsQueryBuilder.java [586-597]

 DocumentField storedField = getResponse.getField(termsLookup.path());
 // A stored field that is absent from the looked-up document yields a null
 // DocumentField; treat it as no terms (no matches) instead of failing.
 if (storedField != null) {
     List<Object> values = storedField.getValues();
-    if (values.size() != 1 && valueType == ValueType.BITMAP) {
-        throw new IllegalArgumentException(
-            "Invalid value for bitmap type: Expected a single base64 encoded serialized bitmap."
-        );
+    if (values != null) {
+        if (values.size() != 1 && valueType == ValueType.BITMAP) {
+            throw new IllegalArgumentException(
+                "Invalid value for bitmap type: Expected a single base64 encoded serialized bitmap."
+            );
+        }
+        terms.addAll(values);
     }
-    terms.addAll(values);
 }
Suggestion importance[1-10]: 3

__

Why: While adding a null check for values is defensive programming, the DocumentField.getValues() method typically returns an empty list rather than null. The suggestion addresses a theoretical edge case but may not reflect actual API behavior, making it a minor improvement at best.

Low

@github-actions

Copy link
Copy Markdown
Contributor

❌ Gradle check result for 3b8d5e2: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@kshitizj03

Copy link
Copy Markdown
Author

The gradle-check failure looks like an unrelated CI flake rather than a real test failure.

The failing task is :plugins:analysis-phonetic:test, and the cause is a Gradle worker connection problem, not a test assertion:

org.gradle.internal.remote.internal.ConnectException: Could not connect to server [...127.0.0.1...]. Connection refused
MessageIOException: Could not read message from '/127.0.0.1:49564'

The Jenkins run also reports "Test Result (no failures)", and :server:test passed in the same run. This change only touches bitmap terms lookup in :server, and the bitmap REST suite (search/380_bitmap_filtering, including the new missing-field case) passes locally.

Could a maintainer please re-run gradle-check? Thanks!

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

Labels

bug Something isn't working good first issue Good for newcomers Search:Query Capabilities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Bitmap terms lookup throws NPE when stored lookup field is missing

1 participant