QPID-2916: throw an exception when a data value cannot be encoded correctly as its type.

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

9 files changed, 110 insertions, 34 deletions

diff --git a/qpid/cpp/examples/qmf-agent/schema.xml b/qpid/cpp/examples/qmf-agent/schema.xml
index ad8510106d..84dc8b7643 100644
--- a/qpid/cpp/examples/qmf-agent/schema.xml
+++ b/qpid/cpp/examples/qmf-agent/schema.xml
@@ -28,7 +28,7 @@
 
     This class represents a parent object
 
-    <property name="name"      type="sstr" access="RC" index="y"/>
+    <property name="name"      type="lstr" access="RC" index="y"/>
     <property name="args"      type="map"  access="RO"/>
     <property name="list"      type="list" access="RO"/>
 
@@ -36,7 +36,7 @@
     <statistic name="count" type="count64" unit="tick" desc="Counter that increases monotonically"/>
 
     <method name="create_child" desc="Create child object">
-      <arg name="name"     dir="I" type="sstr"/>
+      <arg name="name"     dir="I" type="lstr"/>
       <arg name="childRef" dir="O" type="objId"/>
     </method>
 
@@ -55,7 +55,7 @@
   -->
   <class name="Child">
     <property name="ParentRef" type="objId"  references="Parent" access="RC" index="y" parentRef="y"/>
-    <property name="name"      type="sstr"                       access="RC" index="y"/>
+    <property name="name"      type="lstr"                       access="RC" index="y"/>
 
     <statistic name="count" type="count64" unit="tick" desc="Counter that increases monotonically"/>
 
@@ -63,7 +63,7 @@
   </class>
 
   <eventArguments>
-    <arg name="childName" type="sstr"/>
+    <arg name="childName" type="lstr"/>
   </eventArguments>
 
   <event name="ChildCreated"   args="childName"/>
diff --git a/qpid/cpp/src/qpid/acl/management-schema.xml b/qpid/cpp/src/qpid/acl/management-schema.xml
index f4637253d0..7f48a9be34 100644
--- a/qpid/cpp/src/qpid/acl/management-schema.xml
+++ b/qpid/cpp/src/qpid/acl/management-schema.xml
@@ -18,7 +18,7 @@
 
   <class name="Acl">
     <property name="brokerRef"     type="objId"   references="org.apache.qpid.broker:Broker" access="RO" index="y" parentRef="y"/>
-    <property name="policyFile"    type="sstr"    access="RO"    desc="Name of the policy file"/>
+    <property name="policyFile"    type="lstr"    access="RO"    desc="Name of the policy file"/>
     <property name="enforcingAcl"  type="bool"    access="RO"    desc="Currently Enforcing ACL"/>
     <property name="transferAcl"   type="bool"    access="RO"    desc="Any transfer ACL rules in force"/>
     <property name="lastAclLoad"   type="absTime" access="RO"    desc="Timestamp of last successful load of ACL"/>
@@ -32,7 +32,7 @@
     <arg name="arguments"  type="map"/>
     <arg name="objectName" type="sstr"/>
     <arg name="objectType" type="sstr"/>
-    <arg name="reason"     type="sstr"/>
+    <arg name="reason"     type="lstr"/>
     <arg name="userId"     type="sstr"/>
   </eventArguments>
 
diff --git a/qpid/cpp/src/qpid/amqp_0_10/Codecs.cpp b/qpid/cpp/src/qpid/amqp_0_10/Codecs.cpp
index abe6b2c7da..0fbe2a60b9 100644
--- a/qpid/cpp/src/qpid/amqp_0_10/Codecs.cpp
+++ b/qpid/cpp/src/qpid/amqp_0_10/Codecs.cpp
@@ -198,15 +198,21 @@ boost::shared_ptr<FieldValue> convertString(const std::string& value, const std:
         } else {
             return boost::shared_ptr<FieldValue>(new Var16Value(value, 0x90));
         }
-    } else if (encoding == utf8 && !large) {
+    } else if (encoding == utf8) {
+        if (!large)
             return boost::shared_ptr<FieldValue>(new Str16Value(value));
-    } else if (encoding == utf16 && !large) {
-        return boost::shared_ptr<FieldValue>(new Var16Value(value, 0x96));
-    } else if (encoding == iso885915 && !large) {
-        return boost::shared_ptr<FieldValue>(new Var16Value(value, 0x94));
+        throw Exception(QPID_MSG("Could not encode utf8 character string - too long (" << value.size() << " bytes)"));
+    } else if (encoding == utf16) {
+        if (!large)
+            return boost::shared_ptr<FieldValue>(new Var16Value(value, 0x96));
+        throw Exception(QPID_MSG("Could not encode utf16 character string - too long (" << value.size() << " bytes)"));
+    } else if (encoding == iso885915) {
+        if (!large)
+            return boost::shared_ptr<FieldValue>(new Var16Value(value, 0x94));
+        throw Exception(QPID_MSG("Could not encode iso-8859-15 character string - too long (" << value.size() << " bytes)"));
     } else {
-        //either the string is too large for the encoding in amqp 0-10, or the encoding was not recognised
-        QPID_LOG(warning, "Could not encode " << value.size() << " byte value as " << encoding << ", encoding as vbin32.");
+        // the encoding was not recognised
+        QPID_LOG(warning, "Unknown byte encoding: [" << encoding << "], encoding as vbin32.");
         return boost::shared_ptr<FieldValue>(new Var32Value(value, 0xa0));
     }
 }
