You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by shroman <gi...@git.apache.org> on 2017/01/06 10:39:29 UTC
[GitHub] incubator-rocketmq pull request #30: [ROCKETMQ-34] Potential NPE in NettyCon...
GitHub user shroman opened a pull request:
https://github.com/apache/incubator-rocketmq/pull/30
[ROCKETMQ-34] Potential NPE in NettyConnetManageHandler#connect
JIRA issue: https://issues.apache.org/jira/browse/ROCKETMQ-34
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shroman/incubator-rocketmq ROCKETMQ-34
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-rocketmq/pull/30.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #30
----
commit 2550ae56b98622344d2079c44a5f577b8d5a5f82
Author: shtykh_roman <rs...@yahoo.com>
Date: 2017-01-06T10:32:09Z
[ROCKETMQ-34] Potential NPE in NettyConnetManageHandler#connect
JIRA issue: https://issues.apache.org/jira/browse/ROCKETMQ-34
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #30: [ROCKETMQ-34] Potential NPE in NettyConnetMana...
Posted by vintagewang <gi...@git.apache.org>.
Github user vintagewang commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/30
review ok.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #30: [ROCKETMQ-34] Potential NPE in NettyConnetMana...
Posted by shroman <gi...@git.apache.org>.
Github user shroman commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/30
@vintagewang `super.connect` can throw NPE when remoteAddress is null, but as far as I see in netty code, it is ok (notification is raised). And the client handles this in `invoke*` methods by `throw new RemotingConnectException(addr);` (for instance, NettyRemotingClient lines 518-519).
To summarize, it looks safe.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #30: [ROCKETMQ-34] Potential NPE in NettyCon...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-rocketmq/pull/30
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #30: [ROCKETMQ-34] Potential NPE in NettyConnetMana...
Posted by vintagewang <gi...@git.apache.org>.
Github user vintagewang commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/30
```
public void connect(ChannelHandlerContext ctx, SocketAddress remoteAddress, SocketAddress localAddress, ChannelPromise promise)
throws Exception {
final String local = localAddress == null ? "UNKNOW" : localAddress.toString();
final String remote = remoteAddress == null ? "UNKNOW" : remoteAddress.toString();
log.info("NETTY CLIENT PIPELINE: CONNECT {} => {}", local, remote);
super.connect(ctx, remoteAddress, localAddress, promise);
if (NettyRemotingClient.this.channelEventListener != null) {
NettyRemotingClient.this.putNettyEvent(new NettyEvent(NettyEventType.CONNECT, remoteAddress.toString(), ctx.channel()));
}
}
```
Hi, @shroman is this ` super.connect` dangerous ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #30: [ROCKETMQ-34] Potential NPE in NettyConnetMana...
Posted by shroman <gi...@git.apache.org>.
Github user shroman commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/30
@vongosling @zhouxinyu Easy fix. Please review.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #30: [ROCKETMQ-34] Potential NPE in NettyConnetMana...
Posted by zhouxinyu <gi...@git.apache.org>.
Github user zhouxinyu commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/30
Easy fix indeed, thanks @shroman , please @vongosling @stevenschew review.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #30: [ROCKETMQ-34] Potential NPE in NettyConnetMana...
Posted by stevenschew <gi...@git.apache.org>.
Github user stevenschew commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/30
review Ok
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---