Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[STORM-3380] Use Objects.equals to avoid possible NullPointerException #2997

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bd2019us
Copy link
Contributor

@bd2019us bd2019us commented Apr 14, 2019

Hello,
I found that if the Map "data" does not have the key "TYPE", the String "compType" may cause potential risk of NullPointerException since it is immediately used after initialization and there is no null checker. One recommended API is Objects.equals(String,String) which can avoid this exception.

@srdo
Copy link
Contributor

srdo commented Apr 14, 2019

@bd2019us Thanks, looks great. Could you squash to one commit, and we can merge after the 24-hour waiting period?

@@ -2004,7 +2005,7 @@ private static ComponentPageInfo thriftifyCompPageData(
win2stats.put(FAILED, ClientStatsUtil.getMapByKey(data, WIN_TO_FAILED));

String compType = (String) data.get(TYPE);
if (compType.equals(ClientStatsUtil.SPOUT)) {
if (Objects.equals(compType,ClientStatsUtil.SPOUT)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like only avoiding NPE, which also makes misleading as component being interpreted as bolt. We should see what are the actual cases of data.get(TYPE) being null, and decide how to fix based on the case.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree. Maybe also consider throwing exceptions if it will ever be null. Need to understand it more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants