QPID-2452: Fixed control over the encoding used when sending a string valued variant. The user is currently responsible for correctly setting any encoding (e.g. utf8). If none is specified it will be transfered as an amqp0-10 vbin. Fixed bug preventing correct assignment of encoding in variants.

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

6 files changed, 82 insertions, 6 deletions

diff --git a/qpid/cpp/include/qpid/framing/FieldValue.h b/qpid/cpp/include/qpid/framing/FieldValue.h
index f413d5a552..8af1f8dedd 100644
--- a/qpid/cpp/include/qpid/framing/FieldValue.h
+++ b/qpid/cpp/include/qpid/framing/FieldValue.h
@@ -335,6 +335,16 @@ class Str16Value : public FieldValue {
     QPID_COMMON_EXTERN Str16Value(const std::string& v);
 };
 
+class Var16Value : public FieldValue {
+  public:
+    QPID_COMMON_EXTERN Var16Value(const std::string& v, uint8_t code);
+};
+
+class Var32Value : public FieldValue {
+  public:
+    QPID_COMMON_EXTERN Var32Value(const std::string& v, uint8_t code);
+};
+
 class Struct32Value : public FieldValue {
   public:
     QPID_COMMON_EXTERN Struct32Value(const std::string& v);
diff --git a/qpid/cpp/src/qpid/client/amqp0_10/Codecs.cpp b/qpid/cpp/src/qpid/client/amqp0_10/Codecs.cpp
index e6d51ae725..3e17fc968b 100644
--- a/qpid/cpp/src/qpid/client/amqp0_10/Codecs.cpp
+++ b/qpid/cpp/src/qpid/client/amqp0_10/Codecs.cpp
@@ -25,8 +25,10 @@
 #include "qpid/framing/FieldTable.h"
 #include "qpid/framing/FieldValue.h"
 #include "qpid/framing/List.h"
+#include "qpid/log/Statement.h"
 #include <algorithm>
 #include <functional>
+#include <limits>
 
 using namespace qpid::framing;
 using namespace qpid::messaging;
@@ -39,6 +41,7 @@ namespace {
 const std::string iso885915("iso-8859-15");
 const std::string utf8("utf8");
 const std::string utf16("utf16");
+const std::string binary("binary");
 const std::string amqp0_10_binary("amqp0-10:binary");
 const std::string amqp0_10_bit("amqp0-10:bit");
 const std::string amqp0_10_datetime("amqp0-10:datetime");
@@ -163,8 +166,8 @@ Variant toVariant(boost::shared_ptr<FieldValue> in)
       case 0x96: 
       case 0xa0:
       case 0xab:
-        setEncodingFor(out, in->getType());
         out = in->get<std::string>();
+        setEncodingFor(out, in->getType());
         break;
 
       case 0xa8:
@@ -188,6 +191,28 @@ Variant toVariant(boost::shared_ptr<FieldValue> in)
     return out;
 }
 
+boost::shared_ptr<FieldValue> convertString(const std::string& value, const std::string& encoding)
+{
+    bool large = value.size() > std::numeric_limits<uint16_t>::max();
+    if (encoding.empty() || encoding == amqp0_10_binary || encoding == binary) {
+        if (large) {
+            return boost::shared_ptr<FieldValue>(new Var32Value(value, 0xa0));
+        } else {
+            return boost::shared_ptr<FieldValue>(new Var16Value(value, 0x90));
+        }
+    } else if (encoding == utf8 && !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));
+    } 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.");
+        return boost::shared_ptr<FieldValue>(new Var32Value(value, 0xa0));
+    }
+}
+
 boost::shared_ptr<FieldValue> toFieldValue(const Variant& in)
 {
     boost::shared_ptr<FieldValue> out;
@@ -204,8 +229,7 @@ boost::shared_ptr<FieldValue> toFieldValue(const Variant& in)
       case VAR_INT64: out = boost::shared_ptr<FieldValue>(new Integer64Value(in.asInt64())); break;
       case VAR_FLOAT: out = boost::shared_ptr<FieldValue>(new FloatValue(in.asFloat())); break;
       case VAR_DOUBLE: out = boost::shared_ptr<FieldValue>(new DoubleValue(in.asDouble())); break;
-        //TODO: check encoding (and length?) when deciding what AMQP type to treat string as
-      case VAR_STRING: out = boost::shared_ptr<FieldValue>(new Str16Value(in.asString())); break;
+      case VAR_STRING: out = convertString(in.asString(), in.getEncoding()); break;
       case VAR_UUID: out = boost::shared_ptr<FieldValue>(new UuidValue(in.asUuid().data())); break;
       case VAR_MAP: 
         out = boost::shared_ptr<FieldValue>(toFieldTableValue(in.asMap()));
diff --git a/qpid/cpp/src/qpid/framing/FieldValue.cpp b/qpid/cpp/src/qpid/framing/FieldValue.cpp
index fd911645f4..ce5a50117c 100644
--- a/qpid/cpp/src/qpid/framing/FieldValue.cpp
+++ b/qpid/cpp/src/qpid/framing/FieldValue.cpp
@@ -130,6 +130,21 @@ Str16Value::Str16Value(const std::string& v) :
             reinterpret_cast<const uint8_t*>(v.data()+v.size())))
 {}
 
