Details

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

      Description

      When writing unit tests for rocketmq-remoting, I found that RocketMQSerializable misused the charset for RemotingSerializable.

      Go into details. The class RemotingCommand has two formats to serialize its header: JSON format (source code RemotingSerializable.java) and ROCKETMQ (source code RocketMQSerializable.java) format, respectively. Both formats have their own encode method, decode method and charset. But methods in RocketMQSerializable.java use the charset for RemotingSerializable.java.

      Although two charsets are both UTF-8 thus no problem occurs so far, but it's better to modify to make the code commonsensible

        Issue Links

          Activity

          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 457bb61f0489d58c810330b40c62ccb7e75b602b in incubator-rocketmq's branch refs/heads/ROCKETMQ-57 from Kailai Shao
          [ https://git-wip-us.apache.org/repos/asf?p=incubator-rocketmq.git;h=457bb61 ]

          ROCKETMQ-59 Change Charset usages in RocketMQSerializable to RocketMQSerializable#CHARSET_UTF8, closes apache/incubator-rocketmq#43

          Show
          jira-bot ASF subversion and git services added a comment - Commit 457bb61f0489d58c810330b40c62ccb7e75b602b in incubator-rocketmq's branch refs/heads/ ROCKETMQ-57 from Kailai Shao [ https://git-wip-us.apache.org/repos/asf?p=incubator-rocketmq.git;h=457bb61 ] ROCKETMQ-59 Change Charset usages in RocketMQSerializable to RocketMQSerializable#CHARSET_UTF8, closes apache/incubator-rocketmq#43
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/43
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 457bb61f0489d58c810330b40c62ccb7e75b602b in incubator-rocketmq's branch refs/heads/master from Kailai Shao
          [ https://git-wip-us.apache.org/repos/asf?p=incubator-rocketmq.git;h=457bb61 ]

          ROCKETMQ-59 Change Charset usages in RocketMQSerializable to RocketMQSerializable#CHARSET_UTF8, closes apache/incubator-rocketmq#43

          Show
          jira-bot ASF subversion and git services added a comment - Commit 457bb61f0489d58c810330b40c62ccb7e75b602b in incubator-rocketmq's branch refs/heads/master from Kailai Shao [ https://git-wip-us.apache.org/repos/asf?p=incubator-rocketmq.git;h=457bb61 ] ROCKETMQ-59 Change Charset usages in RocketMQSerializable to RocketMQSerializable#CHARSET_UTF8, closes apache/incubator-rocketmq#43
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vongosling commented on the issue:

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

          alright, thanks @iskl

          Show
          githubbot ASF GitHub Bot added a comment - Github user vongosling commented on the issue: https://github.com/apache/incubator-rocketmq/pull/43 alright, thanks @iskl
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user iskl commented on the issue:

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

          @zhouxinyu That's really a better solution! I've change the access modifiers to `private` for borh.

          Since the CHARSET is used in `static` methods, it could not be modified to non-static.

          Please help to review~

          Show
          githubbot ASF GitHub Bot added a comment - Github user iskl commented on the issue: https://github.com/apache/incubator-rocketmq/pull/43 @zhouxinyu That's really a better solution! I've change the access modifiers to `private` for borh. Since the CHARSET is used in `static` methods, it could not be modified to non-static. Please help to review~
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zhouxinyu commented on the issue:

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

          @iskl It could be better if we narrow down `CHARSET_UTF8 ` access modifier to `private` and without `static`, for both `RocketMQSerializable` and `RemotingSerializable`.

          Show
          githubbot ASF GitHub Bot added a comment - Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/43 @iskl It could be better if we narrow down `CHARSET_UTF8 ` access modifier to `private` and without `static`, for both `RocketMQSerializable` and `RemotingSerializable`.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lizhanhui commented on the issue:

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

          Minor changes, appears OK.

          Show
          githubbot ASF GitHub Bot added a comment - Github user lizhanhui commented on the issue: https://github.com/apache/incubator-rocketmq/pull/43 Minor changes, appears OK.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user iskl opened a pull request:

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

          ROCKETMQ-59 Change Charset usages in RocketMQSerializable to Rocket…

          The PR is to resolve [issue-59](https://issues.apache.org/jira/browse/ROCKETMQ-59)

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

          $ git pull https://github.com/iskl/incubator-rocketmq ROCKETMQ-59

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

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


          commit 5358cc2de6bb32a0aa4afed18c342f618f000b75
          Author: Kailai Shao <shao@kailai.me>
          Date: 2017-01-16T03:57:19Z

          ROCKETMQ-59 Change Charset usages in RocketMQSerializable to RocketMQSerializable#CHARSET_UTF8


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user iskl opened a pull request: https://github.com/apache/incubator-rocketmq/pull/43 ROCKETMQ-59 Change Charset usages in RocketMQSerializable to Rocket… The PR is to resolve [issue-59] ( https://issues.apache.org/jira/browse/ROCKETMQ-59 ) You can merge this pull request into a Git repository by running: $ git pull https://github.com/iskl/incubator-rocketmq ROCKETMQ-59 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-rocketmq/pull/43.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 #43 commit 5358cc2de6bb32a0aa4afed18c342f618f000b75 Author: Kailai Shao <shao@kailai.me> Date: 2017-01-16T03:57:19Z ROCKETMQ-59 Change Charset usages in RocketMQSerializable to RocketMQSerializable#CHARSET_UTF8

            People

            • Assignee:
              vongosling vongosling
              Reporter:
              kailai Kailai Shao
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development