diff --git a/qpid/cpp/src/qpid/framing/Buffer.cpp b/qpid/cpp/src/qpid/framing/Buffer.cpp
index 051e7a2362..7506cdca7b 100644
--- a/qpid/cpp/src/qpid/framing/Buffer.cpp
+++ b/qpid/cpp/src/qpid/framing/Buffer.cpp
@@ -20,6 +20,7 @@
  */
 #include "qpid/framing/Buffer.h"
 #include "qpid/framing/FieldTable.h" 
+#include "qpid/Msg.h"
 #include <string.h>
 #include <boost/format.hpp>
 namespace qpid {
@@ -211,17 +212,29 @@ uint64_t Buffer::getUInt<8>() {
 
 template <>
 void Buffer::putUInt<1>(uint64_t i) {
-    putOctet(i);
+    if (std::numeric_limits<uint8_t>::min() <= i && i <= std::numeric_limits<uint8_t>::max()) {
+        putOctet(i);
+        return;
+    }
+    throw Exception(QPID_MSG("Could not encode (" << i << ") as uint8_t."));
 }
 
 template <>
 void Buffer::putUInt<2>(uint64_t i) {
-    putShort(i);
+    if (std::numeric_limits<uint16_t>::min() <= i && i <= std::numeric_limits<uint16_t>::max()) {
+        putShort(i);
+        return;
+    }
+    throw Exception(QPID_MSG("Could not encode (" << i << ") as uint16_t."));
 }
 
 template <>
 void Buffer::putUInt<4>(uint64_t i) {
-    putLong(i);
+    if (std::numeric_limits<uint32_t>::min() <= i && i <= std::numeric_limits<uint32_t>::max()) {
+        putLong(i);
+        return;
+    }
+    throw Exception(QPID_MSG("Could not encode (" << i << ") as uint32_t."));
 }
 
 template <>
@@ -231,18 +244,26 @@ void Buffer::putUInt<8>(uint64_t i) {
 
 void Buffer::putShortString(const string& s){
     size_t slen = s.length();
-    uint8_t len = slen < 0x100 ? (uint8_t) slen : 0xFF;
-    putOctet(len);
-    s.copy(data + position, len);
-    position += len;    
+    if (slen <= std::numeric_limits<uint8_t>::max()) {
+        uint8_t len = (uint8_t) slen;
+        putOctet(len);
+        s.copy(data + position, len);
+        position += len;
+        return;
+    }
+    throw Exception(QPID_MSG("Could not encode string of " << slen << " bytes as uint8_t string."));
 }
 
 void Buffer::putMediumString(const string& s){
     size_t slen = s.length();
-    uint16_t len = slen < 0x10000 ? (uint16_t) slen : 0xFFFF;
-    putShort(len);
-    s.copy(data + position, len);
-    position += len;    
+    if (slen <= std::numeric_limits<uint16_t>::max()) {
+        uint16_t len = (uint16_t) slen;
+        putShort(len);
+        s.copy(data + position, len);
+        position += len;
+        return;
+    }
+    throw Exception(QPID_MSG("Could not encode string of " << slen << " bytes as uint16_t string."));
 }
 
 void Buffer::putLongString(const string& s){
diff --git a/qpid/cpp/src/tests/FramingTest.cpp b/qpid/cpp/src/tests/FramingTest.cpp
index 3d0fa0c0de..f8795316cc 100644
--- a/qpid/cpp/src/tests/FramingTest.cpp
+++ b/qpid/cpp/src/tests/FramingTest.cpp
@@ -151,6 +151,17 @@ QPID_AUTO_TEST_CASE(testMessageCancelBodyFrame)
     BOOST_CHECK_EQUAL(tostring(in), tostring(out));
 }
 
+QPID_AUTO_TEST_CASE(badStrings) {
+    char data[(65535 + 2) + (255 + 1)];
+    Buffer b(data, sizeof(data));
+    BOOST_CHECK_THROW(b.putShortString(std::string(256, 'X')),
+                      Exception);
+    BOOST_CHECK_THROW(b.putMediumString(std::string(65536, 'X')),
+                      Exception);
+    b.putShortString(std::string(255, 'X'));
+    b.putMediumString(std::string(65535, 'X'));
+}
+
 QPID_AUTO_TEST_SUITE_END()
 
 }} // namespace qpid::tests
diff --git a/qpid/cpp/src/tests/Variant.cpp b/qpid/cpp/src/tests/Variant.cpp
index 596bde36de..b4188f524b 100644
--- a/qpid/cpp/src/tests/Variant.cpp
+++ b/qpid/cpp/src/tests/Variant.cpp
@@ -20,10 +20,12 @@
  */
 #include <iostream>
 #include "qpid/types/Variant.h"
+#include "qpid/amqp_0_10/Codecs.h"
 
 #include "unit_test.h"
 
 using namespace qpid::types;
+using namespace qpid::amqp_0_10;
 
 namespace qpid {
 namespace tests {
@@ -686,6 +688,30 @@ QPID_AUTO_TEST_CASE(testEncoding)
     BOOST_CHECK_EQUAL(map.asMap()["a"].getEncoding(), map.asMap()["b"].getEncoding());
 }
 
+QPID_AUTO_TEST_CASE(testBufferEncoding)
+{
+    Variant a("abc");
+    a.setEncoding("utf8");
+    std::string buffer;
+
+    Variant::Map inMap, outMap;
+    inMap["a"] = a;
+
+    MapCodec::encode(inMap, buffer);
+    MapCodec::decode(buffer, outMap);
+    BOOST_CHECK_EQUAL(inMap, outMap);
+
+    inMap["b"] = Variant(std::string(65535, 'X'));
+    inMap["b"].setEncoding("utf16");
+    MapCodec::encode(inMap, buffer);
+    MapCodec::decode(buffer, outMap);
+    BOOST_CHECK_EQUAL(inMap, outMap);
+
+    inMap["fail"] = Variant(std::string(65536, 'X'));
+    inMap["fail"].setEncoding("utf16");
+    BOOST_CHECK_THROW(MapCodec::encode(inMap, buffer), std::exception);
+}
+
 QPID_AUTO_TEST_SUITE_END()
 
 }} // namespace qpid::tests
