QPID-4863: Validate plugin attribute changes and throw UnsupportedOperationException where attribute changes are not supported

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

7 files changed, 141 insertions, 0 deletions

diff --git a/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagement.java b/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagement.java
index 43328c16bf..d41b505d23 100644
--- a/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagement.java
+++ b/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagement.java
@@ -436,4 +436,33 @@ public class HttpManagement extends AbstractPluginAdapter implements HttpManagem
         return getBroker().getSubjectCreator(localAddress);
     }
 
+    @Override
+    protected void changeAttributes(Map<String, Object> attributes)
+    {
+        Map<String, Object> convertedAttributes = MapValueConverter.convert(attributes, ATTRIBUTE_TYPES);
+        validateAttributes(convertedAttributes);
+
+        super.changeAttributes(convertedAttributes);
+    }
+
+    private void validateAttributes(Map<String, Object> convertedAttributes)
+    {
+        if(convertedAttributes.containsKey(HttpManagement.NAME))
+        {
+            String newName = (String) convertedAttributes.get(HttpManagement.NAME);
+            if(!getName().equals(newName))
+            {
+                throw new IllegalConfigurationException("Changing the name of http management plugin is not allowed");
+            }
+        }
+        if (convertedAttributes.containsKey(TIME_OUT))
+        {
+            Number value = (Number) convertedAttributes.get(TIME_OUT);
+            if (value == null || value.longValue() < 0)
+            {
+                throw new IllegalConfigurationException("Only positive integer value can be specified for the session time out attribute");
+            }
+        }
+    }
+
 }
diff --git a/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagement.java b/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagement.java
index cebfb8d9d9..f58ec2bfbd 100644
--- a/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagement.java
+++ b/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagement.java
@@ -33,6 +33,7 @@ import java.util.UUID;
 import javax.management.JMException;
 
 import org.apache.log4j.Logger;
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.jmx.mbeans.LoggingManagementMBean;
 import org.apache.qpid.server.jmx.mbeans.UserManagementMBean;
 import org.apache.qpid.server.jmx.mbeans.ServerInformationMBean;
@@ -362,4 +363,24 @@ public class JMXManagement extends AbstractPluginAdapter implements Configuratio
         return AVAILABLE_ATTRIBUTES;
     }
 
+    @Override
+    protected void changeAttributes(Map<String, Object> attributes)
+    {
+        Map<String, Object> convertedAttributes = MapValueConverter.convert(attributes, ATTRIBUTE_TYPES);
+        validateAttributes(convertedAttributes);
+
+        super.changeAttributes(convertedAttributes);
+    }
+
+    private void validateAttributes(Map<String, Object> convertedAttributes)
+    {
+        if(convertedAttributes.containsKey(JMXManagement.NAME))
+        {
+            String newName = (String) convertedAttributes.get(JMXManagement.NAME);
+            if(!getName().equals(newName))
+            {
+                throw new IllegalConfigurationException("Changing the name of jmx management plugin is not allowed");
+            }
+        }
+    }
 }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/ConnectionAdapter.java b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/ConnectionAdapter.java
index 54a3b634e5..aaa6f460d2 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/ConnectionAdapter.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/ConnectionAdapter.java
@@ -305,4 +305,18 @@ final class ConnectionAdapter extends AbstractAdapter implements Connection
         // TODO: add state management
         return false;
     }
+
+    @Override
+    public Object setAttribute(final String name, final Object expected, final Object desired) throws IllegalStateException,
+            AccessControlException, IllegalArgumentException
+    {
+        throw new UnsupportedOperationException("Changing attributes on connection is not supported.");
+    }
+
+    @Override
+    public void setAttributes(final Map<String, Object> attributes) throws IllegalStateException, AccessControlException,
+            IllegalArgumentException
+    {
+        throw new UnsupportedOperationException("Changing attributes on connection is not supported.");
+    }
 }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/ConsumerAdapter.java b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/ConsumerAdapter.java
index 4633605256..696c59783e 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/ConsumerAdapter.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/ConsumerAdapter.java
@@ -225,4 +225,18 @@ public class ConsumerAdapter extends AbstractAdapter implements Consumer
         // TODO : Add state management
         return false;
     }
+
+    @Override
+    public Object setAttribute(final String name, final Object expected, final Object desired) throws IllegalStateException,
+            AccessControlException, IllegalArgumentException
+    {
+        throw new UnsupportedOperationException("Changing attributes on consumer is not supported.");
+    }
+
+    @Override
+    public void setAttributes(final Map<String, Object> attributes) throws IllegalStateException, AccessControlException,
+            IllegalArgumentException
+    {
+        throw new UnsupportedOperationException("Changing attributes on consumer is not supported.");
+    }
 }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java
index 3bf62dc96b..9323606c83 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java
@@ -544,6 +544,20 @@ public class GroupProviderAdapter extends AbstractAdapter implements
             return false;
         }
 
+        @Override
+        public Object setAttribute(final String name, final Object expected, final Object desired) throws IllegalStateException,
+                AccessControlException, IllegalArgumentException
+        {
+            throw new UnsupportedOperationException("Changing attributes on group is not supported.");
+        }
+
+        @Override
+        public void setAttributes(final Map<String, Object> attributes) throws IllegalStateException, AccessControlException,
+                IllegalArgumentException
+        {
+            throw new UnsupportedOperationException("Changing attributes on group is not supported.");
+        }
+
         private class GroupMemberAdapter extends AbstractAdapter implements
                 GroupMember
         {
@@ -676,6 +690,19 @@ public class GroupProviderAdapter extends AbstractAdapter implements
                 return false;
             }
 
+            @Override
+            public Object setAttribute(final String name, final Object expected, final Object desired) throws IllegalStateException,
+                    AccessControlException, IllegalArgumentException
+            {
+                throw new UnsupportedOperationException("Changing attributes on group member is not supported.");
+            }
+
+            @Override
+            public void setAttributes(final Map<String, Object> attributes) throws IllegalStateException, AccessControlException,
+                    IllegalArgumentException
+            {
+                throw new UnsupportedOperationException("Changing attributes on group member is not supported.");
+            }
         }
     }
 
diff --git a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/SessionAdapter.java b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/SessionAdapter.java
index 550e8cecd6..31ce7e56fd 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/SessionAdapter.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/SessionAdapter.java
@@ -276,4 +276,18 @@ final class SessionAdapter extends AbstractAdapter implements Session
         // TODO : add state management
         return false;
     }
+
+    @Override
+    public Object setAttribute(final String name, final Object expected, final Object desired) throws IllegalStateException,
+            AccessControlException, IllegalArgumentException
+    {
+        throw new UnsupportedOperationException("Changing attributes on session is not supported.");
+    }
+
+    @Override
+    public void setAttributes(final Map<String, Object> attributes) throws IllegalStateException, AccessControlException,
+            IllegalArgumentException
+    {
+        throw new UnsupportedOperationException("Changing attributes on session is not supported.");
+    }
 }
diff --git a/java/systests/src/main/java/org/apache/qpid/systest/rest/HttpManagementRestTest.java b/java/systests/src/main/java/org/apache/qpid/systest/rest/HttpManagementRestTest.java
index 3a0b834cc2..ab38d77b6c 100644
--- a/java/systests/src/main/java/org/apache/qpid/systest/rest/HttpManagementRestTest.java
+++ b/java/systests/src/main/java/org/apache/qpid/systest/rest/HttpManagementRestTest.java
@@ -66,4 +66,26 @@ public class HttpManagementRestTest extends QpidRestTestCase
         assertEquals("Unexpected http sasl auth enabled", false, details.get(HttpManagement.HTTP_SASL_AUTHENTICATION_ENABLED));
         assertEquals("Unexpected https sasl auth enabled", false, details.get(HttpManagement.HTTPS_SASL_AUTHENTICATION_ENABLED));
     }
+
+    public void testUpdateAttributesWithInvalidValues() throws Exception
+    {
+        Map<String, Object> invalidAttributes = new HashMap<String, Object>();
+        invalidAttributes.put(HttpManagement.HTTPS_BASIC_AUTHENTICATION_ENABLED, 1);
+        invalidAttributes.put(HttpManagement.HTTPS_SASL_AUTHENTICATION_ENABLED, 2);
+        invalidAttributes.put(HttpManagement.HTTP_SASL_AUTHENTICATION_ENABLED, 3);
+        invalidAttributes.put(HttpManagement.TIME_OUT, "undefined");
+
+        for (Map.Entry<String, Object> invalidAttribute : invalidAttributes.entrySet())
+        {
+            Map<String, Object> attributes = new HashMap<String, Object>();
+            attributes.put(invalidAttribute.getKey(), invalidAttribute.getValue());
+            int response = getRestTestHelper().submitRequest("/rest/plugin/" + TestBrokerConfiguration.ENTRY_NAME_HTTP_MANAGEMENT, "PUT", attributes);
+            assertEquals("Update should fail for attribute " + invalidAttribute.getKey() + " with value " + invalidAttribute.getValue() , 409, response);
+        }
+
+        Map<String, Object> attributes = new HashMap<String, Object>();
+        attributes.put(HttpManagement.TIME_OUT, -1l);
+        int response  = getRestTestHelper().submitRequest("/rest/plugin/" + TestBrokerConfiguration.ENTRY_NAME_HTTP_MANAGEMENT, "PUT", attributes);
+        assertEquals("Update should fail for invalid session timeout", 409, response);
+    }
 }