From 2317eafef2bde742f86e6e8f41ba65e2bacf7596 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Mon, 11 Mar 2024 18:13:13 +0100 Subject: [PATCH] [#3211] Replaced InvalidBufferPosition exception by OutOfRange --- src/lib/dns/messagerenderer.h | 3 +- src/lib/dns/rdata.h | 8 +-- src/lib/dns/tests/message_unittest.cc | 18 ++--- src/lib/dns/tests/rdata_opt_unittest.cc | 2 +- src/lib/dns/tests/rdata_tkey_unittest.cc | 4 +- src/lib/dns/tests/rdata_tsig_unittest.cc | 4 +- src/lib/dns/tests/rdata_unittest.cc | 2 +- src/lib/util/buffer.h | 83 +++++++++++------------- 8 files changed, 58 insertions(+), 66 deletions(-) diff --git a/src/lib/dns/messagerenderer.h b/src/lib/dns/messagerenderer.h index 7ee2277449..d8e0fe2d82 100644 --- a/src/lib/dns/messagerenderer.h +++ b/src/lib/dns/messagerenderer.h @@ -283,8 +283,7 @@ public: /// /// The buffer must have a sufficient room to store the given data at the /// given position, that is, pos + 2 < getLength(); - /// otherwise an exception of class \c isc::dns::InvalidBufferPosition will - /// be thrown. + /// otherwise an exception of class \c isc::OutOfRange will be thrown. /// Note also that this method never extends the internal buffer. /// /// \param data The 16-bit integer to be written into the internal buffer. diff --git a/src/lib/dns/rdata.h b/src/lib/dns/rdata.h index 0ee7be8c8e..17a9a47583 100644 --- a/src/lib/dns/rdata.h +++ b/src/lib/dns/rdata.h @@ -277,10 +277,10 @@ public: /// /// \c rdata_len must not exceed \c MAX_RDLENGTH; otherwise, an exception /// of class \c InvalidRdataLength will be thrown. - /// If resource allocation to hold the data fails, a corresponding standard - /// exception will be thrown; if the \c buffer doesn't contain \c rdata_len - /// bytes of unread data, an exception of class \c InvalidBufferPosition - /// will be thrown. + /// If resource allocation to hold the data fails, a corresponding + /// standard exception will be thrown; if the \c buffer doesn't + /// contain \c rdata_len bytes of unread data, an exception of + /// class \c isc::OutOfRange will be thrown. /// /// \param buffer A reference to an \c InputBuffer object storing the /// \c Rdata to parse. diff --git a/src/lib/dns/tests/message_unittest.cc b/src/lib/dns/tests/message_unittest.cc index 8959d8469a..c2e4617b0b 100644 --- a/src/lib/dns/tests/message_unittest.cc +++ b/src/lib/dns/tests/message_unittest.cc @@ -279,7 +279,7 @@ TEST_F(MessageTest, getRRCount) { EXPECT_EQ(0, message_render.getRRCount(Message::SECTION_ADDITIONAL)); // out-of-band section ID - EXPECT_THROW(message_parse.getRRCount(bogus_section), OutOfRange); + EXPECT_THROW(message_parse.getRRCount(bogus_section), isc::OutOfRange); } TEST_F(MessageTest, addRRset) { @@ -306,7 +306,7 @@ TEST_F(MessageTest, badAddRRset) { EXPECT_THROW(message_parse.addRRset(Message::SECTION_ANSWER, rrset_a), InvalidMessageOperation); // out-of-band section ID - EXPECT_THROW(message_render.addRRset(bogus_section, rrset_a), OutOfRange); + EXPECT_THROW(message_render.addRRset(bogus_section, rrset_a), isc::OutOfRange); // NULL RRset EXPECT_THROW(message_render.addRRset(Message::SECTION_ANSWER, RRsetPtr()), @@ -334,7 +334,7 @@ TEST_F(MessageTest, hasRRset) { // out-of-band section ID EXPECT_THROW(message_render.hasRRset(bogus_section, test_name, RRClass::IN(), RRType::A()), - OutOfRange); + isc::OutOfRange); // Repeat the checks having created an RRset of the appropriate type. @@ -355,7 +355,7 @@ TEST_F(MessageTest, hasRRset) { RRsetPtr rrs5(new RRset(test_name, RRClass::IN(), RRType::AAAA(), RRTTL(5))); EXPECT_FALSE(message_render.hasRRset(Message::SECTION_ANSWER, rrs4)); - EXPECT_THROW(message_render.hasRRset(bogus_section, rrs1), OutOfRange); + EXPECT_THROW(message_render.hasRRset(bogus_section, rrs1), isc::OutOfRange); } TEST_F(MessageTest, removeRRset) { @@ -457,21 +457,21 @@ TEST_F(MessageTest, badClearSection) { EXPECT_THROW(message_parse.clearSection(Message::SECTION_QUESTION), InvalidMessageOperation); // attempt of clearing out-of-range section - EXPECT_THROW(message_render.clearSection(bogus_section), OutOfRange); + EXPECT_THROW(message_render.clearSection(bogus_section), isc::OutOfRange); } TEST_F(MessageTest, badBeginSection) { // valid cases are tested via other tests EXPECT_THROW(message_render.beginSection(Message::SECTION_QUESTION), InvalidMessageSection); - EXPECT_THROW(message_render.beginSection(bogus_section), OutOfRange); + EXPECT_THROW(message_render.beginSection(bogus_section), isc::OutOfRange); } TEST_F(MessageTest, badEndSection) { // valid cases are tested via other tests EXPECT_THROW(message_render.endSection(Message::SECTION_QUESTION), InvalidMessageSection); - EXPECT_THROW(message_render.endSection(bogus_section), OutOfRange); + EXPECT_THROW(message_render.endSection(bogus_section), isc::OutOfRange); } TEST_F(MessageTest, appendSection) { @@ -479,7 +479,7 @@ TEST_F(MessageTest, appendSection) { // Section check EXPECT_THROW(target.appendSection(bogus_section, message_render), - OutOfRange); + isc::OutOfRange); // Make sure nothing is copied if there is nothing to copy target.appendSection(Message::SECTION_QUESTION, message_render); @@ -635,7 +635,7 @@ TEST_F(MessageTest, fromWireShortBuffer) { // fromWire() should throw an exception while parsing the trimmed RR. UnitTestUtil::readWireData("message_fromWire22.wire", received_data); InputBuffer buffer(&received_data[0], received_data.size() - 1); - EXPECT_THROW(message_parse.fromWire(buffer), InvalidBufferPosition); + EXPECT_THROW(message_parse.fromWire(buffer), isc::OutOfRange); } TEST_F(MessageTest, fromWireCombineRRs) { diff --git a/src/lib/dns/tests/rdata_opt_unittest.cc b/src/lib/dns/tests/rdata_opt_unittest.cc index d5d1519850..a79c4fd832 100644 --- a/src/lib/dns/tests/rdata_opt_unittest.cc +++ b/src/lib/dns/tests/rdata_opt_unittest.cc @@ -70,7 +70,7 @@ TEST_F(Rdata_OPT_Test, createFromWire) { // short buffer case. EXPECT_THROW(rdataFactoryFromFile(RRType::OPT(), RRClass::IN(), "rdata_opt_fromWire1", 11), - InvalidBufferPosition); + isc::OutOfRange); } TEST_F(Rdata_OPT_Test, createFromLexer) { diff --git a/src/lib/dns/tests/rdata_tkey_unittest.cc b/src/lib/dns/tests/rdata_tkey_unittest.cc index 18bf886966..08a91a1326 100644 --- a/src/lib/dns/tests/rdata_tkey_unittest.cc +++ b/src/lib/dns/tests/rdata_tkey_unittest.cc @@ -279,11 +279,11 @@ TEST_F(Rdata_TKEY_Test, badFromWire) { // Key length is bogus: EXPECT_THROW(rdataFactoryFromFile(RRType::TKEY(), RRClass::ANY(), "rdata_tkey_fromWire8.wire"), - InvalidBufferPosition); + isc::OutOfRange); // Other-data length is bogus: EXPECT_THROW(rdataFactoryFromFile(RRType::TKEY(), RRClass::ANY(), "rdata_tkey_fromWire9.wire"), - InvalidBufferPosition); + isc::OutOfRange); } TEST_F(Rdata_TKEY_Test, copyConstruct) { diff --git a/src/lib/dns/tests/rdata_tsig_unittest.cc b/src/lib/dns/tests/rdata_tsig_unittest.cc index fa7be1d6a6..c64e082d22 100644 --- a/src/lib/dns/tests/rdata_tsig_unittest.cc +++ b/src/lib/dns/tests/rdata_tsig_unittest.cc @@ -266,11 +266,11 @@ TEST_F(Rdata_TSIG_Test, badFromWire) { // MAC size is bogus: EXPECT_THROW(rdataFactoryFromFile(RRType::TSIG(), RRClass::ANY(), "rdata_tsig_fromWire8.wire"), - InvalidBufferPosition); + isc::OutOfRange); // Other-data length is bogus: EXPECT_THROW(rdataFactoryFromFile(RRType::TSIG(), RRClass::ANY(), "rdata_tsig_fromWire9.wire"), - InvalidBufferPosition); + isc::OutOfRange); } TEST_F(Rdata_TSIG_Test, copyConstruct) { diff --git a/src/lib/dns/tests/rdata_unittest.cc b/src/lib/dns/tests/rdata_unittest.cc index 93a58c0e4f..c860231673 100644 --- a/src/lib/dns/tests/rdata_unittest.cc +++ b/src/lib/dns/tests/rdata_unittest.cc @@ -324,7 +324,7 @@ TEST_F(Rdata_Unknown_Test, createFromWire) { // buffer too short. the error should be detected in buffer read EXPECT_THROW(rdataFactoryFromFile(unknown_rrtype, RRClass::IN(), "rdata_unknown_fromWire", 8), - InvalidBufferPosition); + isc::OutOfRange); // too large data vector v; diff --git a/src/lib/util/buffer.h b/src/lib/util/buffer.h index 0841dcd065..2a6670d380 100644 --- a/src/lib/util/buffer.h +++ b/src/lib/util/buffer.h @@ -19,15 +19,6 @@ namespace isc { namespace util { -/// @brief A standard DNS module exception that is thrown if an out-of-range -/// buffer operation is being performed. -/// -class InvalidBufferPosition : public isc::OutOfRange { -public: - InvalidBufferPosition(const char* file, size_t line, const char* what) : - isc::OutOfRange(file, line, what) {} -}; - /// @brief The @c InputBuffer class is a buffer abstraction for /// manipulating read-only data. /// @@ -114,14 +105,14 @@ public: /// @brief Set the read position of the buffer to the given value. /// /// The new position must be in the valid range of the buffer; - /// otherwise an exception of class @c - /// isc::dns::InvalidBufferPosition will be thrown. + /// otherwise an exception of class @c isc::OutOfRange will be thrown. /// /// @param position The new position (offset from the beginning of /// the buffer). void setPosition(size_t position) { if (base_ + position > end_) { - throwError("position is too large"); + isc_throw(OutOfRange, + "InputBuffer::setPosition position is too large"); } current_ = base_ + position; } @@ -129,11 +120,11 @@ public: /// @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. + /// an exception of class @c isc::OutOfRange will be thrown. uint8_t peekUint8() { if (current_ + sizeof(uint8_t) > end_) { - throwError("read beyond end of buffer"); + isc_throw(OutOfRange, + "InputBuffer::peekUint8 read beyond end of buffer"); } return (*current_); @@ -142,8 +133,7 @@ public: /// @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. + /// an exception of class @c isc::OutOfRange will be thrown. uint8_t readUint8() { uint8_t ret = peekUint8(); current_ += sizeof(uint8_t); @@ -155,11 +145,11 @@ public: /// 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. + /// an exception of class @c isc::OutOfRange will be thrown. uint16_t peekUint16() { if (current_ + sizeof(uint16_t) > end_) { - throwError("read beyond end of buffer"); + isc_throw(OutOfRange, + "InputBuffer::peekUint16 read beyond end of buffer"); } uint16_t ret; @@ -173,8 +163,7 @@ public: /// 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. + /// an exception of class @c isc::OutOfRange will be thrown. uint16_t readUint16() { uint16_t ret = peekUint16(); current_ += sizeof(uint16_t); @@ -186,11 +175,11 @@ public: /// 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. + /// an exception of class @c isc::OutOfRange will be thrown. uint32_t peekUint32() { if (current_ + sizeof(uint32_t) > end_) { - throwError("read beyond end of buffer"); + isc_throw(OutOfRange, + "InputBuffer::peekUint32 read beyond end of buffer"); } uint32_t ret; @@ -206,8 +195,7 @@ public: /// 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. + /// an exception of class @c isc::OutOfRange will be thrown. uint32_t readUint32() { uint32_t ret = peekUint32(); current_ += sizeof(uint32_t); @@ -220,11 +208,12 @@ public: /// /// 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. + /// than the specified length, an exception of class @c isc::OutOfRange + /// will be thrown. void peekData(void* data, size_t len) { if (current_ + len > end_) { - throwError("read beyond end of buffer"); + isc_throw(OutOfRange, + "InputBuffer::peekData read beyond end of buffer"); } static_cast(std::memmove(data, current_, len)); @@ -235,8 +224,8 @@ public: /// /// 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. + /// than the specified length, an exception of class @c isc::OutOfRange + /// will be thrown. void readData(void* data, size_t len) { peekData(data, len); current_ += len; @@ -245,13 +234,16 @@ public: /// @brief Peek specified number of bytes as a vector. /// /// If specified buffer is too short, it will be expanded using - /// vector::resize() method. + /// vector::resize() method. If the remaining length of the buffer + /// is smaller than the specified length, an exception of class + /// @c isc::OutOfRange will be thrown. /// /// @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& data, size_t len) { if (current_ + len > end_) { - throwError("read beyond end of buffer"); + isc_throw(OutOfRange, + "InputBuffer::peekVector read beyond end of buffer"); } data.resize(len); @@ -261,7 +253,9 @@ public: /// @brief Read specified number of bytes as a vector. /// /// If specified buffer is too short, it will be expanded using - /// vector::resize() method. + /// vector::resize() method. If the remaining length of the buffer + /// is smaller than the specified length, an exception of class + /// @c isc::OutOfRange will be thrown. /// /// @param data Reference to a buffer (data will be stored there). /// @param len Size specified number of bytes to read in a vector. @@ -271,11 +265,6 @@ public: } private: - /// @brief A common helper to throw an exception on invalid operation. - static void throwError(const char* msg) { - isc_throw(InvalidBufferPosition, msg); - } - /// @brief Base of the buffer. const uint8_t* base_; @@ -433,8 +422,9 @@ public: /// @param pos The position in the buffer to be returned. uint8_t operator[](size_t pos) const { if (pos >= buffer_.size()) { - isc_throw(InvalidBufferPosition, - "[]: pos (" << pos << ") >= size (" << buffer_.size() << ")"); + isc_throw(OutOfRange, + "OutputBuffer::[]: pos (" << pos + << ") >= size (" << buffer_.size() << ")"); } return (buffer_[pos]); } @@ -467,7 +457,8 @@ public: /// @param len The length of data that should be trimmed. void trim(size_t len) { if (len > buffer_.size()) { - isc_throw(OutOfRange, "trimming too large from output buffer"); + isc_throw(OutOfRange, + "OutputBuffer::trim length too large from output buffer"); } buffer_.resize(buffer_.size() - len); } @@ -494,7 +485,8 @@ public: /// @param pos The position in the buffer to write the data. void writeUint8At(uint8_t data, size_t pos) { if (pos + sizeof(data) > buffer_.size()) { - isc_throw(InvalidBufferPosition, "write at invalid position"); + isc_throw(OutOfRange, + "OutputBuffer::writeUint8At write at invalid position"); } buffer_[pos] = data; } @@ -521,7 +513,8 @@ public: /// @param pos The beginning position in the buffer to write the data. void writeUint16At(uint16_t data, size_t pos) { if (pos + sizeof(data) > buffer_.size()) { - isc_throw(InvalidBufferPosition, "write at invalid position"); + isc_throw(OutOfRange, + "OutputBuffer::writeUint16At write at invalid position"); } buffer_[pos] = static_cast((data & 0xff00U) >> 8); -- 2.47.3