Skip to content

HDDS-16751. Fix possible NullPointerException in KeyInputStream#getBlockLocationInfo during block location refresh - #11435

Open
cchung100m wants to merge 2 commits into
apache:masterfrom
cchung100m:HDDS-16751
Open

cchung100m wants to merge 2 commits into
apache:masterfrom
cchung100m:HDDS-16751

Conversation

@cchung100m

@cchung100m cchung100m commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This PR adds null checks to KeyInputStream#getBlockLocationInfo, so the block-location refresh returns null instead of throwing a NullPointerException when the refreshed key info or its latest version locations are null, which BlockExtendedInputStream#refershBlockInfo already handles by logging a warning and continuing with the retry policy. The method is now package-private (@VisibleForTesting), and a new unit test, TestKeyInputStream, covers the null key info, missing location versions, and normal lookup cases.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-16751

How was this patch tested?

Existing unit and integration tests

…ockLocationInfo during block location refresh
@github-actions github-actions Bot added the client label Oct 7, 2026
@yandrey321

Copy link
Copy Markdown
Contributor

I'd add extra unit tests to cover new branches

@cchung100m

Copy link
Copy Markdown
Contributor Author

Hi @yandrey321
Thanks for the reminder. I updated the part you mentioned.

@cchung100m
cchung100m marked this pull request as ready for review October 8, 2026 05:28
@cchung100m

Copy link
Copy Markdown
Contributor Author

cc @adoroszlai @peterxcli

@peterxcli peterxcli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thinking instead of adding these null check, is it possible to refactor the code so we dont need them?

}

private static BlockLocationInfo getBlockLocationInfo(OmKeyInfo newKeyInfo,
@VisibleForTesting

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we no longer suggest adding @VisibleForTesting, but I forgot the actual reason. at least getter is harmless

Suggested change
@VisibleForTesting

@peterxcli
peterxcli requested a review from adoroszlai October 8, 2026 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants