QPID-5825: Broker tries to downgrade to the wrong amqp protocol version

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1603786 13f79535-47bb-0310-9956-ffa450edef68

5 files changed, 252 insertions, 1 deletions

diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/plugin/AMQPProtocolVersionWrapper.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/plugin/AMQPProtocolVersionWrapper.java
new file mode 100644
index 0000000000..9947a8fac0
--- /dev/null
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/plugin/AMQPProtocolVersionWrapper.java
@@ -0,0 +1,131 @@
+package org.apache.qpid.server.plugin;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.qpid.server.model.Protocol;
+
+public class AMQPProtocolVersionWrapper
+{
+    private static final char DELIMITER = '_';
+
+    private int major;
+    private int minor;
+    private int patch;
+
+    public AMQPProtocolVersionWrapper(Protocol amqpProtocol)
+    {
+        if (!amqpProtocol.isAMQP())
+        {
+            throw new IllegalArgumentException("Protocol must be of type " + Protocol.ProtocolType.AMQP);
+        }
+
+        final String[] parts = StringUtils.split(amqpProtocol.name(), DELIMITER);
+        for (int i = 0; i < parts.length; i++)
+        {
+            switch (i)
+            {
+                case 1: this.major = Integer.parseInt(parts[i]);
+                    break;
+                case 2: this.minor = Integer.parseInt(parts[i]);
+                    break;
+                case 3: this.patch = Integer.parseInt(parts[i]);
+                    break;
+            }
+        }
+    }
+
+    public AMQPProtocolVersionWrapper(int major, int minor, int patch)
+    {
+        this.major = major;
+        this.minor = minor;
+        this.patch = patch;
+    }
+
+    public int getMajor()
+    {
+        return major;
+    }
+
+    public void setMajor(int major)
+    {
+        this.major = major;
+    }
+
+    public int getMinor()
+    {
+        return minor;
+    }
+
+    public void setMinor(int minor)
+    {
+        this.minor = minor;
+    }
+
+    public int getPatch()
+    {
+        return patch;
+    }
+
+    public void setPatch(int patch)
+    {
+        this.patch = patch;
+    }
+
+    public Protocol getProtocol()
+    {
+        return Protocol.valueOf(this.toString());
+    }
+
+    @Override
+    public boolean equals(Object o)
+    {
+        if (this == o)
+        {
+            return true;
+        }
+        if (!(o instanceof AMQPProtocolVersionWrapper))
+        {
+            return false;
+        }
+
+        final AMQPProtocolVersionWrapper number = (AMQPProtocolVersionWrapper) o;
+
+        if (this.major != number.major)
+        {
+            return false;
+        }
+        else if (this.minor != number.minor)
+        {
+            return false;
+        }
+        else if (this.patch != number.patch)
+        {
+            return false;
+        }
+        else
+        {
+            return true;
+        }
+    }
+
+    @Override
+    public int hashCode()
+    {
+        int result = major;
+        result = 31 * result + minor;
+        result = 31 * result + patch;
+        return result;
+    }
+
+    @Override
+    public String toString()
+    {
+        final StringBuilder sb = new StringBuilder(Protocol.ProtocolType.AMQP.name()).append(DELIMITER)
+                                     .append(major).append(DELIMITER)
+                                     .append(minor);
+        if (patch != 0)
+        {
+            sb.append(DELIMITER).append(patch);
+        }
+        return sb.toString();
+    }
+}
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/plugin/ProtocolEngineCreatorComparator.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/plugin/ProtocolEngineCreatorComparator.java
new file mode 100644
index 0000000000..3619d7000e
--- /dev/null
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/plugin/ProtocolEngineCreatorComparator.java
@@ -0,0 +1,32 @@
+package org.apache.qpid.server.plugin;
+
+import java.util.Comparator;
+
+public class ProtocolEngineCreatorComparator implements Comparator<ProtocolEngineCreator>
+{
+    @Override
+    public int compare(ProtocolEngineCreator pec1, ProtocolEngineCreator pec2)
+    {
+        final AMQPProtocolVersionWrapper v1 = new AMQPProtocolVersionWrapper(pec1.getVersion());
+        final AMQPProtocolVersionWrapper v2 = new AMQPProtocolVersionWrapper(pec2.getVersion());
+
+        if (v1.getMajor() != v2.getMajor())
+        {
+            return v1.getMajor() - v2.getMajor();
+        }
+        else if (v1.getMinor() != v2.getMinor())
+        {
+            return v1.getMinor() - v2.getMinor();
+        }
+        else if (v1.getPatch() != v2.getPatch())
+        {
+            return v1.getPatch() - v2.getPatch();
+        }
+        else
+        {
+            return 0;
+        }
+    }
+
+
+}
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/protocol/MultiVersionProtocolEngineFactory.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/protocol/MultiVersionProtocolEngineFactory.java
index fcc96b8b22..b1c49c6fe5 100755
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/protocol/MultiVersionProtocolEngineFactory.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/protocol/MultiVersionProtocolEngineFactory.java
@@ -21,6 +21,7 @@
 package org.apache.qpid.server.protocol;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicLong;
