From abe62c23d74d5319691a49881719b03cc9b5b090 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 22 Nov 2022 16:21:44 -0500 Subject: SL-18330: Make LLSDSerialize::deserialize() default to notation. LLSDSerialize::serialize() emits a header string, e.g. "" for notation format. Until now, LLSDSerialize::deserialize() has required that header to properly decode the input stream. But none of LLSDBinaryFormatter, LLSDXMLFormatter or LLSDNotationFormatter emit that header themselves. Nor do any of the Python llsd.format_binary(), format_xml() or format_notation() functions. Until now, you could not use LLSD::deserialize() to parse an arbitrary-format LLSD stream serialized by anything but LLSDSerialize::serialize(). Change LLSDSerialize::deserialize() so that if no header is recognized, instead of failing, it attempts to parse as notation. Add tests to exercise this case. The tricky part about this processing is that deserialize() necessarily reads some number of bytes from the input stream first, to try to recognize the header. If it fails to do so, it must prepend the bytes it has already read to the rest of the input stream since they're probably the beginning of the serialized data. To support this use case, introduce cat_streambuf, a std::streambuf subclass that (virtually) concatenates other std::streambuf instances. When read by a std::istream, the sequence of underlying std::streambufs appears to the consumer as a single continuous stream. --- indra/llcommon/llsdserialize.cpp | 87 ++++++++++++++++++++++++---------------- 1 file changed, 52 insertions(+), 35 deletions(-) (limited to 'indra/llcommon/llsdserialize.cpp') diff --git a/indra/llcommon/llsdserialize.cpp b/indra/llcommon/llsdserialize.cpp index 8b4a0ee6d8..9ff4676b95 100644 --- a/indra/llcommon/llsdserialize.cpp +++ b/indra/llcommon/llsdserialize.cpp @@ -45,6 +45,7 @@ #endif #include "lldate.h" +#include "llmemorystream.h" #include "llsd.h" #include "llstring.h" #include "lluri.h" @@ -61,6 +62,23 @@ const std::string LLSD_NOTATION_HEADER("llsd/notation"); #define windowBits 15 #define ENABLE_ZLIB_GZIP 32 +// If we published this in llsdserialize.h, we could use it in the +// implementation of LLSDOStreamer's operator<<(). +template +void format_using(const LLSD& data, std::ostream& ostr, + LLSDFormatter::EFormatterOptions options=LLSDFormatter::OPTIONS_PRETTY_BINARY) +{ + LLPointer f{ new Formatter }; + f->format(data, ostr, options); +} + +template +S32 parse_using(std::istream& istr, LLSD& data, size_t max_bytes, S32 max_depth=-1) +{ + LLPointer p{ new Parser }; + return p->parse(istr, data, max_bytes, max_depth); +} + /** * LLSDSerialize */ @@ -83,10 +101,10 @@ void LLSDSerialize::serialize(const LLSD& sd, std::ostream& str, ELLSD_Serialize f = new LLSDXMLFormatter; break; - case LLSD_NOTATION: - str << "\n"; - f = new LLSDNotationFormatter; - break; + case LLSD_NOTATION: + str << "\n"; + f = new LLSDNotationFormatter; + break; default: LL_WARNS() << "serialize request for unknown ELLSD_Serialize" << LL_ENDL; @@ -101,10 +119,7 @@ void LLSDSerialize::serialize(const LLSD& sd, std::ostream& str, ELLSD_Serialize // static bool LLSDSerialize::deserialize(LLSD& sd, std::istream& str, S32 max_bytes) { - LLPointer p = NULL; char hdr_buf[MAX_HDR_LEN + 1] = ""; /* Flawfinder: ignore */ - int i; - int inbuf = 0; bool legacy_no_header = false; bool fail_if_not_legacy = false; std::string header; @@ -112,7 +127,10 @@ bool LLSDSerialize::deserialize(LLSD& sd, std::istream& str, S32 max_bytes) /* * Get the first line before anything. */ - str.get(hdr_buf, MAX_HDR_LEN, '\n'); + // Remember str's original input position: if there's no header, we'll + // want to back up and retry. + str.get(hdr_buf, sizeof(hdr_buf), '\n'); + auto inbuf = str.gcount(); if (str.fail()) { str.clear(); @@ -122,16 +140,18 @@ bool LLSDSerialize::deserialize(LLSD& sd, std::istream& str, S32 max_bytes) if (!strncasecmp(LEGACY_NON_HEADER, hdr_buf, strlen(LEGACY_NON_HEADER))) /* Flawfinder: ignore */ { legacy_no_header = true; - inbuf = (int)str.gcount(); } else { if (fail_if_not_legacy) - goto fail; + { + LL_WARNS() << "deserialize LLSD parse failure" << LL_ENDL; + return false; + } /* * Remove the newline chars */ - for (i = 0; i < MAX_HDR_LEN; i++) + for (size_t i = 0; i < sizeof(hdr_buf); i++) { if (hdr_buf[i] == 0 || hdr_buf[i] == '\r' || hdr_buf[i] == '\n') @@ -149,50 +169,47 @@ bool LLSDSerialize::deserialize(LLSD& sd, std::istream& str, S32 max_bytes) { end = header.find_first_of(" ?", start); } - if ((start == std::string::npos) || (end == std::string::npos)) - goto fail; - - header = header.substr(start, end - start); - ws(str); + if (! (start == std::string::npos) || (end == std::string::npos)) + { + header = header.substr(start, end - start); + ws(str); + } } /* * Create the parser as appropriate */ if (legacy_no_header) { // Create a LLSD XML parser, and parse the first chunk read above - LLSDXMLParser* x = new LLSDXMLParser(); - x->parsePart(hdr_buf, inbuf); // Parse the first part that was already read - x->parseLines(str, sd); // Parse the rest of it - delete x; - return true; + LLSDXMLParser x; + x.parsePart(hdr_buf, inbuf); // Parse the first part that was already read + auto parsed = x.parseLines(str, sd); // Parse the rest of it + // Formally we should probably check (parsed != PARSE_FAILURE && + // parsed > 0), but since PARSE_FAILURE is -1, this suffices. + return (parsed > 0); } if (header == LLSD_BINARY_HEADER) { - p = new LLSDBinaryParser; + return (parse_using(str, sd, max_bytes) > 0); } else if (header == LLSD_XML_HEADER) { - p = new LLSDXMLParser; + return (parse_using(str, sd, max_bytes) > 0); } else if (header == LLSD_NOTATION_HEADER) { - p = new LLSDNotationParser; + return (parse_using(str, sd, max_bytes) > 0); } else { - LL_WARNS() << "deserialize request for unknown ELLSD_Serialize" << LL_ENDL; + LL_DEBUGS() << "deserialize request with no header, assuming notation" << LL_ENDL; + // Since we've already read 'inbuf' bytes into 'hdr_buf', prepend that + // data to whatever remains in 'str'. + LLMemoryStreamBuf already(reinterpret_cast(hdr_buf), inbuf); + cat_streambuf prebuff(&already, str.rdbuf()); + std::istream prepend(&prebuff); + return (parse_using(prepend, sd, max_bytes) > 0); } - - if (p.notNull()) - { - p->parse(str, sd, max_bytes); - return true; - } - -fail: - LL_WARNS() << "deserialize LLSD parse failure" << LL_ENDL; - return false; } /** -- cgit v1.3 From 42e0787446d35bf967ff088148f8ad5f902e929b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 23 Nov 2022 17:15:15 -0500 Subject: SL-18330: LLSDSerialize::deserialize() w/o hdr uses XML or notation Absent a header from LLSDSerialize::serialize(), make deserialize() distinguish between XML or notation by recognizing an initial '<'. --- indra/llcommon/llsdserialize.cpp | 87 ++++++++++++++--------------- indra/llcommon/tests/llsdserialize_test.cpp | 28 ++++++++++ 2 files changed, 70 insertions(+), 45 deletions(-) (limited to 'indra/llcommon/llsdserialize.cpp') diff --git a/indra/llcommon/llsdserialize.cpp b/indra/llcommon/llsdserialize.cpp index 9ff4676b95..dc8f8f5737 100644 --- a/indra/llcommon/llsdserialize.cpp +++ b/indra/llcommon/llsdserialize.cpp @@ -120,9 +120,7 @@ void LLSDSerialize::serialize(const LLSD& sd, std::ostream& str, ELLSD_Serialize bool LLSDSerialize::deserialize(LLSD& sd, std::istream& str, S32 max_bytes) { char hdr_buf[MAX_HDR_LEN + 1] = ""; /* Flawfinder: ignore */ - bool legacy_no_header = false; bool fail_if_not_legacy = false; - std::string header; /* * Get the first line before anything. @@ -131,6 +129,7 @@ bool LLSDSerialize::deserialize(LLSD& sd, std::istream& str, S32 max_bytes) // want to back up and retry. str.get(hdr_buf, sizeof(hdr_buf), '\n'); auto inbuf = str.gcount(); + std::string header{ hdr_buf, inbuf }; if (str.fail()) { str.clear(); @@ -138,38 +137,36 @@ bool LLSDSerialize::deserialize(LLSD& sd, std::istream& str, S32 max_bytes) } if (!strncasecmp(LEGACY_NON_HEADER, hdr_buf, strlen(LEGACY_NON_HEADER))) /* Flawfinder: ignore */ + { // Create a LLSD XML parser, and parse the first chunk read above. + LLSDXMLParser x; + x.parsePart(hdr_buf, inbuf); // Parse the first part that was already read + auto parsed = x.parse(str, sd, max_bytes - inbuf); // Parse the rest of it + // Formally we should probably check (parsed != PARSE_FAILURE && + // parsed > 0), but since PARSE_FAILURE is -1, this suffices. + return (parsed > 0); + } + + if (fail_if_not_legacy) { - legacy_no_header = true; + LL_WARNS() << "deserialize LLSD parse failure" << LL_ENDL; + return false; } - else + + /* + * Remove the newline chars + */ + auto lastchar = header.find_last_not_of("\r\n"); + if (lastchar != std::string::npos) { - if (fail_if_not_legacy) - { - LL_WARNS() << "deserialize LLSD parse failure" << LL_ENDL; - return false; - } - /* - * Remove the newline chars - */ - for (size_t i = 0; i < sizeof(hdr_buf); i++) - { - if (hdr_buf[i] == 0 || hdr_buf[i] == '\r' || - hdr_buf[i] == '\n') - { - hdr_buf[i] = 0; - break; - } - } - header = hdr_buf; + header.erase(lastchar+1); + } - std::string::size_type start = std::string::npos; - std::string::size_type end = std::string::npos; - start = header.find_first_not_of(" header syntax + auto start = header.find_first_not_of(" 0), but since PARSE_FAILURE is -1, this suffices. - return (parsed > 0); - } - if (header == LLSD_BINARY_HEADER) { return (parse_using(str, sd, max_bytes) > 0); @@ -200,15 +187,27 @@ bool LLSDSerialize::deserialize(LLSD& sd, std::istream& str, S32 max_bytes) { return (parse_using(str, sd, max_bytes) > 0); } - else + else // no header we recognize { - LL_DEBUGS() << "deserialize request with no header, assuming notation" << LL_ENDL; + LLPointer p; + if (inbuf && hdr_buf[0] == '<') + { + // looks like XML + LL_DEBUGS() << "deserialize request with no header, assuming XML" << LL_ENDL; + p = new LLSDXMLParser; + } + else + { + // assume notation + LL_DEBUGS() << "deserialize request with no header, assuming notation" << LL_ENDL; + p = new LLSDNotationParser; + } // Since we've already read 'inbuf' bytes into 'hdr_buf', prepend that // data to whatever remains in 'str'. LLMemoryStreamBuf already(reinterpret_cast(hdr_buf), inbuf); cat_streambuf prebuff(&already, str.rdbuf()); std::istream prepend(&prebuff); - return (parse_using(prepend, sd, max_bytes) > 0); + return (p->parse(prepend, sd, max_bytes) > 0); } } @@ -2411,5 +2410,3 @@ U8* unzip_llsdNavMesh( bool& valid, unsigned int& outsize, std::istream& is, S32 return result; } - - diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index 0334699c7f..bbbc32214b 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -574,6 +574,34 @@ namespace tut doRoundTripTests("LLSDNotationFormatter -> deserialize"); }; + template<> template<> + void TestLLSDSerializeObject::test<9>() + { + setFormatterParser(new LLSDXMLFormatter(false, "", LLSDFormatter::OPTIONS_NONE), + new LLSDXMLParser()); + setParser(LLSDSerialize::deserialize); + // This is an interesting test because LLSDXMLFormatter does not + // emit an LLSD/XML header. + doRoundTripTests("LLSDXMLFormatter -> deserialize"); + }; + +/*==========================================================================*| + // We do not expect this test to succeed. Without a header, neither + // notation LLSD nor binary LLSD reliably start with a distinct character, + // the way XML LLSD starts with '<'. By convention, we default to notation + // rather than binary. + template<> template<> + void TestLLSDSerializeObject::test<10>() + { + setFormatterParser(new LLSDBinaryFormatter(false, "", LLSDFormatter::OPTIONS_NONE), + new LLSDBinaryParser()); + setParser(LLSDSerialize::deserialize); + // This is an interesting test because LLSDBinaryFormatter does not + // emit an LLSD/Binary header. + doRoundTripTests("LLSDBinaryFormatter -> deserialize"); + }; +|*==========================================================================*/ + /** * @class TestLLSDParsing * @brief Base class for of a parse tester. -- cgit v1.3 From 2f557cd7faceec36acace1eee4ee38904ff06130 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 2 Dec 2022 15:17:56 -0500 Subject: SL-18330: Fix new C++ <-> Python LLSD compatibility tests. When sending multiple LEAP packets in the same file (for testing convenience), use a length prefix instead of delimiting with '\n'. Now that we allow a serialization format that includes an LLSD format header (e.g. ""), '\n' is part of the packet content. But in fact, testing binary LLSD means we can't pick any delimiter guaranteed not to appear in the packet content. Using a length prefix also lets us pass a specific max_bytes to the subject C++ LLSD parser. Make llleap_test.cpp use new freestanding Python llsd package when available. Update Python-side LEAP protocol code to work directly with encoded bytes stream, avoiding bytes<->str encoding and decoding, which breaks binary LLSD. Make LLSDSerialize::deserialize() recognize LLSD format header case- insensitively. Python emits and checks for "llsd/binary", while LLSDSerialize emits and checks for "LLSD/Binary". Once any of the headers is recognized, pass corrected max_bytes to the specific parser. Make deserialize() more careful about the no-header case: preserve '\n' in content. Introduce debugging code (disabled) because it's a little tricky to recreate. Revert LLLeap child process stdout parser from LLSDSerialize::deserialize() to the specific LLSDNotationParser(), as at present: the generic parser fails one of LLLeap's integration tests for reasons that remain mysterious. --- indra/llcommon/llleap.cpp | 9 +++ indra/llcommon/llsdserialize.cpp | 49 +++++++++--- indra/llcommon/tests/llleap_test.cpp | 32 ++++---- indra/llcommon/tests/llsdserialize_test.cpp | 118 +++++++++++++++++++--------- 4 files changed, 147 insertions(+), 61 deletions(-) (limited to 'indra/llcommon/llsdserialize.cpp') diff --git a/indra/llcommon/llleap.cpp b/indra/llcommon/llleap.cpp index f71eaf92c4..e8ccc4300d 100644 --- a/indra/llcommon/llleap.cpp +++ b/indra/llcommon/llleap.cpp @@ -317,8 +317,17 @@ public: LL_DEBUGS("LLLeap") << "needed " << mExpect << " bytes, got " << childout.size() << ", parsing LLSD" << LL_ENDL; LLSD data; +#if 1 + // specifically require notation LLSD from child + LLPointer parser(new LLSDNotationParser()); + S32 parse_status(parser->parse(childout.get_istream(), data, mExpect)); + if (parse_status == LLSDParser::PARSE_FAILURE) +#else + // SL-18330: accept any valid LLSD serialization format from child + // Unfortunately this runs into trouble we have not yet debugged. bool parse_status(LLSDSerialize::deserialize(data, childout.get_istream(), mExpect)); if (! parse_status) +#endif { bad_protocol("unparseable LLSD data"); } diff --git a/indra/llcommon/llsdserialize.cpp b/indra/llcommon/llsdserialize.cpp index dc8f8f5737..97bf51eeaa 100644 --- a/indra/llcommon/llsdserialize.cpp +++ b/indra/llcommon/llsdserialize.cpp @@ -123,12 +123,26 @@ bool LLSDSerialize::deserialize(LLSD& sd, std::istream& str, S32 max_bytes) bool fail_if_not_legacy = false; /* - * Get the first line before anything. + * Get the first line before anything. Don't read more than max_bytes: + * this get() overload reads no more than (count-1) bytes into the + * specified buffer. In the usual case when max_bytes exceeds + * sizeof(hdr_buf), get() will read no more than sizeof(hdr_buf)-2. */ - // Remember str's original input position: if there's no header, we'll - // want to back up and retry. - str.get(hdr_buf, sizeof(hdr_buf), '\n'); + str.get(hdr_buf, std::min(max_bytes+1, sizeof(hdr_buf)-1), '\n'); auto inbuf = str.gcount(); + // https://en.cppreference.com/w/cpp/io/basic_istream/get + // When the get() above sees the specified delimiter '\n', it stops there + // without pulling it from the stream. If it turns out that the stream + // does NOT contain a header, and the content includes meaningful '\n', + // it's important to pull that into hdr_buf too. + if (inbuf < max_bytes && str.get(hdr_buf[inbuf])) + { + // got the delimiting '\n' + ++inbuf; + // None of the following requires that hdr_buf contain a final '\0' + // byte. We could store one if needed, since even the incremented + // inbuf won't exceed sizeof(hdr_buf)-1, but there's no need. + } std::string header{ hdr_buf, inbuf }; if (str.fail()) { @@ -175,17 +189,17 @@ bool LLSDSerialize::deserialize(LLSD& sd, std::istream& str, S32 max_bytes) /* * Create the parser as appropriate */ - if (header == LLSD_BINARY_HEADER) + if (0 == LLStringUtil::compareInsensitive(header, LLSD_BINARY_HEADER)) { - return (parse_using(str, sd, max_bytes) > 0); + return (parse_using(str, sd, max_bytes-inbuf) > 0); } - else if (header == LLSD_XML_HEADER) + else if (0 == LLStringUtil::compareInsensitive(header, LLSD_XML_HEADER)) { - return (parse_using(str, sd, max_bytes) > 0); + return (parse_using(str, sd, max_bytes-inbuf) > 0); } - else if (header == LLSD_NOTATION_HEADER) + else if (0 == LLStringUtil::compareInsensitive(header, LLSD_NOTATION_HEADER)) { - return (parse_using(str, sd, max_bytes) > 0); + return (parse_using(str, sd, max_bytes-inbuf) > 0); } else // no header we recognize { @@ -207,7 +221,22 @@ bool LLSDSerialize::deserialize(LLSD& sd, std::istream& str, S32 max_bytes) LLMemoryStreamBuf already(reinterpret_cast(hdr_buf), inbuf); cat_streambuf prebuff(&already, str.rdbuf()); std::istream prepend(&prebuff); +#if 1 return (p->parse(prepend, sd, max_bytes) > 0); +#else + // debugging the reconstituted 'prepend' stream + // allocate a buffer that we hope is big enough for the whole thing + std::vector wholemsg((max_bytes == size_t(SIZE_UNLIMITED))? 1024 : max_bytes); + prepend.read(wholemsg.data(), std::min(max_bytes, wholemsg.size())); + LLMemoryStream replay(reinterpret_cast(wholemsg.data()), prepend.gcount()); + auto success{ p->parse(replay, sd, prepend.gcount()) > 0 }; + { + LL_DEBUGS() << (success? "parsed: $$" : "failed: '") + << std::string(wholemsg.data(), llmin(prepend.gcount(), 100)) << "$$" + << LL_ENDL; + } + return success; +#endif } } diff --git a/indra/llcommon/tests/llleap_test.cpp b/indra/llcommon/tests/llleap_test.cpp index 9754353ab0..6c799b7993 100644 --- a/indra/llcommon/tests/llleap_test.cpp +++ b/indra/llcommon/tests/llleap_test.cpp @@ -110,12 +110,12 @@ namespace tut "import os\n" "import sys\n" "\n" - // Don't forget that this Python script is written to some - // temp directory somewhere! Its __file__ is useless in - // finding indra/lib/python. Use our __FILE__, with - // raw-string syntax to deal with Windows pathnames. - "mydir = os.path.dirname(r'" << __FILE__ << "')\n" - "from llbase import llsd\n" + "try:\n" + // new freestanding llsd package + " import llsd\n" + "except ImportError:\n" + // older llbase.llsd module + " from llbase import llsd\n" "\n" "class ProtocolError(Exception):\n" " def __init__(self, msg, data):\n" @@ -126,26 +126,26 @@ namespace tut " pass\n" "\n" "def get():\n" - " hdr = ''\n" - " while ':' not in hdr and len(hdr) < 20:\n" - " hdr += sys.stdin.read(1)\n" + " hdr = []\n" + " while b':' not in hdr and len(hdr) < 20:\n" + " hdr.append(sys.stdin.buffer.read(1))\n" " if not hdr:\n" " sys.exit(0)\n" - " if not hdr.endswith(':'):\n" + " if not hdr[-1] == b':':\n" " raise ProtocolError('Expected len:data, got %r' % hdr, hdr)\n" " try:\n" - " length = int(hdr[:-1])\n" + " length = int(b''.join(hdr[:-1]))\n" " except ValueError:\n" " raise ProtocolError('Non-numeric len %r' % hdr[:-1], hdr[:-1])\n" " parts = []\n" " received = 0\n" " while received < length:\n" - " parts.append(sys.stdin.read(length - received))\n" + " parts.append(sys.stdin.buffer.read(length - received))\n" " received += len(parts[-1])\n" - " data = ''.join(parts)\n" + " data = b''.join(parts)\n" " assert len(data) == length\n" " try:\n" - " return llsd.parse(data.encode())\n" + " return llsd.parse(data)\n" // Seems the old indra.base.llsd module didn't properly // convert IndexError (from running off end of string) to // LLSDParseError. @@ -185,11 +185,11 @@ namespace tut " return _reply\n" "\n" "def put(req):\n" - " sys.stdout.write(':'.join((str(len(req)), req)))\n" + " sys.stdout.buffer.write(b'%d:%b' % (len(req), req))\n" " sys.stdout.flush()\n" "\n" "def send(pump, data):\n" - " put(llsd.format_notation(dict(pump=pump, data=data)).decode())\n" + " put(llsd.format_notation(dict(pump=pump, data=data)))\n" "\n" "def request(pump, data):\n" " # we expect 'data' is a dict\n" diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index 601f2c580d..39b93cf335 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -51,10 +51,11 @@ typedef U32 uint32_t; #include "boost/phoenix/core/argument.hpp" using namespace boost::phoenix; -#include "../llsd.h" -#include "../llsdserialize.h" +#include "llsd.h" +#include "llsdserialize.h" #include "llsdutil.h" -#include "../llformat.h" +#include "llformat.h" +#include "llmemorystream.h" #include "../test/lltut.h" #include "../test/namedtempfile.h" @@ -1898,14 +1899,22 @@ namespace tut static void writeLLSDArray(const FormatterFunction& serialize, std::ostream& out, const LLSD& array) { - BOOST_FOREACH(LLSD item, llsd::inArray(array)) + for (const LLSD& item : llsd::inArray(array)) { - serialize(item, out); - // It's important to separate with newlines because Python's llsd - // module doesn't support parsing from a file stream, only from a - // string, so we have to know how much of the file to read into a - // string. - out << '\n'; + // It's important to delimit the entries in this file somehow + // because, although Python's llsd.parse() can accept a file + // stream, the XML parser expects EOF after a single outer element + // -- it doesn't just stop. So we must extract a sequence of bytes + // strings from the file. But since one of the serialization + // formats we want to test is binary, we can't pick any single + // byte value as a delimiter! Use a binary integer length prefix + // instead. + std::ostringstream buffer; + serialize(item, buffer); + auto buffstr{ buffer.str() }; + int bufflen{ static_cast(buffstr.length()) }; + out.write(reinterpret_cast(&bufflen), sizeof(bufflen)); + out.write(buffstr.c_str(), buffstr.length()); } } @@ -1932,7 +1941,7 @@ namespace tut " except StopIteration:\n" " pass\n" " else:\n" - " assert False, 'Too many data items'\n"; + " raise AssertionError('Too many data items')\n"; // Create an llsdXXXXXX file containing 'data' serialized to // notation. @@ -1948,10 +1957,23 @@ namespace tut python("read C++ " + desc, placeholders::arg1 << import_llsd << - "def parse_each(iterable):\n" - " for item in iterable:\n" - " yield llsd.parse(item)\n" << - pydata << + "from functools import partial\n" + "import struct\n" + "lenformat = struct.Struct('i')\n" + "def parse_each(inf):\n" + " for rawlen in iter(partial(inf.read, lenformat.size), b''):\n" + " len = lenformat.unpack(rawlen)[0]\n" + // Since llsd.parse() has no max_bytes argument, instead of + // passing the input stream directly to parse(), read the item + // into a distinct bytes object and parse that. + " data = inf.read(len)\n" + " try:\n" + " yield llsd.parse(data)\n" + " except llsd.LLSDParseError as err:\n" + " print(f'*** {err}')\n" + " print(f'Bad content:\\n{data!r}')\n" + " raise\n" + << pydata << // Don't forget raw-string syntax for Windows pathnames. "verify(parse_each(open(r'" << file.getName() << "', 'rb')))\n"); } @@ -2008,6 +2030,26 @@ namespace tut } |*==========================================================================*/ + // helper for test<8> - test<12> + bool itemFromStream(std::istream& istr, LLSD& item, const ParserFunction& parse) + { + // reset the output value for debugging clarity + item.clear(); + // We use an int length prefix as a foolproof delimiter even for + // binary serialized streams. + int length{ 0 }; + istr.read(reinterpret_cast(&length), sizeof(length)); +// return parse(istr, item, length); + // Sadly, as of 2022-12-01 it seems we can't really trust our LLSD + // parsers to honor max_bytes: this test works better when we read + // each item into its own distinct LLMemoryStream, instead of passing + // the original istr with a max_bytes constraint. + std::vector buffer(length); + istr.read(reinterpret_cast(buffer.data()), length); + LLMemoryStream stream(buffer.data(), length); + return parse(stream, item, length); + } + // helper for test<8> - test<12> void fromPythonUsing(const std::string& pyformatter, const ParserFunction& parse= @@ -2022,6 +2064,8 @@ namespace tut python("Python " + pyformatter, placeholders::arg1 << import_llsd << + "import struct\n" + "lenformat = struct.Struct('i')\n" "DATA = [\n" " 17,\n" " 3.14,\n" @@ -2034,29 +2078,33 @@ namespace tut // N.B. Using 'print' implicitly adds newlines. "with open(r'" << file.getName() << "', 'wb') as f:\n" " for item in DATA:\n" - " print(llsd." << pyformatter << "(item), file=f)\n"); + " serialized = llsd." << pyformatter << "(item)\n" + " f.write(lenformat.pack(len(serialized)))\n" + " f.write(serialized)\n"); std::ifstream inf(file.getName().c_str()); LLSD item; - // Notice that we're not doing anything special to parse out the - // newlines: LLSDSerialize::fromNotation ignores them. While it would - // seem they're not strictly necessary, going in this direction, we - // want to ensure that notation-separated-by-newlines works in both - // directions -- since in practice, a given file might be read by - // either language. - ensure("Failed to read LLSD::Integer from Python", - parse(inf, item, LLSDSerialize::SIZE_UNLIMITED)); - ensure_equals(item.asInteger(), 17); - ensure("Failed to read LLSD::Real from Python", - parse(inf, item, LLSDSerialize::SIZE_UNLIMITED)); - ensure_approximately_equals("Bad LLSD::Real value from Python", - item.asReal(), 3.14, 7); // 7 bits ~= 0.01 - ensure("Failed to read LLSD::String from Python", - parse(inf, item, LLSDSerialize::SIZE_UNLIMITED)); - ensure_equals(item.asString(), - "This string\n" - "has several\n" - "lines."); + try + { + ensure("Failed to read LLSD::Integer from Python", + itemFromStream(inf, item, parse)); + ensure_equals(item.asInteger(), 17); + ensure("Failed to read LLSD::Real from Python", + itemFromStream(inf, item, parse)); + ensure_approximately_equals("Bad LLSD::Real value from Python", + item.asReal(), 3.14, 7); // 7 bits ~= 0.01 + ensure("Failed to read LLSD::String from Python", + itemFromStream(inf, item, parse)); + ensure_equals(item.asString(), + "This string\n" + "has several\n" + "lines."); + } + catch (const tut::failure& err) + { + std::cout << "for " << err.what() << ", item = " << item << std::endl; + throw; + } } template<> template<> -- cgit v1.3 From 590e158cf36b6e6fca07f54837db084fe8a163c6 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 6 Dec 2022 16:30:35 -0500 Subject: SL-18330: Adapt LLSDSerialize and tests to llssize max_bytes params. --- indra/llcommon/llsdserialize.cpp | 2 +- indra/llcommon/tests/llsdserialize_test.cpp | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) (limited to 'indra/llcommon/llsdserialize.cpp') diff --git a/indra/llcommon/llsdserialize.cpp b/indra/llcommon/llsdserialize.cpp index 97bf51eeaa..c1b24d8138 100644 --- a/indra/llcommon/llsdserialize.cpp +++ b/indra/llcommon/llsdserialize.cpp @@ -128,7 +128,7 @@ bool LLSDSerialize::deserialize(LLSD& sd, std::istream& str, S32 max_bytes) * specified buffer. In the usual case when max_bytes exceeds * sizeof(hdr_buf), get() will read no more than sizeof(hdr_buf)-2. */ - str.get(hdr_buf, std::min(max_bytes+1, sizeof(hdr_buf)-1), '\n'); + str.get(hdr_buf, llmin(max_bytes+1, sizeof(hdr_buf)-1), '\n'); auto inbuf = str.gcount(); // https://en.cppreference.com/w/cpp/io/basic_istream/get // When the get() above sees the specified delimiter '\n', it stops there diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index 6e5d8a26e1..29e3007aff 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -63,7 +63,7 @@ using namespace boost::phoenix; #include typedef std::function FormatterFunction; -typedef std::function ParserFunction; +typedef std::function ParserFunction; std::vector string_to_vector(const std::string& str) { @@ -263,7 +263,7 @@ namespace tut }; // this lambda must be mutable since otherwise the bound 'parser' // is assumed to point to a const LLSDParser - mParser = [parser](std::istream& istr, LLSD& data, size_t max_bytes) mutable + mParser = [parser](std::istream& istr, LLSD& data, llssize max_bytes) mutable { // reset() call is needed since test code re-uses parser object parser->reset(); @@ -271,10 +271,10 @@ namespace tut }; } - void setParser(bool (*parser)(LLSD&, std::istream&, size_t)) + void setParser(bool (*parser)(LLSD&, std::istream&, llssize)) { // why does LLSDSerialize::deserialize() reverse the parse() params?? - mParser = [parser](std::istream& istr, LLSD& data, size_t max_bytes) + mParser = [parser](std::istream& istr, LLSD& data, llssize max_bytes) { return (parser(data, istr, max_bytes) > 0); }; @@ -2059,7 +2059,7 @@ namespace tut // helper for test<8> - test<12> void fromPythonUsing(const std::string& pyformatter, const ParserFunction& parse= - [](std::istream& istr, LLSD& data, size_t max_bytes) + [](std::istream& istr, LLSD& data, llssize max_bytes) { return LLSDSerialize::deserialize(data, istr, max_bytes); }) { // Create an empty data file. This is just a placeholder for our @@ -2140,7 +2140,7 @@ namespace tut set_test_name("from Python XML using fromXML()"); // fromXML()'s optional 3rd param isn't max_bytes, it's emit_errors fromPythonUsing("format_xml", - [](std::istream& istr, LLSD& data, size_t) + [](std::istream& istr, LLSD& data, llssize) { return LLSDSerialize::fromXML(data, istr) > 0; }); } @@ -2149,7 +2149,7 @@ namespace tut { set_test_name("from Python notation using fromNotation()"); fromPythonUsing("format_notation", - [](std::istream& istr, LLSD& data, size_t max_bytes) + [](std::istream& istr, LLSD& data, llssize max_bytes) { return LLSDSerialize::fromNotation(data, istr, max_bytes) > 0; }); } @@ -2161,7 +2161,7 @@ namespace tut // We don't expect this to work because format_binary() emits a // header, but fromBinary() won't recognize a header. fromPythonUsing("format_binary", - [](std::istream& istr, LLSD& data, size_t max_bytes) + [](std::istream& istr, LLSD& data, llssize max_bytes) { return LLSDSerialize::fromBinary(data, istr, max_bytes) > 0; }); } |*==========================================================================*/ -- cgit v1.3 From 292bb3991b589d39d61cf721b82fe7bdae460785 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 15 Feb 2023 17:29:48 -0500 Subject: SL-18330: Fix per PR review comments. --- indra/llcommon/llsdserialize.cpp | 6 +++++- indra/llcommon/llstreamtools.cpp | 2 +- indra/llcommon/llstreamtools.h | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) (limited to 'indra/llcommon/llsdserialize.cpp') diff --git a/indra/llcommon/llsdserialize.cpp b/indra/llcommon/llsdserialize.cpp index 0a5e3652c7..a14a6b5b1b 100644 --- a/indra/llcommon/llsdserialize.cpp +++ b/indra/llcommon/llsdserialize.cpp @@ -169,9 +169,13 @@ bool LLSDSerialize::deserialize(LLSD& sd, std::istream& str, llssize max_bytes) /* * Remove the newline chars */ - auto lastchar = header.find_last_not_of("\r\n"); + std::string::size_type lastchar = header.find_last_not_of("\r\n"); if (lastchar != std::string::npos) { + // It's important that find_last_not_of() returns size_type, which is + // why lastchar explicitly declares the type above. erase(size_type) + // erases from that offset to the end of the string, whereas + // erase(iterator) erases only a single character. header.erase(lastchar+1); } diff --git a/indra/llcommon/llstreamtools.cpp b/indra/llcommon/llstreamtools.cpp index 979e96b848..bc32b6fd9e 100644 --- a/indra/llcommon/llstreamtools.cpp +++ b/indra/llcommon/llstreamtools.cpp @@ -519,7 +519,7 @@ int cat_streambuf::underflow() if (gptr() == egptr()) { // here because our buffer is empty - std::streamsize size; + std::streamsize size = 0; // Until we've run out of mInputs, try reading the first of them // into mBuffer. If that fetches some characters, break the loop. while (! mInputs.empty() diff --git a/indra/llcommon/llstreamtools.h b/indra/llcommon/llstreamtools.h index 997f738840..bb7bc20327 100644 --- a/indra/llcommon/llstreamtools.h +++ b/indra/llcommon/llstreamtools.h @@ -135,7 +135,7 @@ public: mBuffer(1024) {} - int underflow(); + int underflow() override; }; #endif -- cgit v1.3 From c18f0a61abfe8b9527f9a91b0e52e909cf7debfc Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 1 Mar 2023 14:35:04 -0500 Subject: SL-18330: Fix a narrowing conversion in LLSDSerialize::deserialize(). --- indra/llcommon/llsdserialize.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon/llsdserialize.cpp') diff --git a/indra/llcommon/llsdserialize.cpp b/indra/llcommon/llsdserialize.cpp index a14a6b5b1b..99d0c39694 100644 --- a/indra/llcommon/llsdserialize.cpp +++ b/indra/llcommon/llsdserialize.cpp @@ -143,7 +143,7 @@ bool LLSDSerialize::deserialize(LLSD& sd, std::istream& str, llssize max_bytes) // byte. We could store one if needed, since even the incremented // inbuf won't exceed sizeof(hdr_buf)-1, but there's no need. } - std::string header{ hdr_buf, inbuf }; + std::string header{ hdr_buf, narrow(inbuf) }; if (str.fail()) { str.clear(); -- cgit v1.3 From 6cb6385bc71417c1017dc5ccabe53a678e371684 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 1 Mar 2023 16:37:55 -0500 Subject: SL-18330: Tweaks for Visual Studio builds --- indra/llcommon/llsdserialize.cpp | 2 +- indra/llcommon/tests/llsdserialize_test.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llcommon/llsdserialize.cpp') diff --git a/indra/llcommon/llsdserialize.cpp b/indra/llcommon/llsdserialize.cpp index 99d0c39694..046523dbb1 100644 --- a/indra/llcommon/llsdserialize.cpp +++ b/indra/llcommon/llsdserialize.cpp @@ -143,7 +143,7 @@ bool LLSDSerialize::deserialize(LLSD& sd, std::istream& str, llssize max_bytes) // byte. We could store one if needed, since even the incremented // inbuf won't exceed sizeof(hdr_buf)-1, but there's no need. } - std::string header{ hdr_buf, narrow(inbuf) }; + std::string header{ hdr_buf, static_cast(inbuf) }; if (str.fail()) { str.clear(); diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index 29e3007aff..618f33cc13 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -276,7 +276,7 @@ namespace tut // why does LLSDSerialize::deserialize() reverse the parse() params?? mParser = [parser](std::istream& istr, LLSD& data, llssize max_bytes) { - return (parser(data, istr, max_bytes) > 0); + return parser(data, istr, max_bytes); }; } -- cgit v1.3 From 22a07b1121bb429e6bfe10b5a5e859bcea1262b6 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 3 May 2023 12:48:22 -0400 Subject: SL-18330: fix strip_deprecated_header(cur_size) param to llssize. --- indra/llcommon/llsdserialize.cpp | 2 +- indra/llcommon/llsdserialize.h | 2 +- indra/newview/llmeshrepository.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) (limited to 'indra/llcommon/llsdserialize.cpp') diff --git a/indra/llcommon/llsdserialize.cpp b/indra/llcommon/llsdserialize.cpp index a510b73096..32d0847141 100644 --- a/indra/llcommon/llsdserialize.cpp +++ b/indra/llcommon/llsdserialize.cpp @@ -2388,7 +2388,7 @@ U8* unzip_llsdNavMesh( bool& valid, unsigned int& outsize, std::istream& is, S32 return result; } -char* strip_deprecated_header(char* in, U32& cur_size, U32* header_size) +char* strip_deprecated_header(char* in, llssize& cur_size, U32* header_size) { const char* deprecated_header = ""; constexpr size_t deprecated_header_size = 17; diff --git a/indra/llcommon/llsdserialize.h b/indra/llcommon/llsdserialize.h index d33d2b6f34..4f596856de 100644 --- a/indra/llcommon/llsdserialize.h +++ b/indra/llcommon/llsdserialize.h @@ -873,5 +873,5 @@ LL_COMMON_API std::string zip_llsd(LLSD& data); LL_COMMON_API U8* unzip_llsdNavMesh( bool& valid, unsigned int& outsize,std::istream& is, S32 size); // returns a pointer to the array or past the array if the deprecated header exists -LL_COMMON_API char* strip_deprecated_header(char* in, U32& cur_size, U32* header_size = nullptr); +LL_COMMON_API char* strip_deprecated_header(char* in, llssize& cur_size, U32* header_size = nullptr); #endif // LL_LLSDSERIALIZE_H diff --git a/indra/newview/llmeshrepository.cpp b/indra/newview/llmeshrepository.cpp index f937754368..18420a2356 100644 --- a/indra/newview/llmeshrepository.cpp +++ b/indra/newview/llmeshrepository.cpp @@ -1865,7 +1865,7 @@ EMeshProcessingResult LLMeshRepoThread::headerReceived(const LLVolumeParams& mes U32 header_size = 0; if (data_size > 0) { - U32 dsize = data_size; + llssize dsize = data_size; char* result_ptr = strip_deprecated_header((char*)data, dsize, &header_size); data_size = dsize; -- cgit v1.3 From 626bbe5032cab1fd2641ad02443e421567e7cce5 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 22 May 2023 19:14:48 -0400 Subject: SL-18837: Fix call to strip_deprecated_header(). Pass llssize instead of S32. --- indra/llcommon/llsdserialize.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llcommon/llsdserialize.cpp') diff --git a/indra/llcommon/llsdserialize.cpp b/indra/llcommon/llsdserialize.cpp index ad3f61ce71..c8dbcb2404 100644 --- a/indra/llcommon/llsdserialize.cpp +++ b/indra/llcommon/llsdserialize.cpp @@ -2193,9 +2193,9 @@ LLUZipHelper::EZipRresult LLUZipHelper::unzip_llsd(LLSD& data, std::istream& is, LLUZipHelper::EZipRresult LLUZipHelper::unzip_llsd(LLSD& data, const U8* in, S32 size) { U8* result = NULL; - U32 cur_size = 0; + llssize cur_size = 0; z_stream strm; - + constexpr U32 CHUNK = 1024 * 512; static thread_local std::unique_ptr out; -- cgit v1.3 From 55460b49585ceea5d6388204c7f3bd74ed4f4827 Mon Sep 17 00:00:00 2001 From: simon Date: Fri, 12 May 2023 17:29:41 +0100 Subject: SL-19711 - viewer can't parse environments. Fixed max parsing bug (cherry picked from commit eb0516b9940f200b32349d611f38f1ccee48005d) --- indra/llcommon/llsdserialize.cpp | 20 +++++++++++++------- indra/newview/llsettingsvo.cpp | 4 ++-- 2 files changed, 15 insertions(+), 9 deletions(-) (limited to 'indra/llcommon/llsdserialize.cpp') diff --git a/indra/llcommon/llsdserialize.cpp b/indra/llcommon/llsdserialize.cpp index e7482b601d..56cc141f0d 100644 --- a/indra/llcommon/llsdserialize.cpp +++ b/indra/llcommon/llsdserialize.cpp @@ -125,14 +125,20 @@ bool LLSDSerialize::deserialize(LLSD& sd, std::istream& str, llssize max_bytes) char hdr_buf[MAX_HDR_LEN + 1] = ""; /* Flawfinder: ignore */ bool fail_if_not_legacy = false; - /* - * Get the first line before anything. Don't read more than max_bytes: - * this get() overload reads no more than (count-1) bytes into the - * specified buffer. In the usual case when max_bytes exceeds - * sizeof(hdr_buf), get() will read no more than sizeof(hdr_buf)-2. - */ - str.get(hdr_buf, llmin(max_bytes+1, sizeof(hdr_buf)-1), '\n'); + /* + * Get the first line before anything. Don't read more than max_bytes: + * this get() overload reads no more than (count-1) bytes into the + * specified buffer. In the usual case when max_bytes exceeds + * sizeof(hdr_buf), get() will read no more than sizeof(hdr_buf)-2. + */ + llssize max_hdr_read = MAX_HDR_LEN; + if (max_bytes != LLSDSerialize::SIZE_UNLIMITED) + { + max_hdr_read = llmin(max_bytes + 1, max_hdr_read); + } + str.get(hdr_buf, max_hdr_read, '\n'); auto inbuf = str.gcount(); + // https://en.cppreference.com/w/cpp/io/basic_istream/get // When the get() above sees the specified delimiter '\n', it stops there // without pulling it from the stream. If it turns out that the stream diff --git a/indra/newview/llsettingsvo.cpp b/indra/newview/llsettingsvo.cpp index b1eed81476..f5212a3026 100644 --- a/indra/newview/llsettingsvo.cpp +++ b/indra/newview/llsettingsvo.cpp @@ -312,7 +312,7 @@ void LLSettingsVOBase::onAssetDownloadComplete(const LLUUID &asset_id, S32 statu std::stringstream llsdstream(buffer); LLSD llsdsettings; - if (LLSDSerialize::deserialize(llsdsettings, llsdstream, -1)) + if (LLSDSerialize::deserialize(llsdsettings, llsdstream, LLSDSerialize::SIZE_UNLIMITED)) { settings = createFromLLSD(llsdsettings); } @@ -379,7 +379,7 @@ LLSettingsBase::ptr_t LLSettingsVOBase::importFile(const std::string &filename) return LLSettingsBase::ptr_t(); } - if (!LLSDSerialize::deserialize(settings, file, -1)) + if (!LLSDSerialize::deserialize(settings, file, LLSDSerialize::SIZE_UNLIMITED)) { LL_WARNS("SETTINGS") << "Unable to deserialize settings from '" << filename << "'" << LL_ENDL; return LLSettingsBase::ptr_t(); -- cgit v1.3 From 3b13c0ca15e2bc413731b0be07db6446bb32df1b Mon Sep 17 00:00:00 2001 From: Andrey Lihatskiy Date: Fri, 26 May 2023 19:06:07 +0300 Subject: DRTVWR-582 xcode-14.3 merge follow up --- indra/llcommon/llsdserialize.cpp | 2 +- indra/llcommon/llsdserialize.h | 2 +- indra/newview/llmeshrepository.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) (limited to 'indra/llcommon/llsdserialize.cpp') diff --git a/indra/llcommon/llsdserialize.cpp b/indra/llcommon/llsdserialize.cpp index a904ce700e..3db456ddb3 100644 --- a/indra/llcommon/llsdserialize.cpp +++ b/indra/llcommon/llsdserialize.cpp @@ -2443,7 +2443,7 @@ U8* unzip_llsdNavMesh( bool& valid, size_t& outsize, std::istream& is, S32 size return result; } -char* strip_deprecated_header(char* in, llssize& cur_size, U32* header_size) +char* strip_deprecated_header(char* in, llssize& cur_size, llssize* header_size) { const char* deprecated_header = ""; constexpr size_t deprecated_header_size = 17; diff --git a/indra/llcommon/llsdserialize.h b/indra/llcommon/llsdserialize.h index 5ddf0ff552..676b7bfd6a 100644 --- a/indra/llcommon/llsdserialize.h +++ b/indra/llcommon/llsdserialize.h @@ -873,5 +873,5 @@ LL_COMMON_API std::string zip_llsd(LLSD& data); LL_COMMON_API U8* unzip_llsdNavMesh( bool& valid, size_t& outsize,std::istream& is, S32 size); // returns a pointer to the array or past the array if the deprecated header exists -LL_COMMON_API char* strip_deprecated_header(char* in, llssize& cur_size, U32* header_size = nullptr); +LL_COMMON_API char* strip_deprecated_header(char* in, llssize& cur_size, llssize* header_size = nullptr); #endif // LL_LLSDSERIALIZE_H diff --git a/indra/newview/llmeshrepository.cpp b/indra/newview/llmeshrepository.cpp index ff888a5af2..d6eba6b4bf 100644 --- a/indra/newview/llmeshrepository.cpp +++ b/indra/newview/llmeshrepository.cpp @@ -1863,7 +1863,7 @@ EMeshProcessingResult LLMeshRepoThread::headerReceived(const LLVolumeParams& mes const LLUUID mesh_id = mesh_params.getSculptID(); LLSD header; - U32 header_size = 0; + llssize header_size = 0; if (data_size > 0) { llssize dsize = data_size; -- cgit v1.3