Skip to content

Fixed NPE in toString #2162

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 26, 2022
Merged

Fixed NPE in toString #2162

merged 1 commit into from
Mar 26, 2022

Conversation

dmatej
Copy link
Contributor

@dmatej dmatej commented Mar 25, 2022

  • getMessage can return also null char[] array; String.valueOf(char[]) doesn't expect that.

Signed-off-by: David Matějček [email protected]

@dmatej
Copy link
Contributor Author

dmatej commented Mar 25, 2022

Don't merge yet, neither previous nor this version works - it is a trap with generics :-/

@kofemann
Copy link
Contributor

kofemann commented Mar 25, 2022 •
edited
Loading

Instead of 'don't merge' warning you should change the PR type to 'Draft', then the merge will be disabled until you release it.

@pzygielo

This comment was marked as outdated.

Sign in to view
@dmatej
Copy link
Contributor Author

dmatej commented Mar 25, 2022

Instead of 'don't merge' warning you should change the PR type to 'Draft', then the merge will be disabled until you release it.

If I could I would do that, but there is no such option (Drafts disabled on this repo?)

The bug was visible in glassfish tests with full logging and also from Eclipse editor. Simply null typed as means "use the most concrete method compatible, which is with char[] argument, not Object."
Quite interesting, but String.valueOf(char[]) doesn't like null.

@dmatej
Copy link
Contributor Author

dmatej commented Mar 25, 2022

Original version vs. trivial test:
image

@dmatej
Fixed NPE in toString
d7a96f1 
- getMessage can return also null char[] array; String.valueOf(char[]) doesn't
  expect that.

Signed-off-by: David Matějček <[email protected]>
@dmatej
Copy link
Contributor Author

dmatej commented Mar 26, 2022

Note - grizzly-framework tests passed on my machine, seems like some conflict on ports. Also my test was executed after failing tests.

Error:  Tests run: 8, Failures: 0, Errors: 2, Skipped: 0, Time elapsed: 1.209 s <<< FAILURE! - in org.glassfish.grizzly.TCPNIOTransportTest
Error:  org.glassfish.grizzly.TCPNIOTransportTest.testClose  Time elapsed: 0.237 s
Error:  org.glassfish.grizzly.TCPNIOTransportTest.testThreadInterruptionDuringAcceptDoesNotMakeServerDeaf  Time elapsed: 0.554 s
Error:  org.glassfish.grizzly.TCPNIOTransportTest.testMultiBind  Time elapsed: 0.008 s  <<< ERROR!
java.net.BindException: Address already in use
	at org.glassfish.grizzly.TCPNIOTransportTest.testMultiBind(TCPNIOTransportTest.java:139)

@arjantijms arjantijms merged commit ec43978 into eclipse-ee4j:master Mar 26, 2022
@dmatej dmatej deleted the fixedNpe branch March 28, 2022 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pzygielo pzygielo pzygielo left review comments

@arjantijms arjantijms arjantijms approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dmatej @kofemann @pzygielo @arjantijms