-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
fix NullPointerException hazard in StringUtils.join(..) method #3983
fix NullPointerException hazard in StringUtils.join(..) method #3983
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3983 +/- ##
=========================================
Coverage 50.76% 50.76%
+ Complexity 2468 2462 -6
=========================================
Files 475 475
Lines 14600 14580 -20
Branches 1521 1514 -7
=========================================
- Hits 7412 7402 -10
+ Misses 6661 6651 -10
Partials 527 527
Continue to review full report at Codecov.
|
@@ -355,11 +355,11 @@ public String format(T obj) { | |||
|
|||
public static <T> String join(Collection<T> collection, String separator, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems this method is only used in tests, let's just remove it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed method and changed the commit message to 'delete unused method StringUtils.join(..)'
Signed-off-by: WillardHu <[email protected]>
58771ba
to
9e30ebe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, although this is kind of breaking changes in apollo-client
, I think these 2 methods are not meant to be used by users, and it's easy to migrate to other string util, I'm OK to remove them, WDYT @nobodyiam
@kezhenxu94 I agree with you that it's ok to remove these 2 methods |
Signed-off-by: WillardHu [email protected]
What's the purpose of this PR
When the
collection
is null, aNullPointerException
is thrown here. Otherwiseiterator == null
is always false.Which issue(s) this PR fixes:
Fixes #
Brief changelog
fix NullPointerException hazard in StringUtils.join(..) method.
Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean test
to make sure this pull request doesn't break anything.CHANGES
log.