+Var16Value::Var16Value(const std::string& v, uint8_t code) :
+    FieldValue(
+        code,
+        new VariableWidthValue<2>(
+            reinterpret_cast<const uint8_t*>(v.data()),
+            reinterpret_cast<const uint8_t*>(v.data()+v.size())))
+{}
+Var32Value::Var32Value(const std::string& v, uint8_t code) :
+    FieldValue(
+        code,
+        new VariableWidthValue<4>(
+            reinterpret_cast<const uint8_t*>(v.data()),
+            reinterpret_cast<const uint8_t*>(v.data()+v.size())))
+{}
+
 Struct32Value::Struct32Value(const std::string& v) :
     FieldValue(
         0xAB,
diff --git a/qpid/cpp/src/qpid/messaging/Variant.cpp b/qpid/cpp/src/qpid/messaging/Variant.cpp
index ba93f160ec..2567b7508b 100644
--- a/qpid/cpp/src/qpid/messaging/Variant.cpp
+++ b/qpid/cpp/src/qpid/messaging/Variant.cpp
@@ -20,6 +20,7 @@
  */
 #include "qpid/messaging/Variant.h"
 #include "qpid/Msg.h"
+#include "qpid/log/Statement.h"
 #include <boost/format.hpp>
 #include <boost/lexical_cast.hpp>
 #include <algorithm>
@@ -50,7 +51,7 @@ class VariantImpl
     VariantImpl(int64_t);
     VariantImpl(float);
     VariantImpl(double);
-    VariantImpl(const std::string&);
+    VariantImpl(const std::string&, const std::string& encoding=std::string());
     VariantImpl(const Variant::Map&);
     VariantImpl(const Variant::List&);
     VariantImpl(const Uuid&);
@@ -130,7 +131,8 @@ VariantImpl::VariantImpl(int32_t i) : type(VAR_INT32) { value.i32 = i; }
 VariantImpl::VariantImpl(int64_t i) : type(VAR_INT64) { value.i64 = i; }
 VariantImpl::VariantImpl(float f) : type(VAR_FLOAT) { value.f = f; }
 VariantImpl::VariantImpl(double d) : type(VAR_DOUBLE) { value.d = d; }
-VariantImpl::VariantImpl(const std::string& s) : type(VAR_STRING) { value.v = new std::string(s); }
+VariantImpl::VariantImpl(const std::string& s, const std::string& e)
+    : type(VAR_STRING), encoding(e) { value.v = new std::string(s); }
 VariantImpl::VariantImpl(const Variant::Map& m) : type(VAR_MAP) { value.v = new Variant::Map(m); }
 VariantImpl::VariantImpl(const Variant::List& l) : type(VAR_LIST) { value.v = new Variant::List(l); }
 VariantImpl::VariantImpl(const Uuid& u) : type(VAR_UUID) { value.v = new Uuid(u); }
@@ -448,7 +450,7 @@ VariantImpl* VariantImpl::create(const Variant& v)
       case VAR_INT64: return new VariantImpl(v.asInt64());
       case VAR_FLOAT: return new VariantImpl(v.asFloat());
       case VAR_DOUBLE: return new VariantImpl(v.asDouble());
-      case VAR_STRING: return new VariantImpl(v.asString());
+      case VAR_STRING: return new VariantImpl(v.asString(), v.getEncoding());
       case VAR_MAP: return new VariantImpl(v.asMap());
       case VAR_LIST: return new VariantImpl(v.asList());
       case VAR_UUID: return new VariantImpl(v.asUuid());
diff --git a/qpid/cpp/src/tests/MessagingSessionTests.cpp b/qpid/cpp/src/tests/MessagingSessionTests.cpp
index dea98216b4..a1e90f83f3 100644
--- a/qpid/cpp/src/tests/MessagingSessionTests.cpp
+++ b/qpid/cpp/src/tests/MessagingSessionTests.cpp
@@ -336,6 +336,12 @@ QPID_AUTO_TEST_CASE(testMapMessage)
     MapContent content(out);
     content["abc"] = "def";
     content["pi"] = 3.14f;
+    Variant utf8("A utf 8 string");
+    utf8.setEncoding("utf8");
+    content["utf8"] = utf8;
+    Variant utf16("\x00\x61\x00\x62\x00\x63");
+    utf16.setEncoding("utf16");
+    content["utf16"] = utf16;
     content.encode();
     sender.send(out);
     Receiver receiver = fix.session.createReceiver(fix.queue);
@@ -343,6 +349,10 @@ QPID_AUTO_TEST_CASE(testMapMessage)
     MapView view(in);
     BOOST_CHECK_EQUAL(view["abc"].asString(), "def");
     BOOST_CHECK_EQUAL(view["pi"].asFloat(), 3.14f);
+    BOOST_CHECK_EQUAL(view["utf8"].asString(), utf8.asString());
+    BOOST_CHECK_EQUAL(view["utf8"].getEncoding(), utf8.getEncoding());
+    BOOST_CHECK_EQUAL(view["utf16"].asString(), utf16.asString());
+    BOOST_CHECK_EQUAL(view["utf16"].getEncoding(), utf16.getEncoding());
     fix.session.acknowledge();
 }
 
