From 7d3b3dcb143f20f936f948ef9803bf08665b87d5 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sun, 11 Aug 2024 16:55:38 +0200 Subject: [PATCH] WIP: generalization, more diagnostics and checks --- src/tl/tl/tlProtocolBuffer.cc | 25 ++++++++----- src/tl/tl/tlProtocolBuffer.h | 11 +++++- src/tl/tl/tlProtocolBufferStruct.cc | 15 ++++++++ src/tl/tl/tlProtocolBufferStruct.h | 41 +++++++++++++++++----- src/tl/unit_tests/tlProtocolBufferTests.cc | 8 +++-- 5 files changed, 80 insertions(+), 20 deletions(-) diff --git a/src/tl/tl/tlProtocolBuffer.cc b/src/tl/tl/tlProtocolBuffer.cc index a6a62dab1..b12b8f761 100644 --- a/src/tl/tl/tlProtocolBuffer.cc +++ b/src/tl/tl/tlProtocolBuffer.cc @@ -26,9 +26,6 @@ namespace tl { -// @@@ -// Missing: readers should check for proper wire type (i.e. read(int64)->either VARINT or I64, not I32 - // ---------------------------------------------------------------------------------- ProtocolBufferReader::ProtocolBufferReader (tl::InputStream &input) @@ -92,6 +89,10 @@ ProtocolBufferReader::read (float &f) void ProtocolBufferReader::read (std::string &s) { + if (m_type != PB_LEN) { + error (tl::to_string (tr ("Expected a LEN wire type for a string"))); + } + size_t value = 0; read (value); @@ -109,7 +110,7 @@ ProtocolBufferReader::read (std::string &s) void ProtocolBufferReader::read (uint32_t &ui32) { - if (m_type != PB_I32) { + if (m_type == PB_VARINT || m_type == PB_LEN) { pb_varint ui64 = read_varint (); if (ui64 > std::numeric_limits::max ()) { @@ -118,7 +119,7 @@ ProtocolBufferReader::read (uint32_t &ui32) ui32 = uint32_t (ui64); - } else { + } else if (m_type == PB_I32) { ui32 = 0; const char *cp = get (sizeof (ui32)); @@ -127,6 +128,8 @@ ProtocolBufferReader::read (uint32_t &ui32) ui32 |= (unsigned char) cp [sizeof (ui32) - 1 - i]; } + } else { + error (tl::to_string (tr ("Expected a VARINT or I32 wire type"))); } } @@ -150,11 +153,11 @@ ProtocolBufferReader::read (int32_t &i32) void ProtocolBufferReader::read (uint64_t &ui64) { - if (m_type != PB_I64) { + if (m_type == PB_VARINT || m_type == PB_LEN) { ui64 = read_varint (); - } else { + } else if (m_type == PB_I64) { ui64 = 0; const char *cp = get (sizeof (ui64)); @@ -163,6 +166,8 @@ ProtocolBufferReader::read (uint64_t &ui64) ui64 |= (unsigned char) cp [sizeof (ui64) - 1 - i]; } + } else { + error (tl::to_string (tr ("Expected a VARINT or I64 wire type"))); } } @@ -194,6 +199,10 @@ ProtocolBufferReader::read (bool &b) void ProtocolBufferReader::open () { + if (m_type != PB_LEN) { + error (tl::to_string (tr ("Expected a LEN wire type for a submessage"))); + } + size_t value = 0; read (value); if (! m_seq_counts.empty ()) { @@ -259,7 +268,7 @@ ProtocolBufferReader::read_varint () if (! cp) { error (tl::to_string (tr ("unexpected end of file"))); } - if ((v & 0xfe00000000000000l) != 0) { + if (s + 7 > 64 && pb_varint ((unsigned char) (*cp & 0x7f)) >> (s + 7 - 64) != 0) { error (tl::to_string (tr ("64 bit integer overflow"))); } v |= (pb_varint ((unsigned char) (*cp & 0x7f)) << s); diff --git a/src/tl/tl/tlProtocolBuffer.h b/src/tl/tl/tlProtocolBuffer.h index 0d7d8b4d6..1b8ab25ce 100644 --- a/src/tl/tl/tlProtocolBuffer.h +++ b/src/tl/tl/tlProtocolBuffer.h @@ -176,6 +176,11 @@ public: * @brief Returns true if at the end of the file or end of a block */ virtual bool at_end () const = 0; + + /** + * @brief Emits an error at the current position + */ + virtual void error (const std::string &msg) = 0; }; /** @@ -283,6 +288,11 @@ public: */ bool at_end () const; + /** + * @brief Emits an error at the current position + */ + void error (const std::string &msg); + private: tl::InputStream *mp_stream; PBWireType m_type; @@ -292,7 +302,6 @@ private: pb_varint read_varint (); void skip_bytes (size_t n); - void error (const std::string &msg); const char *get (size_t n); }; diff --git a/src/tl/tl/tlProtocolBufferStruct.cc b/src/tl/tl/tlProtocolBufferStruct.cc index 2df45d4ff..6a675031f 100644 --- a/src/tl/tl/tlProtocolBufferStruct.cc +++ b/src/tl/tl/tlProtocolBufferStruct.cc @@ -74,6 +74,21 @@ PBParser::parse_element (const PBElementBase *parent, tl::ProtocolBufferReaderBa } } +void +PBParser::expect_header (tl::ProtocolBufferReaderBase &reader, int name_tag, const std::string &name) +{ + int tag = reader.read_tag (); + if (tag != name_tag) { + reader.error (tl::sprintf (tl::to_string (tr ("Expected header field with ID %d (got %d)")), name_tag, tag)); + } + + std::string n; + reader.read (n); + if (n != name) { + reader.error (tl::sprintf (tl::to_string (tr ("Expected header field with string '%s' (got '%s')")), name, n)); + } +} + // -------------------------------------------------------------------- // PBElementProxy implementation diff --git a/src/tl/tl/tlProtocolBufferStruct.h b/src/tl/tl/tlProtocolBufferStruct.h index 68fd22e7e..d569622dc 100644 --- a/src/tl/tl/tlProtocolBufferStruct.h +++ b/src/tl/tl/tlProtocolBufferStruct.h @@ -299,6 +299,8 @@ public: void parse (tl::ProtocolBufferReaderBase &reader, const PBElementBase *root, PBReaderState *reader_state); void parse_element (const PBElementBase *parent, tl::ProtocolBufferReaderBase &reader); + void expect_header (tl::ProtocolBufferReaderBase &reader, int name_tag, const std::string &name); + PBReaderState &reader_state () { return *mp_state; @@ -949,9 +951,16 @@ private: /** * @brief The root element of the PB structure * - * The root element is also the handler implementation. - * It must be supplied a pointer to an actual root object, - * a name and a list of children. + * The root object is also a element node. It belongs to an object + * representing the root of the object hierarchy. Correspondingly, + * child elements (members, elements - aka submessages) need to be + * specified. + * + * In addition, the root element has a name and a name tag. + * The name / name tag are emitted as the first element in the + * message, providing a file header with fixed content. This + * allows identifying the file as a encoded protocol buffer + * file for a specific object type. */ template @@ -959,20 +968,20 @@ class TL_PUBLIC_TEMPLATE PBStruct : public PBElementBase { public: - PBStruct (const PBElementList *children) - : PBElementBase (0, children) + PBStruct (const std::string &name, int name_tag, const PBElementList *children) + : PBElementBase (0, children), m_name (name), m_name_tag (name_tag) { // .. nothing yet .. } - PBStruct (const PBElementList &children) - : PBElementBase (0, children) + PBStruct (const std::string &name, int name_tag, const PBElementList &children) + : PBElementBase (0, children), m_name (name), m_name_tag (name_tag) { // .. nothing yet .. } PBStruct (const PBStruct &d) - : PBElementBase (d) + : PBElementBase (d), m_name (d.name ()), m_name_tag (d.name_tag ()) { // .. nothing yet .. } @@ -982,12 +991,22 @@ public: return new PBStruct (*this); } + const std::string &name () const + { + return m_name; + } + + int name_tag () const + { + return m_name_tag; + } + void write (tl::ProtocolBufferWriterBase &writer, const Obj &root) const { PBWriterState writer_state; writer_state.push (& root); - // @@@ writer.write (0, name ()); + writer.write (name_tag (), name ()); for (PBElementBase::iterator c = this->begin (); c != this->end (); ++c) { c->get ()->write (this, writer, writer_state); @@ -1000,6 +1019,7 @@ public: PBReaderState rs; rs.push (&root); PBParser h; + h.expect_header (reader, m_name_tag, m_name); h.parse (reader, this, &rs); rs.pop (tag); tl_assert (rs.empty ()); @@ -1025,6 +1045,9 @@ private: { // disable base class implementation } + + std::string m_name; + int m_name_tag; }; /** diff --git a/src/tl/unit_tests/tlProtocolBufferTests.cc b/src/tl/unit_tests/tlProtocolBufferTests.cc index fb73a859b..919dcb319 100644 --- a/src/tl/unit_tests/tlProtocolBufferTests.cc +++ b/src/tl/unit_tests/tlProtocolBufferTests.cc @@ -214,7 +214,7 @@ TEST (100_BasicStruct) tl::pb_make_member (&Child::d, 2) + tl::pb_make_element (&Child::begin_children, &Child::end_children, &Child::add_child, 3, &child_struct); - tl::PBStruct structure ( + tl::PBStruct structure ("pbtest-struct", 88888888, tl::pb_make_member (&Root::begin_subs, &Root::end_subs, &Root::add_sub, 1) + tl::pb_make_member (&Root::begin_isubs, &Root::end_isubs, &Root::add_isub, 2) + tl::pb_make_element (&Root::begin_children, &Root::end_children, &Root::add_child, 3, &child_struct) + @@ -270,8 +270,12 @@ TEST (100_BasicStruct) { tl::OutputStream os (fn); tl::ProtocolBufferWriter writer (os); - // For development: writer.set_debug (true); structure.write (writer, root); + /* + for debugging: + tl::ProtocolBufferDumper dumper; + structure.write (dumper, root); + */ } root = Root ();