Skip to content

Fix NullPointerException Risk in Dependency Extraction in BomPlugin #41744

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

Conversation

hyunmin0317
Copy link

Summary

This pull request addresses a potential NullPointerException issue in the BomPlugin class. The original code invoked the text() method on the result of findChild() without null-checking, which could lead to exceptions if findChild() returns null.

Changes Made

Introduced a new method extractText() to safely handle null values when extracting text.
Updated the usage of the text() method to use extractText() to prevent NullPointerException.

Details

The changes involve modifying the BomPlugin class as follows:

String groupId = extractText(findChild(dependency, "groupId"));
String artifactId = extractText(findChild(dependency, "artifactId"));

The extractText() method ensures that if findChild() returns null, it will not cause a NullPointerException, thus improving the robustness of the code.

Reason for Change

This modification enhances the stability of the code by handling potential null values gracefully, ensuring that the application can handle missing child elements without crashing.

Thank you for reviewing this pull request!

@pivotal-cla
Copy link

@hyunmin0317 Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-cla
Copy link

@hyunmin0317 Thank you for signing the Contributor License Agreement!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 8, 2024
@wilkinsona
Copy link
Member

Thanks for the PR but I don't think this change is necessary. There should always be <groupId> and <artifactId> nodes. If one or both of those nodes are missing then the bom that is being processed is an invalid state and we should fail hard as we currently do.

@wilkinsona wilkinsona closed this Aug 12, 2024
@wilkinsona wilkinsona added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 12, 2024
@hyunmin0317
Copy link
Author

Thank you very much for your thoughtful feedback.
I appreciate you taking the time to review the changes and share your insights.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants