You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by gaohoward <gi...@git.apache.org> on 2017/03/27 03:55:53 UTC

[GitHub] activemq-artemis pull request #1134: ARTEMIS-1071 Invalid Type exception han...

GitHub user gaohoward opened a pull request:

    https://github.com/apache/activemq-artemis/pull/1134

    ARTEMIS-1071 Invalid Type exception handling improvements

    If broker fails to decode any packets from buffer, it should
    treat it as a critical bug and disconnect immediately.
    Currently broker only logs an error message.

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

    $ git pull https://github.com/gaohoward/activemq-artemis master_invalid_packet

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

    https://github.com/apache/activemq-artemis/pull/1134.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 #1134
    
----
commit 02f84508eb6e8ce94a3689b49d65b5f7cede640c
Author: Howard Gao <ho...@gmail.com>
Date:   2017-03-27T03:53:47Z

    ARTEMIS-1071 Invalid Type exception handling improvements
    
    If broker fails to decode any packets from buffer, it should
    treat it as a critical bug and disconnect immediately.
    Currently broker only logs an error message.

----


---
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] activemq-artemis pull request #1134: ARTEMIS-1071 Invalid Type exception han...

Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1134#discussion_r108572088
  
    --- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/impl/RemotingServiceImpl.java ---
    @@ -626,7 +626,12 @@ public void bufferReceived(final Object connectionID, final ActiveMQBuffer buffe
              ConnectionEntry conn = connections.get(connectionID);
     
              if (conn != null) {
    -            conn.connection.bufferReceived(connectionID, buffer);
    +            try {
    +               conn.connection.bufferReceived(connectionID, buffer);
    +            } catch (RuntimeException e) {
    +               ActiveMQServerLogger.LOGGER.warn("Failed to decode buffer, disconnect immediately.", e);
    --- End diff --
    
    @jbertram fair point. I'll change that. thx.


---
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] activemq-artemis pull request #1134: ARTEMIS-1071 Invalid Type exception han...

Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1134#discussion_r108575576
  
    --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/util/JMSTestBase.java ---
    @@ -197,21 +198,33 @@ protected void registerConnectionFactory() throws Exception {
           List<TransportConfiguration> connectorConfigs = new ArrayList<>();
           connectorConfigs.add(new TransportConfiguration(INVM_CONNECTOR_FACTORY));
     
    +      List<TransportConfiguration> connectorConfigs1 = new ArrayList<>();
    +      connectorConfigs1.add(new TransportConfiguration(NETTY_CONNECTOR_FACTORY));
    +
           createCF(connectorConfigs, "/cf");
    +      createCF("NettyCF", connectorConfigs1, "/nettyCf");
     
           cf = (ConnectionFactory) namingContext.lookup("/cf");
    +      nettyCf = (ConnectionFactory)namingContext.lookup("/nettyCf");
    +   }
    +
    +   protected void createCF(final List<TransportConfiguration> connectorConfigs,
    +                           final String... jndiBindings) throws Exception {
    +      createCF(name.getMethodName(), connectorConfigs, jndiBindings);
        }
     
    +
        /**
         * @param connectorConfigs
         * @param jndiBindings
         * @throws Exception
         */
    -   protected void createCF(final List<TransportConfiguration> connectorConfigs,
    +   protected void createCF(final String cfName,
    +                           final List<TransportConfiguration> connectorConfigs,
    --- End diff --
    
    @yourcaptain will so the correction. thx.


---
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] activemq-artemis pull request #1134: ARTEMIS-1071 Invalid Type exception han...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/activemq-artemis/pull/1134


---
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] activemq-artemis pull request #1134: ARTEMIS-1071 Invalid Type exception han...

Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1134#discussion_r108462405
  
    --- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/impl/RemotingServiceImpl.java ---
    @@ -626,7 +626,12 @@ public void bufferReceived(final Object connectionID, final ActiveMQBuffer buffe
              ConnectionEntry conn = connections.get(connectionID);
     
              if (conn != null) {
    -            conn.connection.bufferReceived(connectionID, buffer);
    +            try {
    +               conn.connection.bufferReceived(connectionID, buffer);
    +            } catch (RuntimeException e) {
    +               ActiveMQServerLogger.LOGGER.warn("Failed to decode buffer, disconnect immediately.", e);
    --- End diff --
    
    This should be done through a custom method on ActiveMQServerLogger.LOGGER rather than through the generic warn() method.


---
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] activemq-artemis pull request #1134: ARTEMIS-1071 Invalid Type exception han...

Posted by yourcaptain <gi...@git.apache.org>.
Github user yourcaptain commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1134#discussion_r108573779
  
    --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/util/JMSTestBase.java ---
    @@ -197,21 +198,33 @@ protected void registerConnectionFactory() throws Exception {
           List<TransportConfiguration> connectorConfigs = new ArrayList<>();
           connectorConfigs.add(new TransportConfiguration(INVM_CONNECTOR_FACTORY));
     
    +      List<TransportConfiguration> connectorConfigs1 = new ArrayList<>();
    +      connectorConfigs1.add(new TransportConfiguration(NETTY_CONNECTOR_FACTORY));
    +
           createCF(connectorConfigs, "/cf");
    +      createCF("NettyCF", connectorConfigs1, "/nettyCf");
     
           cf = (ConnectionFactory) namingContext.lookup("/cf");
    +      nettyCf = (ConnectionFactory)namingContext.lookup("/nettyCf");
    +   }
    +
    +   protected void createCF(final List<TransportConfiguration> connectorConfigs,
    +                           final String... jndiBindings) throws Exception {
    +      createCF(name.getMethodName(), connectorConfigs, jndiBindings);
        }
     
    +
        /**
         * @param connectorConfigs
         * @param jndiBindings
         * @throws Exception
         */
    -   protected void createCF(final List<TransportConfiguration> connectorConfigs,
    +   protected void createCF(final String cfName,
    +                           final List<TransportConfiguration> connectorConfigs,
    --- End diff --
    
    cfName should be added to comment


---
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] activemq-artemis pull request #1134: ARTEMIS-1071 Invalid Type exception han...

Posted by yourcaptain <gi...@git.apache.org>.
Github user yourcaptain commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1134#discussion_r108573033
  
    --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/util/JMSTestBase.java ---
    @@ -197,21 +198,33 @@ protected void registerConnectionFactory() throws Exception {
           List<TransportConfiguration> connectorConfigs = new ArrayList<>();
           connectorConfigs.add(new TransportConfiguration(INVM_CONNECTOR_FACTORY));
     
    +      List<TransportConfiguration> connectorConfigs1 = new ArrayList<>();
    +      connectorConfigs1.add(new TransportConfiguration(NETTY_CONNECTOR_FACTORY));
    +
           createCF(connectorConfigs, "/cf");
    +      createCF("NettyCF", connectorConfigs1, "/nettyCf");
     
           cf = (ConnectionFactory) namingContext.lookup("/cf");
    +      nettyCf = (ConnectionFactory)namingContext.lookup("/nettyCf");
    +   }
    +
    +   protected void createCF(final List<TransportConfiguration> connectorConfigs,
    +                           final String... jndiBindings) throws Exception {
    +      createCF(name.getMethodName(), connectorConfigs, jndiBindings);
        }
     
    +
        /**
         * @param connectorConfigs
    --- End diff --
    
    those comments should match the actual parameters


---
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.
---