[#3211] Checkpoint: rewrote InputBuffer

2 files changed, 152 insertions, 52 deletions

diff --git a/src/lib/util/buffer.h b/src/lib/util/buffer.h
index 4f918566af..aad629740f 100644
--- a/src/lib/util/buffer.h
+++ b/src/lib/util/buffer.h
@@ -1,4 +1,4 @@
-// Copyright (C) 2009-2022 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2009-2024 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -24,10 +24,10 @@ namespace util {
 /// \brief A standard DNS module exception that is thrown if an out-of-range
 /// buffer operation is being performed.
 ///
-class InvalidBufferPosition : public Exception {
+class InvalidBufferPosition : public isc::OutOfRange {
 public:
     InvalidBufferPosition(const char* file, size_t line, const char* what) :
-        isc::Exception(file, line, what) {}
+        isc::OutOfRange(file, line, what) {}
 };
 
 ///\brief The \c InputBuffer class is a buffer abstraction for manipulating
@@ -73,11 +73,18 @@ public:
 /// be a perfect solution, we have adopted what we thought a "least bad" one.
 ///
 /// Methods for reading data from the buffer generally work like an input
+
+/// Methods for reading data from the buffer generally work like an input
 /// stream: it begins with the head of the data, and once some length of data
 /// is read from the buffer, the next read operation will take place from the
 /// head of the unread data.  An object of this class internally holds (a
 /// notion of) where the next read operation should start.  We call it the
-/// <em>read position</em> in this document.
+/// <em>current pointer</em> in this document.
+///
+/// The inequality base_ <= current_ <= end_ is enforced, current_ == base_
+/// at the initial state, current_ == end_ when the whole buffer was read.
+/// Even the difference of two pointers is a std::ptrdiff_t it is safe to
+/// cast to a size_t because of the inequality.
 class InputBuffer {
 public:
     ///
@@ -90,16 +97,23 @@ public:
     /// \param data A pointer to the data stored in the buffer.
     /// \param len The length of the data in bytes.
     InputBuffer(const void* data, size_t len) :
-        position_(0), data_(static_cast<const uint8_t*>(data)), len_(len) {}
+      base_(static_cast<const uint8_t*>(data)), current_(base_),
+      end_(base_ + len) {
+    }
     //@}
 
     ///
     /// \name Getter Methods
     //@{
     /// \brief Return the length of the data stored in the buffer.
-    size_t getLength() const { return (len_); }
+    size_t getLength() const {
+        return (static_cast<size_t>(end_ - base_));
+    }
+
     /// \brief Return the current read position.
-    size_t getPosition() const { return (position_); }
+    size_t getPosition() const {
+        return (static_cast<size_t>(current_ - base_));
+    }
     //@}
 
     ///
@@ -113,69 +127,114 @@ public:
     /// \param position The new position (offset from the beginning of the
     /// buffer).
     void setPosition(size_t position) {
-        if (position > len_) {
+        if (base_ + position > end_) {
             throwError("position is too large");
         }
-        position_ = position;
+        current_ = base_ + position;
     }
     //@}
 
     ///
     /// \name Methods for reading data from the buffer.
     //@{
+    /// \brief Peek an unsigned 8-bit integer from the buffer and return it.
+    ///
+    /// If the remaining length of the buffer is smaller than 8-bit, an
+    /// exception of class \c isc::dns::InvalidBufferPosition will be thrown.
+    uint8_t peekUint8() {
+        if (current_ + sizeof(uint8_t) > end_) {
+            throwError("read beyond end of buffer");
+        }
+
+        return (*current_);
+    }
+
     /// \brief Read an unsigned 8-bit integer from the buffer and return it.
     ///
     /// If the remaining length of the buffer is smaller than 8-bit, an
     /// exception of class \c isc::dns::InvalidBufferPosition will be thrown.
     uint8_t readUint8() {
-        if (position_ + sizeof(uint8_t) > len_) {
+        uint8_t ret = peekUint8();
+        current_ += sizeof(uint8_t);
+
+        return (ret);
+    }
+
+    /// \brief Peek an unsigned 16-bit integer in network byte order from the
+    /// buffer, and return it.
+    ///
+    /// If the remaining length of the buffer is smaller than 16-bit, an
+    /// exception of class \c isc::dns::InvalidBufferPosition will be thrown.
+    uint16_t peekUint16() {
+        if (current_ + sizeof(uint16_t) > end_) {
             throwError("read beyond end of buffer");
         }
 
-        return (data_[position_++]);
+        uint16_t ret;
+        ret = (static_cast<uint16_t>(current_[0])) << 8;
+        ret |= (static_cast<uint16_t>(current_[1]));
+
+        return (ret);
     }
+
     /// \brief Read an unsigned 16-bit integer in network byte order from the
-    /// buffer, convert it to host byte order, and return it.
+    /// buffer, and return it.
     ///
     /// If the remaining length of the buffer is smaller than 16-bit, an
     /// exception of class \c isc::dns::InvalidBufferPosition will be thrown.
     uint16_t readUint16() {
-        uint16_t data;
-        const uint8_t* cp;
+        uint16_t ret = peekUint16();
+        current_ += sizeof(uint16_t);
+
+        return (ret);
+    }
 
-        if (position_ + sizeof(data) > len_) {
+    /// \brief Read an unsigned 32-bit integer in network byte order from the
+    /// buffer, and return it.
+    ///
+    /// If the remaining length of the buffer is smaller than 32-bit, an
+    /// exception of class \c isc::dns::InvalidBufferPosition will be thrown.
+    uint32_t peekUint32() {
+        if (current_ + sizeof(uint32_t) > end_) {
             throwError("read beyond end of buffer");
         }
 
-        cp = &data_[position_];
-        data = ((unsigned int)(cp[0])) << 8;
-        data |= ((unsigned int)(cp[1]));
-        position_ += sizeof(data);
+        uint32_t ret;
+        ret = (static_cast<uint32_t>(current_[0])) << 24;
+        ret |= (static_cast<uint32_t>(current_[1])) << 16;
+        ret |= (static_cast<uint32_t>(current_[2])) << 8;
+        ret |= (static_cast<uint32_t>(current_[3]));
 
-        return (data);
+        return (ret);
     }
+
     /// \brief Read an unsigned 32-bit integer in network byte order from the
-    /// buffer, convert it to host byte order, and return it.
+    /// buffer, and return it.
     ///
     /// If the remaining length of the buffer is smaller than 32-bit, an
     /// exception of class \c isc::dns::InvalidBufferPosition will be thrown.
     uint32_t readUint32() {
-        uint32_t data;
-        const uint8_t* cp;
+        uint32_t ret = peekUint32();
+        current_ += sizeof(uint32_t);
+
+        return (ret);
+    }
 
-        if (position_ + sizeof(data) > len_) {
+    /// \brief Peek data of the specified length from the buffer and copy it to
+    /// the caller supplied buffer.
+    ///
+    /// The data is copied as stored in the buffer; no conversion is performed.
+    /// If the remaining length of the buffer is smaller than the specified
+    /// length, an exception of class \c isc::dns::InvalidBufferPosition will
+    /// be thrown.
+    void peekData(void* data, size_t len) {
+        if (current_ + len > end_) {
             throwError("read beyond end of buffer");
         }
 
-        cp = &data_[position_];
-        data = ((unsigned int)(cp[0])) << 24;
-        data |= ((unsigned int)(cp[1])) << 16;
-        data |= ((unsigned int)(cp[2])) << 8;
-        data |= ((unsigned int)(cp[3]));
-        position_ += sizeof(data);
-
-        return (data);
+        static_cast<void>(std::memmove(data, current_, len));
     }
+
     /// \brief Read data of the specified length from the buffer and copy it to
     /// the caller supplied buffer.
     ///
@@ -184,14 +243,26 @@ public:
     /// length, an exception of class \c isc::dns::InvalidBufferPosition will
     /// be thrown.
     void readData(void* data, size_t len) {
-        if (position_ + len > len_) {
+        peekData(data, len);
+        current_ += len;
+    }
+
+    /// @brief Peek specified number of bytes as a vector.
+    ///
+    /// If specified buffer is too short, it will be expanded
+    /// using vector::resize() method.
+    ///
+    /// @param data Reference to a buffer (data will be stored there).
+    /// @param len Size specified number of bytes to read in a vector.
+    ///
+    void peekVector(std::vector<uint8_t>& data, size_t len) {
+        if (current_ + len > end_) {
             throwError("read beyond end of buffer");
         }
 
-        static_cast<void>(std::memmove(data, &data_[position_], len));
-        position_ += len;
+        data.resize(len);
+        peekData(&data[0], len);
     }
-    //@}
 
     /// @brief Read specified number of bytes as a vector.
     ///
@@ -202,13 +273,10 @@ public:
     /// @param len Size specified number of bytes to read in a vector.
     ///
     void readVector(std::vector<uint8_t>& data, size_t len) {
-        if (position_ + len > len_) {
-            throwError("read beyond end of buffer");
-        }
-
-        data.resize(len);
-        readData(&data[0], len);
+        peekVector(data, len);
+        current_ += len;
     }
+    //@}
 
 private:
     /// \brief A common helper to throw an exception on invalid operation.
@@ -220,14 +288,14 @@ private:
         isc_throw(InvalidBufferPosition, msg);
     }
 
-    size_t position_;
+    /// \brief Base of the buffer.
+    const uint8_t* base_;
+
+    /// \brief Current poisition in the buffer.
+    const uint8_t* current_;
 
-    // XXX: The following must be private, but for a short term workaround with
-    // Boost.Python binding, we changed it to protected.  We should soon
-    // revisit it.
-protected:
-    const uint8_t* data_;
-    size_t len_;
+    /// \brief End of the buffer (address of the byte after).
+    const uint8_t* end_;
 };
 
 ///
diff --git a/src/lib/util/tests/buffer_unittest.cc b/src/lib/util/tests/buffer_unittest.cc
index 1f631673da..0f45569701 100644
--- a/src/lib/util/tests/buffer_unittest.cc
+++ b/src/lib/util/tests/buffer_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2009-2020 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2009-2024 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -41,42 +41,74 @@ protected:
     size_t expected_size;
     uint16_t data16;
     uint32_t data32;
+    std::vector<uint8_t> datav;
 };
 
 const uint8_t BufferTest::testdata[5] = {1, 2, 3, 4, 5};
 
 TEST_F(BufferTest, inputBufferRead) {
     EXPECT_EQ(5, ibuffer.getLength());
+    EXPECT_EQ(1, ibuffer.peekUint8());
+    EXPECT_EQ(0, ibuffer.getPosition());
     EXPECT_EQ(1, ibuffer.readUint8());
     EXPECT_EQ(1, ibuffer.getPosition());
-    data16 = ibuffer.readUint16();
+    data16 = ibuffer.peekUint16();
+    EXPECT_EQ(1, ibuffer.getPosition());
+    EXPECT_EQ(data16, ibuffer.readUint16());
     EXPECT_EQ((2 << 8) | 3, data16);
     EXPECT_EQ(3, ibuffer.getPosition());
     ibuffer.setPosition(1);
     EXPECT_EQ(1, ibuffer.getPosition());
-    data32 = ibuffer.readUint32();
+    data32 = ibuffer.peekUint32();
+    EXPECT_EQ(1, ibuffer.getPosition());
+    EXPECT_EQ(data32, ibuffer.readUint32());
     EXPECT_EQ((2 << 24) | (3 << 16) | (4 << 8) | 5, data32);
     ibuffer.setPosition(0);
     memset(vdata, 0, sizeof(vdata));
+    ibuffer.peekData(vdata, sizeof(vdata));
+    EXPECT_EQ(0, memcmp(vdata, testdata, sizeof(testdata)));
+    EXPECT_EQ(0, ibuffer.getPosition());
+    memset(vdata, 0, sizeof(vdata));
     ibuffer.readData(vdata, sizeof(vdata));
     EXPECT_EQ(0, memcmp(vdata, testdata, sizeof(testdata)));
+    EXPECT_EQ(sizeof(vdata), ibuffer.getPosition());
+    ibuffer.setPosition(0);
+    datav.clear();
+    ibuffer.peekVector(datav, sizeof(vdata));
+    ASSERT_EQ(sizeof(vdata), datav.size());
+    EXPECT_EQ(0, memcmp(&vdata[0], testdata, sizeof(testdata)));
+    EXPECT_EQ(0, ibuffer.getPosition());
+    datav.clear();
+    ibuffer.readVector(datav, sizeof(vdata));
+    ASSERT_EQ(sizeof(vdata), datav.size());
+    EXPECT_EQ(0, memcmp(&vdata[0], testdata, sizeof(testdata)));
+    EXPECT_EQ(sizeof(vdata), ibuffer.getPosition());
 }
 
 TEST_F(BufferTest, inputBufferException) {
     EXPECT_THROW(ibuffer.setPosition(6), isc::util::InvalidBufferPosition);
 
     ibuffer.setPosition(sizeof(testdata));
+    EXPECT_THROW(ibuffer.peekUint8(), isc::util::InvalidBufferPosition);
     EXPECT_THROW(ibuffer.readUint8(), isc::util::InvalidBufferPosition);
 
     ibuffer.setPosition(sizeof(testdata) - 1);
+    EXPECT_THROW(ibuffer.peekUint16(), isc::util::InvalidBufferPosition);
     EXPECT_THROW(ibuffer.readUint16(), isc::util::InvalidBufferPosition);
 
     ibuffer.setPosition(sizeof(testdata) - 3);
+    EXPECT_THROW(ibuffer.peekUint32(), isc::util::InvalidBufferPosition);
     EXPECT_THROW(ibuffer.readUint32(), isc::util::InvalidBufferPosition);
 
     ibuffer.setPosition(sizeof(testdata) - 4);
+    EXPECT_THROW(ibuffer.peekData(vdata, sizeof(vdata)),
+                 isc::util::InvalidBufferPosition);
     EXPECT_THROW(ibuffer.readData(vdata, sizeof(vdata)),
                  isc::util::InvalidBufferPosition);
+    EXPECT_THROW(ibuffer.peekVector(datav, sizeof(vdata)),
+                 isc::util::InvalidBufferPosition);
+    EXPECT_THROW(ibuffer.readVector(datav, sizeof(vdata)),
+                 isc::util::InvalidBufferPosition);
 }
 
 TEST_F(BufferTest, outputBufferExtend) {