From 066fb5dafce71acc93bb04f2a271b43870a6b0bb Mon Sep 17 00:00:00 2001
From: Nat Goodspeed <nat@lindenlab.com>
Date: Wed, 13 May 2020 16:37:12 -0400
Subject: [PATCH] DRTVWR-476: Default LLSDNotationFormatter now
OPTIONS_PRETTY_BINARY.
LLSDNotationFormatter (also LLSDNotationStreamer that uses it, plus
operator<<(std::ostream&, const LLSD&) that uses LLSDNotationStreamer) is most
useful for displaying LLSD to a human, e.g. for logging. Having the default
dump raw binary bytes into the log file is not only suboptimal, it can
truncate the output if one of those bytes is '\0'. (This is a problem with the
logging subsystem, but that's a story for another day.)
Use OPTIONS_PRETTY_BINARY wherever there is a default LLSDFormatter
::EFormatterOptions argument.
Also, allow setting LLSDFormatter subclass boolalpha(), realFormat() and
format(options) using optional constructor arguments. Naturally, each subclass
that supports this must accept and forward these constructor arguments to its
LLSDFormatter base class constructor.
Fix a couple bugs in LLSDNotationFormatter::format_impl() for an LLSD::Binary
value with OPTIONS_PRETTY_BINARY:
- The code unconditionally emitted a b(len) type prefix followed by either raw
binary or hex, depending on the option flag. OPTIONS_PRETTY_BINARY caused it
to emit "0x" before the hex representation of the data. This is wrong in
that it can't be read back by either the C++ or the Python LLSD parser.
Correct OPTIONS_PRETTY_BINARY formatting consists of b16"hex digits" rather
than b(len)"raw bytes".
- Although the code did set hex mode, it didn't set either the field width or
the fill character, so that a byte value less than 16 would emit a single
digit rather than two.
Instead of having one LLSDFormatter::format() method with an optional options
argument, declare two overloads. The format() overload without options passes
the mOptions data member to the overload accepting options.
Refactor the LLSDFormatter family, hoisting the recursive format_impl() method
(accepting level) to a pure virtual method at LLSDFormatter base-class level.
Most subclasses therefore need not override either base-class format() method,
only format_impl(). In fact the short format() overload isn't even virtual.
Consistently use LLSDFormatter::EFormatterOptions enum as the options
parameter wherever such options are accepted.
---
indra/llcommon/llsdserialize.cpp | 74 +++++++++------
indra/llcommon/llsdserialize.h | 100 ++++++++++++--------
indra/llcommon/llsdserialize_xml.cpp | 10 +-
indra/llcommon/tests/llsdserialize_test.cpp | 29 ++++--
4 files changed, 134 insertions(+), 79 deletions(-)
diff --git a/indra/llcommon/llsdserialize.cpp b/indra/llcommon/llsdserialize.cpp
index 79934642ae2..598bec05585 100644
--- a/indra/llcommon/llsdserialize.cpp
+++ b/indra/llcommon/llsdserialize.cpp
@@ -66,7 +66,8 @@ const std::string LLSD_NOTATION_HEADER("llsd/notation");
*/
// static
-void LLSDSerialize::serialize(const LLSD& sd, std::ostream& str, ELLSD_Serialize type, U32 options)
+void LLSDSerialize::serialize(const LLSD& sd, std::ostream& str, ELLSD_Serialize type,
+ LLSDFormatter::EFormatterOptions options)
{
LLPointer<LLSDFormatter> f = NULL;
@@ -174,10 +175,10 @@ bool LLSDSerialize::deserialize(LLSD& sd, std::istream& str, S32 max_bytes)
{
p = new LLSDXMLParser;
}
- else if (header == LLSD_NOTATION_HEADER)
- {
- p = new LLSDNotationParser;
- }
+ else if (header == LLSD_NOTATION_HEADER)
+ {
+ p = new LLSDNotationParser;
+ }
else
{
LL_WARNS() << "deserialize request for unknown ELLSD_Serialize" << LL_ENDL;
@@ -1234,9 +1235,11 @@ bool LLSDBinaryParser::parseString(
/**
* LLSDFormatter
*/
-LLSDFormatter::LLSDFormatter() :
- mBoolAlpha(false)
+LLSDFormatter::LLSDFormatter(bool boolAlpha, const std::string& realFmt, EFormatterOptions options):
+ mOptions(options)
{
+ boolalpha(boolAlpha);
+ realFormat(realFmt);
}
// virtual
@@ -1253,6 +1256,17 @@ void LLSDFormatter::realFormat(const std::string& format)
mRealFormat = format;
}
+S32 LLSDFormatter::format(const LLSD& data, std::ostream& ostr) const
+{
+ // pass options captured by constructor
+ return format(data, ostr, mOptions);
+}
+
+S32 LLSDFormatter::format(const LLSD& data, std::ostream& ostr, EFormatterOptions options) const
+{
+ return format_impl(data, ostr, options, 0);
+}
+
void LLSDFormatter::formatReal(LLSD::Real real, std::ostream& ostr) const
{
std::string buffer = llformat(mRealFormat.c_str(), real);
@@ -1262,7 +1276,9 @@ void LLSDFormatter::formatReal(LLSD::Real real, std::ostream& ostr) const
/**
* LLSDNotationFormatter
*/
-LLSDNotationFormatter::LLSDNotationFormatter()
+LLSDNotationFormatter::LLSDNotationFormatter(bool boolAlpha, const std::string& realFormat,
+ EFormatterOptions options):
+ LLSDFormatter(boolAlpha, realFormat, options)
{
}
@@ -1278,14 +1294,8 @@ std::string LLSDNotationFormatter::escapeString(const std::string& in)
return ostr.str();
}
-// virtual
-S32 LLSDNotationFormatter::format(const LLSD& data, std::ostream& ostr, U32 options) const
-{
- S32 rv = format_impl(data, ostr, options, 0);
- return rv;
-}
-
-S32 LLSDNotationFormatter::format_impl(const LLSD& data, std::ostream& ostr, U32 options, U32 level) const
+S32 LLSDNotationFormatter::format_impl(const LLSD& data, std::ostream& ostr,
+ EFormatterOptions options, U32 level) const
{
S32 format_count = 1;
std::string pre;
@@ -1406,21 +1416,29 @@ S32 LLSDNotationFormatter::format_impl(const LLSD& data, std::ostream& ostr, U32
{
// *FIX: memory inefficient.
const std::vector<U8>& buffer = data.asBinary();
- ostr << "b(" << buffer.size() << ")\"";
- if(buffer.size())
+ if (options & LLSDFormatter::OPTIONS_PRETTY_BINARY)
{
- if (options & LLSDFormatter::OPTIONS_PRETTY_BINARY)
+ ostr << "b16\"";
+ if (! buffer.empty())
{
std::ios_base::fmtflags old_flags = ostr.flags();
ostr.setf( std::ios::hex, std::ios::basefield );
- ostr << "0x";
+ auto oldfill(ostr.fill('0'));
+ auto oldwidth(ostr.width());
for (int i = 0; i < buffer.size(); i++)
{
- ostr << (int) buffer[i];
+ // have to restate setw() before every conversion
+ ostr << std::setw(2) << (int) buffer[i];
}
+ ostr.width(oldwidth);
+ ostr.fill(oldfill);
ostr.flags(old_flags);
}
- else
+ }
+ else // ! OPTIONS_PRETTY_BINARY
+ {
+ ostr << "b(" << buffer.size() << ")\"";
+ if (! buffer.empty())
{
ostr.write((const char*)&buffer[0], buffer.size());
}
@@ -1437,11 +1455,12 @@ S32 LLSDNotationFormatter::format_impl(const LLSD& data, std::ostream& ostr, U32
return format_count;
}
-
/**
* LLSDBinaryFormatter
*/
-LLSDBinaryFormatter::LLSDBinaryFormatter()
+LLSDBinaryFormatter::LLSDBinaryFormatter(bool boolAlpha, const std::string& realFormat,
+ EFormatterOptions options):
+ LLSDFormatter(boolAlpha, realFormat, options)
{
}
@@ -1450,7 +1469,8 @@ LLSDBinaryFormatter::~LLSDBinaryFormatter()
{ }
// virtual
-S32 LLSDBinaryFormatter::format(const LLSD& data, std::ostream& ostr, U32 options) const
+S32 LLSDBinaryFormatter::format_impl(const LLSD& data, std::ostream& ostr,
+ EFormatterOptions options, U32 level) const
{
S32 format_count = 1;
switch(data.type())
@@ -1466,7 +1486,7 @@ S32 LLSDBinaryFormatter::format(const LLSD& data, std::ostream& ostr, U32 option
{
ostr.put('k');
formatString((*iter).first, ostr);
- format_count += format((*iter).second, ostr);
+ format_count += format_impl((*iter).second, ostr, options, level+1);
}
ostr.put('}');
break;
@@ -1481,7 +1501,7 @@ S32 LLSDBinaryFormatter::format(const LLSD& data, std::ostream& ostr, U32 option
LLSD::array_const_iterator end = data.endArray();
for(; iter != end; ++iter)
{
- format_count += format(*iter, ostr);
+ format_count += format_impl(*iter, ostr, options, level+1);
}
ostr.put(']');
break;
diff --git a/indra/llcommon/llsdserialize.h b/indra/llcommon/llsdserialize.h
index fe0f4443ef2..d6079fd9fa8 100644
--- a/indra/llcommon/llsdserialize.h
+++ b/indra/llcommon/llsdserialize.h
@@ -435,7 +435,8 @@ class LL_COMMON_API LLSDFormatter : public LLRefCount
/**
* @brief Constructor
*/
- LLSDFormatter();
+ LLSDFormatter(bool boolAlpha=false, const std::string& realFormat="",
+ EFormatterOptions options=OPTIONS_PRETTY_BINARY);
/**
* @brief Set the boolean serialization format.
@@ -459,15 +460,37 @@ class LL_COMMON_API LLSDFormatter : public LLRefCount
void realFormat(const std::string& format);
/**
- * @brief Call this method to format an LLSD to a stream.
+ * @brief Call this method to format an LLSD to a stream with options as
+ * set by the constructor.
+ *
+ * @param data The data to write.
+ * @param ostr The destination stream for the data.
+ * @return Returns The number of LLSD objects formatted out
+ */
+ S32 format(const LLSD& data, std::ostream& ostr) const;
+
+ /**
+ * @brief Call this method to format an LLSD to a stream, passing options
+ * explicitly.
*
* @param data The data to write.
* @param ostr The destination stream for the data.
- * @return Returns The number of LLSD objects fomatted out
+ * @param options OPTIONS_NONE to emit LLSD::Binary as raw bytes
+ * @return Returns The number of LLSD objects formatted out
*/
- virtual S32 format(const LLSD& data, std::ostream& ostr, U32 options = LLSDFormatter::OPTIONS_NONE) const = 0;
+ virtual S32 format(const LLSD& data, std::ostream& ostr, EFormatterOptions options) const;
protected:
+ /**
+ * @brief Implementation to format the data. This is called recursively.
+ *
+ * @param data The data to write.
+ * @param ostr The destination stream for the data.
+ * @return Returns The number of LLSD objects formatted out
+ */
+ virtual S32 format_impl(const LLSD& data, std::ostream& ostr, EFormatterOptions options,
+ U32 level) const = 0;
+
/**
* @brief Helper method which appropriately obeys the real format.
*
@@ -476,9 +499,9 @@ class LL_COMMON_API LLSDFormatter : public LLRefCount
*/
void formatReal(LLSD::Real real, std::ostream& ostr) const;
-protected:
bool mBoolAlpha;
std::string mRealFormat;
+ EFormatterOptions mOptions;
};
@@ -498,7 +521,8 @@ class LL_COMMON_API LLSDNotationFormatter : public LLSDFormatter
/**
* @brief Constructor
*/
- LLSDNotationFormatter();
+ LLSDNotationFormatter(bool boolAlpha=false, const std::string& realFormat="",
+ EFormatterOptions options=OPTIONS_PRETTY_BINARY);
/**
* @brief Helper static method to return a notation escaped string
@@ -512,25 +536,16 @@ class LL_COMMON_API LLSDNotationFormatter : public LLSDFormatter
*/
static std::string escapeString(const std::string& in);
- /**
- * @brief Call this method to format an LLSD to a stream.
- *
- * @param data The data to write.
- * @param ostr The destination stream for the data.
- * @return Returns The number of LLSD objects fomatted out
- */
- virtual S32 format(const LLSD& data, std::ostream& ostr, U32 options = LLSDFormatter::OPTIONS_NONE) const;
-
protected:
-
/**
* @brief Implementation to format the data. This is called recursively.
*
* @param data The data to write.
* @param ostr The destination stream for the data.
- * @return Returns The number of LLSD objects fomatted out
+ * @return Returns The number of LLSD objects formatted out
*/
- S32 format_impl(const LLSD& data, std::ostream& ostr, U32 options, U32 level) const;
+ S32 format_impl(const LLSD& data, std::ostream& ostr, EFormatterOptions options,
+ U32 level) const override;
};
@@ -550,7 +565,8 @@ class LL_COMMON_API LLSDXMLFormatter : public LLSDFormatter
/**
* @brief Constructor
*/
- LLSDXMLFormatter();
+ LLSDXMLFormatter(bool boolAlpha=false, const std::string& realFormat="",
+ EFormatterOptions options=OPTIONS_PRETTY_BINARY);
/**
* @brief Helper static method to return an xml escaped string
@@ -565,20 +581,23 @@ class LL_COMMON_API LLSDXMLFormatter : public LLSDFormatter
*
* @param data The data to write.
* @param ostr The destination stream for the data.
- * @return Returns The number of LLSD objects fomatted out
+ * @return Returns The number of LLSD objects formatted out
*/
- virtual S32 format(const LLSD& data, std::ostream& ostr, U32 options = LLSDFormatter::OPTIONS_NONE) const;
+ S32 format(const LLSD& data, std::ostream& ostr, EFormatterOptions options) const override;
-protected:
+ // also pull down base-class format() method that isn't overridden
+ using LLSDFormatter::format;
+protected:
/**
* @brief Implementation to format the data. This is called recursively.
*
* @param data The data to write.
* @param ostr The destination stream for the data.
- * @return Returns The number of LLSD objects fomatted out
+ * @return Returns The number of LLSD objects formatted out
*/
- S32 format_impl(const LLSD& data, std::ostream& ostr, U32 options, U32 level) const;
+ S32 format_impl(const LLSD& data, std::ostream& ostr, EFormatterOptions options,
+ U32 level) const override;
};
@@ -618,18 +637,20 @@ class LL_COMMON_API LLSDBinaryFormatter : public LLSDFormatter
/**
* @brief Constructor
*/
- LLSDBinaryFormatter();
+ LLSDBinaryFormatter(bool boolAlpha=false, const std::string& realFormat="",
+ EFormatterOptions options=OPTIONS_PRETTY_BINARY);
+protected:
/**
- * @brief Call this method to format an LLSD to a stream.
+ * @brief Implementation to format the data. This is called recursively.
*
* @param data The data to write.
* @param ostr The destination stream for the data.
- * @return Returns The number of LLSD objects fomatted out
+ * @return Returns The number of LLSD objects formatted out
*/
- virtual S32 format(const LLSD& data, std::ostream& ostr, U32 options = LLSDFormatter::OPTIONS_NONE) const;
+ S32 format_impl(const LLSD& data, std::ostream& ostr, EFormatterOptions options,
+ U32 level) const override;
-protected:
/**
* @brief Helper method to serialize strings
*
@@ -669,7 +690,8 @@ class LLSDOStreamer
/**
* @brief Constructor
*/
- LLSDOStreamer(const LLSD& data, U32 options = LLSDFormatter::OPTIONS_NONE) :
+ LLSDOStreamer(const LLSD& data,
+ LLSDFormatter::EFormatterOptions options=LLSDFormatter::OPTIONS_PRETTY_BINARY) :
mSD(data), mOptions(options) {}
/**
@@ -681,17 +703,17 @@ class LLSDOStreamer
* @return Returns the stream passed in after streaming mSD.
*/
friend std::ostream& operator<<(
- std::ostream& str,
- const LLSDOStreamer<Formatter>& formatter)
+ std::ostream& out,
+ const LLSDOStreamer<Formatter>& streamer)
{
LLPointer<Formatter> f = new Formatter;
- f->format(formatter.mSD, str, formatter.mOptions);
- return str;
+ f->format(streamer.mSD, out, streamer.mOptions);
+ return out;
}
protected:
LLSD mSD;
- U32 mOptions;
+ LLSDFormatter::EFormatterOptions mOptions;
};
typedef LLSDOStreamer<LLSDNotationFormatter> LLSDNotationStreamer;
@@ -724,7 +746,7 @@ class LL_COMMON_API LLSDSerialize
* Generic in/outs
*/
static void serialize(const LLSD& sd, std::ostream& str, ELLSD_Serialize,
- U32 options = LLSDFormatter::OPTIONS_NONE);
+ LLSDFormatter::EFormatterOptions options=LLSDFormatter::OPTIONS_PRETTY_BINARY);
/**
* @brief Examine a stream, and parse 1 sd object out based on contents.
@@ -752,9 +774,9 @@ class LL_COMMON_API LLSDSerialize
static S32 toPrettyBinaryNotation(const LLSD& sd, std::ostream& str)
{
LLPointer<LLSDNotationFormatter> f = new LLSDNotationFormatter;
- return f->format(sd, str,
- LLSDFormatter::OPTIONS_PRETTY |
- LLSDFormatter::OPTIONS_PRETTY_BINARY);
+ return f->format(sd, str,
+ LLSDFormatter::EFormatterOptions(LLSDFormatter::OPTIONS_PRETTY |
+ LLSDFormatter::OPTIONS_PRETTY_BINARY));
}
static S32 fromNotation(LLSD& sd, std::istream& str, S32 max_bytes)
{
diff --git a/indra/llcommon/llsdserialize_xml.cpp b/indra/llcommon/llsdserialize_xml.cpp
index 6d0fe862b91..0da824d6945 100644
--- a/indra/llcommon/llsdserialize_xml.cpp
+++ b/indra/llcommon/llsdserialize_xml.cpp
@@ -45,7 +45,9 @@ extern "C"
/**
* LLSDXMLFormatter
*/
-LLSDXMLFormatter::LLSDXMLFormatter()
+LLSDXMLFormatter::LLSDXMLFormatter(bool boolAlpha, const std::string& realFormat,
+ EFormatterOptions options):
+ LLSDFormatter(boolAlpha, realFormat, options)
{
}
@@ -55,7 +57,8 @@ LLSDXMLFormatter::~LLSDXMLFormatter()
}
// virtual
-S32 LLSDXMLFormatter::format(const LLSD& data, std::ostream& ostr, U32 options) const
+S32 LLSDXMLFormatter::format(const LLSD& data, std::ostream& ostr,
+ EFormatterOptions options) const
{
std::streamsize old_precision = ostr.precision(25);
@@ -72,7 +75,8 @@ S32 LLSDXMLFormatter::format(const LLSD& data, std::ostream& ostr, U32 options)
return rv;
}
-S32 LLSDXMLFormatter::format_impl(const LLSD& data, std::ostream& ostr, U32 options, U32 level) const
+S32 LLSDXMLFormatter::format_impl(const LLSD& data, std::ostream& ostr,
+ EFormatterOptions options, U32 level) const
{
S32 format_count = 1;
std::string pre;
diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp
index 6ac974e6593..642c1c38790 100644
--- a/indra/llcommon/tests/llsdserialize_test.cpp
+++ b/indra/llcommon/tests/llsdserialize_test.cpp
@@ -271,10 +271,10 @@ namespace tut
LLSD w;
mParser->reset(); // reset() call is needed since test code re-uses mParser
mParser->parse(stream, w, stream.str().size());
-
+
try
{
- ensure_equals(msg.c_str(), w, v);
+ ensure_equals(msg, w, v);
}
catch (...)
{
@@ -432,6 +432,7 @@ namespace tut
const char source[] = "it must be a blue moon again";
std::vector<U8> data;
+ // note, includes terminating '\0'
copy(&source[0], &source[sizeof(source)], back_inserter(data));
v = data;
@@ -468,28 +469,36 @@ namespace tut
checkRoundTrip(msg + " many nested maps", v);
}
- typedef tut::test_group<TestLLSDSerializeData> TestLLSDSerialzeGroup;
- typedef TestLLSDSerialzeGroup::object TestLLSDSerializeObject;
- TestLLSDSerialzeGroup gTestLLSDSerializeGroup("llsd serialization");
+ typedef tut::test_group<TestLLSDSerializeData> TestLLSDSerializeGroup;
+ typedef TestLLSDSerializeGroup::object TestLLSDSerializeObject;
+ TestLLSDSerializeGroup gTestLLSDSerializeGroup("llsd serialization");
template<> template<>
void TestLLSDSerializeObject::test<1>()
{
- mFormatter = new LLSDNotationFormatter();
+ mFormatter = new LLSDNotationFormatter(false, "", LLSDFormatter::OPTIONS_PRETTY_BINARY);
mParser = new LLSDNotationParser();
- doRoundTripTests("notation serialization");
+ doRoundTripTests("pretty binary notation serialization");
}
-
+
template<> template<>
void TestLLSDSerializeObject::test<2>()
+ {
+ mFormatter = new LLSDNotationFormatter(false, "", LLSDFormatter::OPTIONS_NONE);
+ mParser = new LLSDNotationParser();
+ doRoundTripTests("raw binary notation serialization");
+ }
+
+ template<> template<>
+ void TestLLSDSerializeObject::test<3>()
{
mFormatter = new LLSDXMLFormatter();
mParser = new LLSDXMLParser();
doRoundTripTests("xml serialization");
}
-
+
template<> template<>
- void TestLLSDSerializeObject::test<3>()
+ void TestLLSDSerializeObject::test<4>()
{
mFormatter = new LLSDBinaryFormatter();
mParser = new LLSDBinaryParser();
--
GitLab