diff --git a/qpid/cpp/src/tests/testagent.xml b/qpid/cpp/src/tests/testagent.xml
index 8be21b7e68..0b1436f999 100644
--- a/qpid/cpp/src/tests/testagent.xml
+++ b/qpid/cpp/src/tests/testagent.xml
@@ -28,13 +28,13 @@
 
     This class represents a parent object
 
-    <property name="name"      type="sstr" access="RC" index="y"/>
+    <property name="name"      type="lstr" access="RC" index="y"/>
 
     <statistic name="state" type="sstr"                desc="Operational state of the link"/>
     <statistic name="count" type="count64" unit="tick" desc="Counter that increases monotonically"/>
 
     <method name="create_child" desc="Create child object">
-      <arg name="name"     dir="I" type="sstr"/>
+      <arg name="name"     dir="I" type="lstr"/>
       <arg name="childRef" dir="O" type="objId"/>
     </method>
   </class>
@@ -47,7 +47,7 @@
   -->
   <class name="Child">
     <property name="ParentRef" type="objId"  references="Parent" access="RC" index="y" parentRef="y"/>
-    <property name="name"      type="sstr"                       access="RC" index="y"/>
+    <property name="name"      type="lstr"                       access="RC" index="y"/>
 
     <statistic name="count" type="count64" unit="tick" desc="Counter that increases monotonically"/>
 
@@ -55,7 +55,7 @@
   </class>
 
   <eventArguments>
-    <arg name="childName" type="sstr"/>
+    <arg name="childName" type="lstr"/>
   </eventArguments>
 
   <event name="ChildCreated"   args="childName"/>
diff --git a/qpid/python/qpid/codec010.py b/qpid/python/qpid/codec010.py
index 5ad1ef14c1..0846db6bf7 100644
--- a/qpid/python/qpid/codec010.py
+++ b/qpid/python/qpid/codec010.py
@@ -85,11 +85,15 @@ class Codec(Packer):
   def read_uint8(self):
     return self.unpack("!B")
   def write_uint8(self, n):
+    if n < 0 or n > 255:
+      raise CodecException("Cannot encode %d as uint8" % n)
     return self.pack("!B", n)
 
   def read_int8(self):
     return self.unpack("!b")
   def write_int8(self, n):
+    if n < -128 or n > 127:
+      raise CodecException("Cannot encode %d as int8" % n)
     self.pack("!b", n)
 
   def read_char(self):
@@ -108,22 +112,30 @@ class Codec(Packer):
   def read_uint16(self):
     return self.unpack("!H")
   def write_uint16(self, n):
