Uploaded image for project: 'Apache RocketMQ'
  1. Apache RocketMQ
  2. ROCKETMQ-34

Potential NPE in NettyConnetManageHandler#connect

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.0.0-incubating
    • Component/s: None
    • Labels:
      None

      Description

      According to NettyConnetManageHandler#connect logic, remoteAddress can be null, therefore referring to it may be dangerous.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          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 <rshtykh@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


          Show
          githubbot ASF GitHub Bot added a comment - 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 <rshtykh@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
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user shroman commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/30

          @vongosling @zhouxinyu Easy fix. Please review.

          Show
          githubbot ASF GitHub Bot added a comment - Github user shroman commented on the issue: https://github.com/apache/incubator-rocketmq/pull/30 @vongosling @zhouxinyu Easy fix. Please review.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user qinliujie opened a pull request:

          https://github.com/apache/incubator-rocketmq/pull/31

          ROCKETMQ-34 CPU Occupy 100%

          https://issues.apache.org/jira/browse/ROCKETMQ-33

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/qinliujie/incubator-rocketmq master

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/incubator-rocketmq/pull/31.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 #31


          commit 48b3277dccba96b1d12744ce31dfd045aff134a7
          Author: nantian <nantian@juanpi.com>
          Date: 2017-01-07T02:36:49Z

          ROCKETMQ-34 CPU Occupy 100%
          https://issues.apache.org/jira/browse/ROCKETMQ-33


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user qinliujie opened a pull request: https://github.com/apache/incubator-rocketmq/pull/31 ROCKETMQ-34 CPU Occupy 100% https://issues.apache.org/jira/browse/ROCKETMQ-33 You can merge this pull request into a Git repository by running: $ git pull https://github.com/qinliujie/incubator-rocketmq master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-rocketmq/pull/31.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 #31 commit 48b3277dccba96b1d12744ce31dfd045aff134a7 Author: nantian <nantian@juanpi.com> Date: 2017-01-07T02:36:49Z ROCKETMQ-34 CPU Occupy 100% https://issues.apache.org/jira/browse/ROCKETMQ-33
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user qinliujie commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/31

          Sorry,I wrote the wrong issue number,it shoud be ROCKETMQ-33 CPU Occupy 100%

          Show
          githubbot ASF GitHub Bot added a comment - Github user qinliujie commented on the issue: https://github.com/apache/incubator-rocketmq/pull/31 Sorry,I wrote the wrong issue number,it shoud be ROCKETMQ-33 CPU Occupy 100%
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zhouxinyu commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/30

          Easy fix indeed, thanks @shroman , please @vongosling @stevenschew review.

          Show
          githubbot ASF GitHub Bot added a comment - Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/30 Easy fix indeed, thanks @shroman , please @vongosling @stevenschew review.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user stevenschew commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/30

          review Ok

          Show
          githubbot ASF GitHub Bot added a comment - Github user stevenschew commented on the issue: https://github.com/apache/incubator-rocketmq/pull/30 review Ok
          Hide
          githubbot ASF GitHub Bot added a comment -

          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 ?

          Show
          githubbot ASF GitHub Bot added a comment - 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 ?
          Show
          githubbot ASF GitHub Bot added a comment - Github user vintagewang commented on the issue: https://github.com/apache/incubator-rocketmq/pull/30 ! [image] ( https://cloud.githubusercontent.com/assets/1527648/21756759/5a4efd26-d660-11e6-9fde-f6c9ca66fde1.png ) review ok.
          Hide
          githubbot ASF GitHub Bot added a comment -

          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.

          Show
          githubbot ASF GitHub Bot added a comment - 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.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 776911d458d45280de3a4c0f4d6b2bd2ee98d6b2 in incubator-rocketmq's branch refs/heads/master from shroman
          [ https://git-wip-us.apache.org/repos/asf?p=incubator-rocketmq.git;h=776911d ]

          ROCKETMQ-34 Potential NPE in NettyConnetManageHandler#connect, closes apache/incubator-rocketmq#30

          Show
          jira-bot ASF subversion and git services added a comment - Commit 776911d458d45280de3a4c0f4d6b2bd2ee98d6b2 in incubator-rocketmq's branch refs/heads/master from shroman [ https://git-wip-us.apache.org/repos/asf?p=incubator-rocketmq.git;h=776911d ] ROCKETMQ-34 Potential NPE in NettyConnetManageHandler#connect, closes apache/incubator-rocketmq#30
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/incubator-rocketmq/pull/30

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/30

            People

            • Assignee:
              roman_s Roman Shtykh
              Reporter:
              roman_s Roman Shtykh
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development