From: Thomas Markwalder Date: Fri, 24 May 2019 12:54:44 +0000 (-0400) Subject: [#539, !330] Addressed review comments X-Git-Tag: Kea-1.6.0-beta~77 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f3aa0f9b9029badc0b9cd247283709e78648a73d;p=thirdparty%2Fkea.git [#539, !330] Addressed review comments src/lib/util/strutil.* seekTrimmed() - new templated function src/lib/util/tests/strutil_unittest.cc TEST(StringUtilTest, seekTrimmed) - new test src/lib/dhcp/option_data_types.cc src/lib/dhcp/option_string.c updated to use seekTrimmed() --- diff --git a/src/lib/dhcp/option_data_types.cc b/src/lib/dhcp/option_data_types.cc index a64707a9ba..5365c3ef51 100644 --- a/src/lib/dhcp/option_data_types.cc +++ b/src/lib/dhcp/option_data_types.cc @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -576,9 +577,7 @@ OptionDataTypeUtil::readString(const std::vector& buf) { if (!buf.empty()) { // Per RFC 2132, section 2 we need to drop trailing NULLs auto begin = buf.begin(); - auto end = buf.end(); - for ( ; end != begin && *(end - 1) == 0x0; --end); - + auto end = util::str::seekTrimmed(begin, buf.end(), 0x0); if (std::distance(begin, end) == 0) { isc_throw(isc::OutOfRange, "string value carried by the option " << " contained only NULLs"); diff --git a/src/lib/dhcp/option_string.cc b/src/lib/dhcp/option_string.cc index 2b185520a5..68b80a40f0 100644 --- a/src/lib/dhcp/option_string.cc +++ b/src/lib/dhcp/option_string.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2019 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 @@ -7,6 +7,7 @@ #include #include +#include #include namespace isc { @@ -52,8 +53,7 @@ OptionString::setValue(const std::string& value) { // Trim off any trailing NULLs. auto begin = value.begin(); - auto end = value.end(); - for ( ; end != begin && *(end - 1) == 0x0; --end); + auto end = util::str::seekTrimmed(begin, value.end(), 0x0); if (std::distance(begin, end) == 0) { isc_throw(isc::OutOfRange, "string value carried by the option '" @@ -86,8 +86,7 @@ void OptionString::unpack(OptionBufferConstIter begin, OptionBufferConstIter end) { // Trim off trailing null(s) - for ( ; end != begin && *(end - 1) == 0x0; --end); - + end = util::str::seekTrimmed(begin, end, 0x0); if (std::distance(begin, end) == 0) { isc_throw(isc::OutOfRange, "failed to parse an option '" << getType() << "' holding string value" diff --git a/src/lib/dhcp/option_string.h b/src/lib/dhcp/option_string.h index c590fe1fe2..784257d945 100644 --- a/src/lib/dhcp/option_string.h +++ b/src/lib/dhcp/option_string.h @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2019 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 diff --git a/src/lib/util/strutil.h b/src/lib/util/strutil.h index 8160bee8f6..2ce16960b9 100644 --- a/src/lib/util/strutil.h +++ b/src/lib/util/strutil.h @@ -54,6 +54,26 @@ void normalizeSlash(std::string& name); /// \return String with leading and trailing spaces removed std::string trim(const std::string& instring); +/// \brief Finds the "trimmed" end of a buffer +/// +/// Works backward from the the end of the buffer, looking for the first +/// character not equal to the trim value, and returns an iterator +/// pointing that that position. +/// +/// \param begin - Forward iterator pointing to the beginning of the +/// buffer to trim +/// \param end - Forward iterator pointing to the untrimmed end of +/// the buffer to trim +/// \param val - byte value to trim off +/// +/// \retrun Iterator pointing the first character from the end of the +/// buffer not equal to the trim value +template +Iterator +seekTrimmed(Iterator begin, Iterator end, uint8_t trim_val) { + for ( ; end != begin && *(end - 1) == trim_val; --end); + return(end); +} /// \brief Split String into Tokens /// diff --git a/src/lib/util/tests/strutil_unittest.cc b/src/lib/util/tests/strutil_unittest.cc index 07f3d84121..125bf881d9 100644 --- a/src/lib/util/tests/strutil_unittest.cc +++ b/src/lib/util/tests/strutil_unittest.cc @@ -551,4 +551,50 @@ TEST(StringUtilTest, stringSanitizer) { "abc.123"); } +// Verifies templated buffer iterator trim() function +TEST(StringUtilTest, seekTrimmed) { + + // Empty buffer should be fine. + std::vector buffer; + auto begin = buffer.end(); + auto end = buffer.end(); + ASSERT_NO_THROW(end = seekTrimmed(begin, end, 0)); + EXPECT_EQ(0, std::distance(begin, end)); + + // Buffer of only trim values, should be fine. + buffer = { 1, 1 }; + begin = buffer.begin(); + end = buffer.end(); + ASSERT_NO_THROW(end = seekTrimmed(begin, end, 1)); + EXPECT_EQ(0, std::distance(begin, end)); + + // One trailing null should trim off. + buffer = {'o', 'n', 'e', 0 }; + begin = buffer.begin(); + end = buffer.end(); + ASSERT_NO_THROW(end = seekTrimmed(begin, end, 0)); + EXPECT_EQ(3, std::distance(begin, end)); + + // More than one trailing null should trim off. + buffer = { 't', 'h', 'r', 'e', 'e', 0, 0, 0 }; + begin = buffer.begin(); + end = buffer.end(); + ASSERT_NO_THROW(end = seekTrimmed(begin, end, 0)); + EXPECT_EQ(5, std::distance(begin, end)); + + // Embedded null should be left in place. + buffer = { 'e', 'm', 0, 'b', 'e', 'd' }; + begin = buffer.begin(); + end = buffer.end(); + ASSERT_NO_THROW(end = seekTrimmed(begin, end, 0)); + EXPECT_EQ(6, std::distance(begin, end)); + + // Leading null should be left in place. + buffer = { 0, 'l', 'e', 'a', 'd', 'i', 'n', 'g' }; + begin = buffer.begin(); + end = buffer.end(); + ASSERT_NO_THROW(end = seekTrimmed(begin, end, 0)); + EXPECT_EQ(8, std::distance(begin, end)); +} + } // end of anonymous namespace