@@ -34,6 +35,7 @@ import org.apache.qpid.server.model.Port;
 import org.apache.qpid.server.model.Protocol;
 import org.apache.qpid.server.model.Transport;
 import org.apache.qpid.server.plugin.ProtocolEngineCreator;
+import org.apache.qpid.server.plugin.ProtocolEngineCreatorComparator;
 import org.apache.qpid.server.plugin.QpidServiceLoader;
 
 public class MultiVersionProtocolEngineFactory implements ProtocolEngineFactory
@@ -69,11 +71,12 @@ public class MultiVersionProtocolEngineFactory implements ProtocolEngineFactory
         _sslContext = sslContext;
         _supported = supportedVersions;
         _defaultSupportedReply = defaultSupportedReply;
-        List<ProtocolEngineCreator> creators = new ArrayList<ProtocolEngineCreator>();
+        final List<ProtocolEngineCreator> creators = new ArrayList<ProtocolEngineCreator>();
         for(ProtocolEngineCreator c : new QpidServiceLoader<ProtocolEngineCreator>().instancesOf(ProtocolEngineCreator.class))
         {
             creators.add(c);
         }
+        Collections.sort(creators, new ProtocolEngineCreatorComparator());
         _creators = creators.toArray(new ProtocolEngineCreator[creators.size()]);
         _wantClientAuth = wantClientAuth;
         _needClientAuth = needClientAuth;
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/plugin/AMQPProtocolVersionWrapperTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/plugin/AMQPProtocolVersionWrapperTest.java
new file mode 100644
index 0000000000..4d57c6c7e7
--- /dev/null
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/plugin/AMQPProtocolVersionWrapperTest.java
@@ -0,0 +1,43 @@
+package org.apache.qpid.server.plugin;
+
+import org.apache.qpid.server.model.Protocol;
+import org.apache.qpid.test.utils.QpidTestCase;
+
+public class AMQPProtocolVersionWrapperTest extends QpidTestCase
+{
+    public void testAMQPProtocolVersionWrapper() throws Exception
+    {
+        assertEquals(new AMQPProtocolVersionWrapper(0,8,0), new AMQPProtocolVersionWrapper(Protocol.AMQP_0_8));
+        assertEquals(new AMQPProtocolVersionWrapper(0,9,0), new AMQPProtocolVersionWrapper(Protocol.AMQP_0_9));
+        assertEquals(new AMQPProtocolVersionWrapper(0,9,1), new AMQPProtocolVersionWrapper(Protocol.AMQP_0_9_1));
+        assertEquals(new AMQPProtocolVersionWrapper(0,10,0),new AMQPProtocolVersionWrapper(Protocol.AMQP_0_10));
+        assertEquals(new AMQPProtocolVersionWrapper(1,0,0), new AMQPProtocolVersionWrapper(Protocol.AMQP_1_0));
+
+        assertNotSame(new AMQPProtocolVersionWrapper(0, 9, 1), new AMQPProtocolVersionWrapper(Protocol.AMQP_0_9));
+        assertNotSame(new AMQPProtocolVersionWrapper(0, 10, 0), new AMQPProtocolVersionWrapper(Protocol.AMQP_1_0));
+    }
+
+    public void testAMQPProtocolVersionWrapperGetProtocol() throws Exception
+    {
+        for (final Protocol protocol : Protocol.values())
+        {
+            if (protocol.isAMQP())
+            {
+                assertEquals(protocol, new AMQPProtocolVersionWrapper(protocol).getProtocol());
+            }
+        }
+    }
+
+    public void testWrappingNonAMQPProtocol() throws Exception
+    {
+        try
+        {
+            new AMQPProtocolVersionWrapper(Protocol.HTTP);
+            fail("IllegalArgumentException exception expected when Protocol is not AMQP based");
+        }
+        catch (IllegalArgumentException iae)
+        {
+            // pass
+        }
+    }
+}
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/plugin/ProtocolEngineCreatorComparatorTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/plugin/ProtocolEngineCreatorComparatorTest.java
new file mode 100644
index 0000000000..6eceab13c4
--- /dev/null
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/plugin/ProtocolEngineCreatorComparatorTest.java
@@ -0,0 +1,42 @@
+package org.apache.qpid.server.plugin;
+
+import org.apache.qpid.server.model.Protocol;
+import org.apache.qpid.test.utils.QpidTestCase;
+import org.mockito.Mockito;
+
+public class ProtocolEngineCreatorComparatorTest extends QpidTestCase
+{
+    public void testProtocolEngineCreatorComparator() throws Exception
+    {
+        final ProtocolEngineCreatorComparator comparator = new ProtocolEngineCreatorComparator();
+
+        final ProtocolEngineCreator amqp_0_8 = createAMQPProtocolEngineCreatorMock(Protocol.AMQP_0_8);
+        final ProtocolEngineCreator amqp_0_9 = createAMQPProtocolEngineCreatorMock(Protocol.AMQP_0_9);
+        final ProtocolEngineCreator amqp_0_9_1 = createAMQPProtocolEngineCreatorMock(Protocol.AMQP_0_9_1);
+        final ProtocolEngineCreator amqp_0_10 = createAMQPProtocolEngineCreatorMock(Protocol.AMQP_0_10);
+        final ProtocolEngineCreator amqp_1_0 = createAMQPProtocolEngineCreatorMock(Protocol.AMQP_1_0);
+
+        assertTrue(comparator.compare(amqp_0_8,amqp_0_9) < 0);
+        assertTrue(comparator.compare(amqp_0_9,amqp_0_9_1) < 0);
+        assertTrue(comparator.compare(amqp_0_9_1,amqp_0_10) < 0);
+        assertTrue(comparator.compare(amqp_0_10,amqp_1_0) < 0);
+
+        assertTrue(comparator.compare(amqp_0_9,amqp_0_8) > 0);
+        assertTrue(comparator.compare(amqp_0_9_1,amqp_0_9) > 0);
+        assertTrue(comparator.compare(amqp_0_10,amqp_0_9_1) > 0);
+        assertTrue(comparator.compare(amqp_1_0,amqp_0_10) > 0);
+
+        assertTrue(comparator.compare(amqp_0_8,amqp_0_8) == 0);
+        assertTrue(comparator.compare(amqp_0_9,amqp_0_9) == 0);
+        assertTrue(comparator.compare(amqp_0_9_1,amqp_0_9_1) == 0);
+        assertTrue(comparator.compare(amqp_0_10,amqp_0_10) == 0);
+        assertTrue(comparator.compare(amqp_1_0,amqp_1_0) == 0);
+    }
+
+    private ProtocolEngineCreator createAMQPProtocolEngineCreatorMock(Protocol protocol)
+    {
+        final ProtocolEngineCreator protocolMock = Mockito.mock(ProtocolEngineCreator.class);
+        Mockito.when(protocolMock.getVersion()).thenReturn(protocol);
+        return protocolMock;
+    }
+}