Skip to content

Add explicit null checks in OpenSslX509KeyManagerFactory#11230

Merged
normanmaurer merged 2 commits intonetty:4.1from
skyguard1:add_not_null_check
May 7, 2021
Merged

Add explicit null checks in OpenSslX509KeyManagerFactory#11230
normanmaurer merged 2 commits intonetty:4.1from
skyguard1:add_not_null_check

Conversation

@skyguard1
Copy link
Copy Markdown
Contributor

@skyguard1 skyguard1 commented May 7, 2021

Motivation:

The return value of io.netty.handler.ssl.SslContext.toX509Certificates() may be null, so the non-null check is required for the return value

Modification:

Should do the non-null check on the return value of io.netty.handler.ssl.SslContext.toX509Certificates() in SslContext and OpenSslX509KeyManagerFactory, thanks

Result:

Added a non-null check for the return value of io.netty.handler.ssl.SslContext.toX509Certificates()

…slX509KeyManagerFactory

Signed-off-by: xingrufei <xingrufei@sogou-inc.com>
Copy link
Copy Markdown
Contributor

@hyperxpro hyperxpro left a comment

Choose a reason for hiding this comment

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

This will break API as users are not expecting NullPointerException to be thrown. @normanmaurer WDYT?

@normanmaurer
Copy link
Copy Markdown
Member

@hyperxpro actually I think this is fine as otherwise it would also throw either NPE or something else once trying to use it.

@normanmaurer
Copy link
Copy Markdown
Member

@chrisvest WDYT ?

@chrisvest
Copy link
Copy Markdown
Member

The OpenSslX509KeyManagerFactory changes look fine.

Signed-off-by: xingrufei <xingrufei@sogou-inc.com>
@normanmaurer normanmaurer changed the title Add not null check to the return value of toX509Certificates in OpenSslX509KeyManagerFactory Add explicit null checks in OpenSslX509KeyManagerFactory May 7, 2021
@normanmaurer normanmaurer merged commit bb7b05b into netty:4.1 May 7, 2021
@normanmaurer normanmaurer added this to the 4.1.64.Final milestone May 7, 2021
normanmaurer pushed a commit that referenced this pull request May 7, 2021
Motivation:

We should add explicit null checks so its easier for people to understand why it throws.

Modification:

Add explicit checkNotNull(...)

Result:

Easier to understand for users why it fails.

Signed-off-by: xingrufei <xingrufei@sogou-inc.com>

Co-authored-by: xingrufei <xingrufei@sogou-inc.com>
raidyue pushed a commit to raidyue/netty that referenced this pull request Jul 8, 2022
Motivation:

We should add explicit null checks so its easier for people to understand why it throws.

Modification:

Add explicit checkNotNull(...)

Result:

Easier to understand for users why it fails.

Signed-off-by: xingrufei <xingrufei@sogou-inc.com>

Co-authored-by: xingrufei <xingrufei@sogou-inc.com>
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.

4 participants