Skip to content

Fix NPE in ArrayUtils.toStringArray() when array contains nulls #1413

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

Closed

Conversation

Naveenkumarsuk
Copy link

This fixes an issue in ArrayUtils.toStringArray(Object[]) where passing null elements causes a NullPointerException.
The method now returns "null" for null elements, similar to how String.valueOf() behaves.

@garydgregory
Copy link
Member

garydgregory commented Jul 16, 2025

Hello @Naveenkumarsuk
This PR doesn't fix anything, instead it adds a new API, so why call the PR "Fix NPE in ArrayUtils.toStringArray() when array contains nulls".

Also, this PR breaks the build, it doesn't even complile.

TY

@Naveenkumarsuk
Copy link
Author

Naveenkumarsuk commented Jul 17, 2025

@garydgregory Thank you for the feedback and for pointing this out. You're absolutely right as the current implementation introduces a new overload rather than fixing the existing method, and I mistakenly titled the PR as a "fix" and sorry about that.

My original intention was to safely handle null elements inside the array, but I now realize that changing the existing toStringArray(Object[]) method directly (while preserving backward compatibility) is the correct approach.

I'll update the PR shortly to modify the existing method instead of adding a new one, and ensure it compiles and passes all tests. Thanks again for your guidance.

@garydgregory
Copy link
Member

@Naveenkumarsuk
This PR is too messy WRT formatting and fails the build anyway. You've moved the method instead of editing it in place, which makes it impossible to see clearly what's changed. Note that the PR messes up the indentation as well.

Note:

git apply \tmp\lang-1413.diff
/tmp/lang-1413.diff:69: trailing whitespace.
    
warning: 1 line adds whitespace errors.

You're NOT testing the PR:

[INFO] --- checkstyle:3.6.0:check (default-cli) @ commons-lang3 ---
[INFO] There are 3 errors reported by Checkstyle 10.25.0 with src/site/resources/checkstyle/checkstyle.xml ruleset.
[ERROR] src\main\java\org\apache\commons\lang3\ArrayUtils.java:[9586] (regexp) RegexpSingleline: Line has trailing spaces.
[ERROR] src\test\java\org\apache\commons\lang3\ArrayUtilsTest.java:[95,14] (coding) FinalLocalVariable: Variable 'input' should be declared final.
[ERROR] src\test\java\org\apache\commons\lang3\ArrayUtilsTest.java:[96,14] (coding) FinalLocalVariable: Variable 'result' should be declared final.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------

Run mvn (by itself) as documented in the PR template: https://github.com/apache/commons-lang/blob/master/.github/pull_request_template.md

TY

@Naveenkumarsuk
Copy link
Author

Thanks for the honest feedback on the previous PR — that helped a lot.

I've opened a fresh PR here (#1415) where:

  • The method is now edited in place (no more moving it around)
  • Formatting is left clean and untouched
  • I added a small test to handle the null case properly
  • Ran mvn clean verify and made sure everything passes

Let me know if you spot anything else — happy to improve further.

TY

@garydgregory
Copy link
Member

Closing broken PR.

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

Successfully merging this pull request may close these issues.

2 participants