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

[java producer] op.cmd may be null, cause NPE #9400

Merged
merged 2 commits into from
Feb 2, 2021

Conversation

xxxxpenny
Copy link
Contributor

check msg = op.cms == null, so use op.cmd. readableBytes() may throw NPE

@eolivelli
Copy link
Contributor

Do have seen this problem in production ? or is it just a static analysis of code that led you to this patch ?

thanks for sharing your fix

@xxxxpenny
Copy link
Contributor Author

xxxxpenny commented Feb 1, 2021

No, just a static analysis of code that led me to this patch。

@@ -1518,8 +1518,7 @@ private void stripChecksum(OpSendMsg op) {
headerFrame.resetReaderIndex();
}
} else {
log.warn("[{}] Failed while casting {} into ByteBufPair", producerName,
(op.cmd == null ? null : op.cmd.getClass().getName()));
log.warn("[{}] Failed while casting {} into ByteBufPair", producerName, null);
Copy link
Contributor

Choose a reason for hiding this comment

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

here you do not need to pass null, you can simply write null in the message

@eolivelli
Copy link
Contributor

I left one suggestion.
Can you please change the title pointing out that we are in the Producer, in the Java Client ?

@xxxxpenny xxxxpenny changed the title op.cmd may be null, cause NPE [java producer]op.cmd may be null, cause NPE Feb 1, 2021
@xxxxpenny xxxxpenny changed the title [java producer]op.cmd may be null, cause NPE [java producer] op.cmd may be null, cause NPE Feb 1, 2021
@xxxxpenny
Copy link
Contributor Author

xxxxpenny commented Feb 1, 2021

yeah,I change the title.

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

@sijie sijie added this to the 2.8.0 milestone Feb 2, 2021
@sijie sijie added java type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages labels Feb 2, 2021
@merlimat merlimat merged commit 60f6238 into apache:master Feb 2, 2021
Anonymitaet pushed a commit that referenced this pull request Feb 3, 2021
* op.cmd may be null, cause NPE

* write null in the log
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants