Fixes broker issues when max_count or max_size are invalid.

Accepts non-negative integer values, or strings containing the lexical representation of such values.


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

3 files changed, 95 insertions, 8 deletions

diff --git a/qpid/cpp/src/qpid/broker/QueuePolicy.cpp b/qpid/cpp/src/qpid/broker/QueuePolicy.cpp
index 4b185ef025..c1d979ac33 100644
--- a/qpid/cpp/src/qpid/broker/QueuePolicy.cpp
+++ b/qpid/cpp/src/qpid/broker/QueuePolicy.cpp
@@ -24,6 +24,7 @@
 #include "qpid/framing/FieldValue.h"
 #include "qpid/framing/reply_exceptions.h"
 #include "qpid/log/Statement.h"
+#include <sstream>
 
 using namespace qpid::broker;
 using namespace qpid::framing;
@@ -115,12 +116,24 @@ void QueuePolicy::update(FieldTable& settings)
     settings.setString(typeKey, type);
 }
 
-
-int QueuePolicy::getInt(const FieldTable& settings, const std::string& key, int defaultValue)
+uint32_t QueuePolicy::getCapacity(const FieldTable& settings, const std::string& key, uint32_t defaultValue)
 {
     FieldTable::ValuePtr v = settings.get(key);
-    if (v && v->convertsTo<int>()) return v->get<int>();
-    else return defaultValue;
+
+    int32_t result = 0;
+
+    if (!v) return defaultValue;
+    if (v->convertsTo<int>()) {
+        result = v->get<int>();
+        if (result >= 0) return result;
+    }
+    else {
+        string s(v->get<string>());  // I assume anything can be converted to a string
+        std::istringstream convert(s);
+        if (convert >> result && result >= 0) return result;
+    }
+
+    throw InvalidArgumentException(QPID_MSG("Cannot convert " << key << " to unsigned integer"));
 }
 
 std::string QueuePolicy::getType(const FieldTable& settings)
@@ -297,8 +310,8 @@ std::auto_ptr<QueuePolicy> QueuePolicy::createQueuePolicy(const qpid::framing::F
 
 std::auto_ptr<QueuePolicy> QueuePolicy::createQueuePolicy(const std::string& name, const qpid::framing::FieldTable& settings)
 {
-    uint32_t maxCount = getInt(settings, maxCountKey, 0);
-    uint32_t maxSize = getInt(settings, maxSizeKey, defaultMaxSize);
+    uint32_t maxCount = getCapacity(settings, maxCountKey, 0);
+    uint32_t maxSize = getCapacity(settings, maxSizeKey, defaultMaxSize);
     if (maxCount || maxSize) {
         return createQueuePolicy(name, maxCount, maxSize, getType(settings));
     } else {
diff --git a/qpid/cpp/src/qpid/broker/QueuePolicy.h b/qpid/cpp/src/qpid/broker/QueuePolicy.h
index 7006b617a1..3cdd63784d 100644
--- a/qpid/cpp/src/qpid/broker/QueuePolicy.h
+++ b/qpid/cpp/src/qpid/broker/QueuePolicy.h
@@ -44,7 +44,7 @@ class QueuePolicy
     uint64_t size;
     bool policyExceeded;
             
-    static int getInt(const qpid::framing::FieldTable& settings, const std::string& key, int defaultValue);
+    static uint32_t getCapacity(const qpid::framing::FieldTable& settings, const std::string& key, uint32_t defaultValue);
 
   protected:
     uint64_t getCurrentQueueSize() const { return size; } 
diff --git a/qpid/tests/src/py/qpid_tests/broker_0_10/queue.py b/qpid/tests/src/py/qpid_tests/broker_0_10/queue.py
index eb38965190..294155ee8c 100644
--- a/qpid/tests/src/py/qpid_tests/broker_0_10/queue.py
+++ b/qpid/tests/src/py/qpid_tests/broker_0_10/queue.py
@@ -115,6 +115,80 @@ class QueueTests(TestBase010):
         except SessionException, e:
             self.assertEquals(405, e.args[0].error_code)
 
+    def test_declare_capacity_valid(self):
+        session = self.session
+
+        try:
+            queue_options = {}
+            queue_options["qpid.max_count"] = 0
+            queue_options["qpid.max_size"] = 0
+            session.queue_declare(queue="queue_capacity1", exclusive=True, arguments=queue_options)
+        except SessionException, e:
+            self.fail("Should accept queue create request with name=queue_capacity1, qpid.max_count=0 and qpid.max_size=0")
+
+        try:
+            queue_options = {}
+            queue_options["qpid.max_count"] = 3
+            queue_options["qpid.max_size"] = 0
+            session.queue_declare(queue="queue_capacity2", exclusive=True, arguments=queue_options)
+        except SessionException, e:
+            self.fail("Should accept queue create request with name=queue_capacity2, qpid.max_count=3 and qpid.max_size=0")
+
+        try:
+            queue_options = {}
+            queue_options["qpid.max_count"] = 0
+            queue_options["qpid.max_size"] = 300
+            session.queue_declare(queue="queue_capacity3", exclusive=True, arguments=queue_options)
+        except SessionException, e:
+            self.fail("Should accept queue create request with name=queue_capacity3, qpid.max_count=0 and qpid.max_size=300")
+
+        try:
+            queue_options = {}
+            queue_options["qpid.max_count"] = 0
+            queue_options["qpid.max_size"] = "3"
+            session.queue_declare(queue="queue_capacity6", exclusive=True, arguments=queue_options)
+        except SessionException, e:
+            self.fail("Should allow queue create request with qpid.max_size='3'")
+
+
+    def test_declare_capacity_invalid(self):
+        session = self.session
+        try:
+            queue_options = {}
+            queue_options["qpid.max_count"] = -1
+            queue_options["qpid.max_size"] = 0
+            session.queue_declare(queue="queue_capacity4", exclusive=True, arguments=queue_options)
+            self.fail("Should reject queue create request with name=queue_capacity4, qpid.max_count=-1 and qpid.max_size=0")
+        except SessionException, e:
+            pass
+
+        try:
+            queue_options = {}
+            queue_options["qpid.max_count"] = 0
+            queue_options["qpid.max_size"] = -1
+            session.queue_declare(queue="queue_capacity5", exclusive=True, arguments=queue_options)
+            self.fail("Should reject queue create request with name=queue_capacity5, qpid.max_count=0 and qpid.max_size=-1")
+        except SessionException, e:
+            pass
+
+        try:
+            queue_options = {}
+            queue_options["qpid.max_count"] = 0
+            queue_options["qpid.max_size"] = "flummery"
+            session.queue_declare(queue="queue_capacity7", exclusive=True, arguments=queue_options)
+            self.fail("Should deny queue create request with name=queue_capacity7, qpid.max_count=0 and qpid.max_size='flummery'")
+        except SessionException, e:
+            pass
+
+        try:
+            queue_options = {}
+            queue_options["qpid.max_count"] = 0
+            queue_options["qpid.max_size"] = 3.14159
+            session.queue_declare(queue="queue_capacity8", exclusive=True, arguments=queue_options)
+            self.fail("Should deny queue create request with name=queue_capacity4, qpid.max_count=0 and qpid.max_size=3.14159");
+        except SessionException, e:
+            pass
+
 
     def test_declare_passive(self):
         """
@@ -134,7 +208,7 @@ class QueueTests(TestBase010):
 
     def test_bind(self):
         """
-        Test various permutations of the queue.bind method
+        Test various permutations of the queue.bind method+
         """
         session = self.session
         session.queue_declare(queue="queue-1", exclusive=True, auto_delete=True)