QPID-3596 : Python tests fails as Java Broker does not validate for reserved exchange names on create/bind

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

3 files changed, 63 insertions, 35 deletions

diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeclareHandler.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeclareHandler.java
index 81e2e5557a..504d9f9411 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeclareHandler.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/handler/ExchangeDeclareHandler.java
@@ -26,6 +26,7 @@ import org.apache.qpid.AMQConnectionException;
 import org.apache.qpid.AMQException;
 import org.apache.qpid.AMQUnknownExchangeType;
 import org.apache.qpid.framing.AMQMethodBody;
+import org.apache.qpid.framing.AMQShortString;
 import org.apache.qpid.framing.ExchangeDeclareBody;
 import org.apache.qpid.framing.MethodRegistry;
 import org.apache.qpid.protocol.AMQConstant;
@@ -37,6 +38,7 @@ import org.apache.qpid.server.protocol.AMQProtocolSession;
 import org.apache.qpid.server.state.AMQStateManager;
 import org.apache.qpid.server.state.StateAwareMethodListener;
 import org.apache.qpid.server.virtualhost.VirtualHost;
+import org.apache.qpid.transport.ExecutionErrorCode;
 
 public class ExchangeDeclareHandler implements StateAwareMethodListener<ExchangeDeclareBody>
 {
@@ -65,26 +67,39 @@ public class ExchangeDeclareHandler implements StateAwareMethodListener<Exchange
             throw body.getChannelNotFoundException(channelId);
         }
 