+    if n < 0 or n > 65535:
+      raise CodecException("Cannot encode %d as uint16" % n)
     self.pack("!H", n)
 
   def read_int16(self):
     return self.unpack("!h")
   def write_int16(self, n):
+    if n < -32768 or n > 32767:
+      raise CodecException("Cannot encode %d as int16" % n)
     self.pack("!h", n)
 
 
   def read_uint32(self):
     return self.unpack("!L")
   def write_uint32(self, n):
+    if n < 0 or n > 4294967295:
+      raise CodecException("Cannot encode %d as uint32" % n)
     self.pack("!L", n)
 
   def read_int32(self):
     return self.unpack("!l")
   def write_int32(self, n):
+    if n < -2147483648 or n > 2147483647:
+      raise CodecException("Cannot encode %d as int32" % n)
     self.pack("!l", n)
 
   def read_float(self):
diff --git a/qpid/specs/management-schema.xml b/qpid/specs/management-schema.xml
index d8786ea4e5..30be0093ac 100644
--- a/qpid/specs/management-schema.xml
+++ b/qpid/specs/management-schema.xml
@@ -70,7 +70,7 @@
     <property name="stagingThreshold" type="uint32" access="RO" desc="Broker stages messages over this size to disk"/>
     <property name="mgmtPubInterval"  type="uint16" access="RW" unit="second" min="1" desc="Interval for management broadcasts"/>
     <property name="version"          type="sstr"   access="RO" desc="Running software version"/>
-    <property name="dataDir"          type="sstr"   access="RO" optional="y" desc="Persistent configuration storage location"/>
+    <property name="dataDir"          type="lstr"   access="RO" optional="y" desc="Persistent configuration storage location"/>
     <statistic name="uptime" type="deltaTime"/>
 
     <method name="echo" desc="Request a response to test the path to the management broker">
@@ -198,7 +198,7 @@
   <class name="Binding">
     <property name="exchangeRef" type="objId" references="Exchange" access="RC" index="y" parentRef="y"/>
     <property name="queueRef"    type="objId" references="Queue"    access="RC" index="y"/>
-    <property name="bindingKey"  type="sstr"  access="RC" index="y"/>
+    <property name="bindingKey"  type="lstr"  access="RC" index="y"/>
     <property name="arguments"   type="map"   access="RC"/>
     <property name="origin"      type="sstr"  access="RO" optional="y"/>
 
@@ -234,7 +234,7 @@
     <property name="SystemConnection"   type="bool"   access="RC" desc="Infrastucture/ Inter-system connection (Cluster, Federation, ...)"/>
     <property name="federationLink"     type="bool"   access="RO" desc="Is this a federation link"/>
     <property name="authIdentity"       type="sstr"   access="RO" desc="authId of connection if authentication enabled"/>
-    <property name="remoteProcessName"  type="sstr"   access="RO" optional="y" desc="Name of executable running as remote client"/>
+    <property name="remoteProcessName"  type="lstr"   access="RO" optional="y" desc="Name of executable running as remote client"/>
     <property name="remotePid"          type="uint32" access="RO" optional="y" desc="Process ID of remote client"/>
     <property name="remoteParentPid"    type="uint32" access="RO" optional="y" desc="Parent Process ID of remote client"/>
     <property name="shadow"             type="bool"   access="RO" desc="True for shadow connections"/>
@@ -263,7 +263,7 @@
     <property name="durable"   type="bool"   access="RC"/>
 
     <statistic name="state"       type="sstr" desc="Operational state of the link"/>
-    <statistic name="lastError"   type="sstr" desc="Reason link is not operational"/>
+    <statistic name="lastError"   type="lstr" desc="Reason link is not operational"/>
 
     <method name="close"/> 
 
@@ -271,7 +271,7 @@
       <arg name="durable"     dir="I" type="bool"/>
       <arg name="src"         dir="I" type="sstr"/>
       <arg name="dest"        dir="I" type="sstr"/>
-      <arg name="key"         dir="I" type="sstr"/>
+      <arg name="key"         dir="I" type="lstr"/>
       <arg name="tag"         dir="I" type="sstr"/>
       <arg name="excludes"    dir="I" type="sstr"/>
       <arg name="srcIsQueue"  dir="I" type="bool"/>
@@ -293,7 +293,7 @@
     <property name="durable"     type="bool"   access="RC"/>
     <property name="src"         type="sstr"   access="RC"/>
     <property name="dest"        type="sstr"   access="RC"/>
-    <property name="key"         type="sstr"   access="RC"/>
+    <property name="key"         type="lstr"   access="RC"/>
     <property name="srcIsQueue"  type="bool"   access="RC"/>
     <property name="srcIsLocal"  type="bool"   access="RC"/>
     <property name="tag"         type="sstr"   access="RC"/>