Skip to content
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 NPE caused by null value of SchemaInfo's properties #9985

Merged

Conversation

BewareMyPower
Copy link
Contributor

Fixes #9964

Motivation

If there exists a null value of SchemaInfo's properties, NPE will be thrown in SchemaInfo#toString or Commands#newSubscribe.

Modifications

  • Add null checks before JsonPrimitive's constructor, KeyValue#setKey and KeyValue#setValue.
  • Add related tests.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows: SchemaTest#testNullKeyValueProperty and SchemaInfoTest#testNullPropertyValue.

@BewareMyPower
Copy link
Contributor Author

@jianyun8023

I've pushed a PR to fix your issue. But I'm curious about why did you set a property value to null? When Gson serializes a JSON object to String, the null values will be skipped. Is it your intended behavior? If no, it may be conflict with the current implementation.

@BewareMyPower
Copy link
Contributor Author

/pulsarbot run-failure-checks

1 similar comment
@BewareMyPower
Copy link
Contributor Author

/pulsarbot run-failure-checks

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

LGTM

@BewareMyPower
Copy link
Contributor Author

/pulsarbot run-failure-checks

@BewareMyPower
Copy link
Contributor Author

/pulsarbot run-failure-checks

@codelipenghui codelipenghui added the type/bug The PR fixed a bug or issue reported a bug label Mar 22, 2021
@codelipenghui codelipenghui added this to the 2.8.0 milestone Mar 22, 2021
@codelipenghui codelipenghui merged commit ee5b558 into apache:master Mar 22, 2021
@BewareMyPower BewareMyPower deleted the bewaremypower/schema-prop-npe branch March 22, 2021 03:32
yangl pushed a commit to yangl/pulsar that referenced this pull request Mar 22, 2021
Fixes apache#9964 

### Motivation

If there exists a null value of `SchemaInfo`'s properties, NPE will be thrown in `SchemaInfo#toString` or `Commands#newSubscribe`.

### Modifications

- Add null checks before `JsonPrimitive`'s constructor, `KeyValue#setKey` and `KeyValue#setValue`.
- Add related tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] SchemaInfo toString occurs NPE
4 participants