+        final AMQShortString exchangeName = body.getExchange();
         if (_logger.isDebugEnabled())
         {
-            _logger.debug("Request to declare exchange of type " + body.getType() + " with name " + body.getExchange());
+            _logger.debug("Request to declare exchange of type " + body.getType() + " with name " + exchangeName);
         }
         
         synchronized(exchangeRegistry)
         {
-            Exchange exchange = exchangeRegistry.getExchange(body.getExchange());
+            Exchange exchange = exchangeRegistry.getExchange(exchangeName);
 
             if (exchange == null)
             {
                 if(body.getPassive() && ((body.getType() == null) || body.getType().length() ==0))
                 {
-                    throw body.getChannelException(AMQConstant.NOT_FOUND, "Unknown exchange: " + body.getExchange());
+                    throw body.getChannelException(AMQConstant.NOT_FOUND, "Unknown exchange: " + exchangeName);
+                }
+                else if(exchangeName.startsWith("amq."))
+                {
+                    throw body.getConnectionException(AMQConstant.NOT_ALLOWED,
+                              "Attempt to declare exchange: " + exchangeName +
+                              " which begins with reserved prefix 'amq.'.");
+                }
+                else if(exchangeName.startsWith("qpid."))
+                {
+                    throw body.getConnectionException(AMQConstant.NOT_ALLOWED,
+                                                      "Attempt to declare exchange: " + exchangeName +
+                                                      " which begins with reserved prefix 'qpid.'.");
                 }
                 else
                 {
                     try
                     {
-                        exchange = exchangeFactory.createExchange(body.getExchange() == null ? null : body.getExchange().intern(),
+                        exchange = exchangeFactory.createExchange(exchangeName == null ? null : exchangeName.intern(),
                                                                   body.getType() == null ? null : body.getType().intern(),
                                                                   body.getDurable(),
                                                                   body.getPassive(), body.getTicket());
@@ -97,14 +112,15 @@ public class ExchangeDeclareHandler implements StateAwareMethodListener<Exchange
                     }
                     catch(AMQUnknownExchangeType e)
                     {
-                        throw body.getConnectionException(AMQConstant.COMMAND_INVALID, "Unknown exchange: " + body.getExchange(),e);
+                        throw body.getConnectionException(AMQConstant.COMMAND_INVALID, "Unknown exchange: " + exchangeName,e);
                     }
                 }
             }
             else if (!exchange.getTypeShortString().equals(body.getType()) && !((body.getType() == null || body.getType().length() ==0) && body.getPassive()))
             {
 
-                throw new AMQConnectionException(AMQConstant.NOT_ALLOWED, "Attempt to redeclare exchange: " + body.getExchange() + " of type " + exchange.getTypeShortString() + " to " + body.getType() +".",body.getClazz(), body.getMethod(),body.getMajor(),body.getMinor(),null);
+                throw new AMQConnectionException(AMQConstant.NOT_ALLOWED, "Attempt to redeclare exchange: " +
+                                                                          exchangeName + " of type " + exchange.getTypeShortString() + " to " + body.getType() +".",body.getClazz(), body.getMethod(),body.getMajor(),body.getMinor(),null);
             }
         }
         if(!body.getNowait())
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSessionDelegate.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSessionDelegate.java
index 8e4fb3635b..2ee6f31af7 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSessionDelegate.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSessionDelegate.java
@@ -474,41 +474,56 @@ public class ServerSessionDelegate extends SessionDelegate
         {
             if (exchange == null)
             {
-                ExchangeRegistry exchangeRegistry = getExchangeRegistry(session);
-                ExchangeFactory exchangeFactory = virtualHost.getExchangeFactory();
+                if(exchangeName.startsWith("amq."))
+                {
+                    exception(session, method, ExecutionErrorCode.NOT_ALLOWED,
+                              "Attempt to declare exchange: " + exchangeName +
+                              " which begins with reserved prefix 'amq.'.");
+                }
+                else if(exchangeName.startsWith("qpid."))
+                {
+                    exception(session, method, ExecutionErrorCode.NOT_ALLOWED,
+                              "Attempt to declare exchange: " + exchangeName +
+                              " which begins with reserved prefix 'qpid.'.");
+                }
+                else
+                {
+                    ExchangeRegistry exchangeRegistry = getExchangeRegistry(session);
+                    ExchangeFactory exchangeFactory = virtualHost.getExchangeFactory();
 
 
 
-                try
-                {
+                    try
+                    {
+
+                        exchange = exchangeFactory.createExchange(method.getExchange(),
+                                                                  method.getType(),
+                                                                  method.getDurable(),
+                                                                  method.getAutoDelete());
+
+                        String alternateExchangeName = method.getAlternateExchange();
+                        if(alternateExchangeName != null && alternateExchangeName.length() != 0)
+                        {
+                            Exchange alternate = getExchange(session, alternateExchangeName);
+                            exchange.setAlternateExchange(alternate);
+                        }
 
-                    exchange = exchangeFactory.createExchange(method.getExchange(),
-                                                              method.getType(),
-                                                              method.getDurable(),
-                                                              method.getAutoDelete());
+                        if (exchange.isDurable())
+                        {
+                            DurableConfigurationStore store = virtualHost.getDurableConfigurationStore();
+                            store.createExchange(exchange);
+                        }
 
-                    String alternateExchangeName = method.getAlternateExchange();
-                    if(alternateExchangeName != null && alternateExchangeName.length() != 0)
+                        exchangeRegistry.registerExchange(exchange);
+                    }
+                    catch(AMQUnknownExchangeType e)
                     {
-                        Exchange alternate = getExchange(session, alternateExchangeName);
-                        exchange.setAlternateExchange(alternate);
+                        exception(session, method, ExecutionErrorCode.NOT_FOUND, "Unknown Exchange Type: " + method.getType());
                     }
-
-                    if (exchange.isDurable())
+                    catch (AMQException e)
                     {
-                        DurableConfigurationStore store = virtualHost.getDurableConfigurationStore();
-                        store.createExchange(exchange);
+                        exception(session, method, e, "Cannot declare exchange '" + exchangeName);
                     }
-
-                    exchangeRegistry.registerExchange(exchange);
-                }
-                catch(AMQUnknownExchangeType e)
-                {
-                    exception(session, method, ExecutionErrorCode.NOT_FOUND, "Unknown Exchange Type: " + method.getType());
-                }
-                catch (AMQException e)
-                {
-                    exception(session, method, e, "Cannot declare exchange '" + exchangeName);
                 }
             }
             else
diff --git a/qpid/java/test-profiles/python_tests/Java010PythonExcludes b/qpid/java/test-profiles/python_tests/Java010PythonExcludes
index 6e05a21d91..b25a8e507d 100644
--- a/qpid/java/test-profiles/python_tests/Java010PythonExcludes
+++ b/qpid/java/test-profiles/python_tests/Java010PythonExcludes
@@ -77,9 +77,6 @@ qpid_tests.broker_0_10.alternate_exchange.AlternateExchangeTests.test_reject_no_
 qpid_tests.broker_0_10.alternate_exchange.AlternateExchangeTests.test_add_alternate_to_exchange
 qpid_tests.broker_0_10.new_api.GeneralTests.test_qpid_3481_acquired_to_alt_exchange
 
-#QPID-3596 Broker does not validate for reserved exchange names on create/bind.
-qpid_tests.broker_0_10.exchange.DeclareMethodExchangeFieldReservedRuleTests.*
-
 #QPID-3597 Headers exchange issues
 qpid_tests.broker_0_10.exchange.HeadersExchangeTests.*
 qpid_tests.broker_0_10.queue.QueueTests.test_unbind_headers