diff --git a/qpid/cpp/src/tests/Variant.cpp b/qpid/cpp/src/tests/Variant.cpp
index c0bb9772c8..db9e419eab 100644
--- a/qpid/cpp/src/tests/Variant.cpp
+++ b/qpid/cpp/src/tests/Variant.cpp
@@ -178,6 +178,21 @@ QPID_AUTO_TEST_CASE(testIsEqualTo)
     BOOST_CHECK_EQUAL(a, b);
 }
 
+QPID_AUTO_TEST_CASE(testEncoding)
+{
+    Variant a("abc");
+    a.setEncoding("utf8");
+    Variant b = a;
+    Variant map = Variant::Map();
+    map.asMap()["a"] = a;
+    map.asMap()["b"] = b;
+    BOOST_CHECK_EQUAL(a.getEncoding(), std::string("utf8"));
+    BOOST_CHECK_EQUAL(a.getEncoding(), b.getEncoding());
+    BOOST_CHECK_EQUAL(a.getEncoding(), map.asMap()["a"].getEncoding());
+    BOOST_CHECK_EQUAL(b.getEncoding(), map.asMap()["b"].getEncoding());
+    BOOST_CHECK_EQUAL(map.asMap()["a"].getEncoding(), map.asMap()["b"].getEncoding());
+}
+
 QPID_AUTO_TEST_SUITE_END()
 
 }} // namespace qpid::tests