From 30532ac320f7a65701af1172fbdb4d61c625c99b Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Fri, 4 Feb 2022 12:40:36 -0500 Subject: [PATCH 1/4] [#2285] Fixed out of scope error in PgSql code in dhcpsrv src/lib/dhcpsrv/pgsql_host_data_source.cc PgSqlHostDataSource::del() - use PsqlBindArray::addTempString() src/lib/dhcpsrv/pgsql_lease_mgr.cc PgSqlLeaseStatsQuery::start() - use PsqlBindArray::addTempString() --- src/lib/dhcpsrv/pgsql_host_data_source.cc | 2 +- src/lib/dhcpsrv/pgsql_lease_mgr.cc | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.cc b/src/lib/dhcpsrv/pgsql_host_data_source.cc index d3e162cc3d..44950908b1 100644 --- a/src/lib/dhcpsrv/pgsql_host_data_source.cc +++ b/src/lib/dhcpsrv/pgsql_host_data_source.cc @@ -2676,7 +2676,7 @@ PgSqlHostDataSource::del(const SubnetID& subnet_id, } // v6 - bind_array->add(addr.toText()); + bind_array->addTempString(addr.toText()); return (impl_->delStatement(ctx, PgSqlHostDataSourceImpl::DEL_HOST_ADDR6, bind_array)); diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc index 3167e7dc32..2470c26a04 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc @@ -1074,14 +1074,12 @@ public: PsqlBindArray parms; // Add first_subnet_id used by both single and range. - std::string subnet_id_str = boost::lexical_cast(getFirstSubnetID()); - parms.add(subnet_id_str); + parms.addTempString(boost::lexical_cast(getFirstSubnetID())); // Add last_subnet_id for range. if (getSelectMode() == SUBNET_RANGE) { // Add last_subnet_id used by range. - std::string subnet_id_str = boost::lexical_cast(getLastSubnetID()); - parms.add(subnet_id_str); + parms.addTempString(boost::lexical_cast(getLastSubnetID())); } // Run the query with where clause parameters. -- GitLab From adaa8c4199dd04f0371dbf49c0abf918d85f6aa6 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Mon, 7 Feb 2022 11:05:38 -0500 Subject: [PATCH 2/4] [#2285] Addressed review comments, fixed doxygen Minor changes modified: src/lib/dhcpsrv/pgsql_host_data_source.cc src/lib/pgsql/pgsql_exchange.h --- src/lib/dhcpsrv/pgsql_host_data_source.cc | 8 +++----- src/lib/pgsql/pgsql_exchange.h | 9 ++++----- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.cc b/src/lib/dhcpsrv/pgsql_host_data_source.cc index 44950908b1..f0e0ff1635 100644 --- a/src/lib/dhcpsrv/pgsql_host_data_source.cc +++ b/src/lib/dhcpsrv/pgsql_host_data_source.cc @@ -220,16 +220,14 @@ public: // dhcp4_subnet_id : INT NULL if (host->getIPv4SubnetID() == SUBNET_ID_UNUSED) { bind_array->addNull(); - } - else { + } else { bind_array->add(host->getIPv4SubnetID()); } // dhcp6_subnet_id : INT NULL if (host->getIPv6SubnetID() == SUBNET_ID_UNUSED) { bind_array->addNull(); - } - else { + } else { bind_array->add(host->getIPv6SubnetID()); } @@ -269,7 +267,7 @@ public: if (key.empty()) { bind_array->addNull(); } else { - bind_array->add(key); + bind_array->addTempString(key); } // When checking whether the IP is unique we need to bind the IPv4 address diff --git a/src/lib/pgsql/pgsql_exchange.h b/src/lib/pgsql/pgsql_exchange.h index 0b0383610f..9c40c30756 100644 --- a/src/lib/pgsql/pgsql_exchange.h +++ b/src/lib/pgsql/pgsql_exchange.h @@ -26,8 +26,6 @@ namespace isc { namespace db { -/// @} - /// @brief RAII wrapper for PostgreSQL Result sets /// /// When a Postgresql statement is executed, the results are returned @@ -331,7 +329,7 @@ struct PsqlBindArray { /// Stores the current value of a triplet to the bind array. /// If it is unspecified it stores a NULL. /// - /// @param triple Triplet instance from which to get the value. + /// @param triplet Triplet instance from which to get the value. void add(const isc::util::Triplet& triplet); /// @brief Adds an integer Triplet's minimum value to the bind array @@ -339,7 +337,7 @@ struct PsqlBindArray { /// Stores the minimum value of a triplet to the bind array. /// If it is unspecified it stores a NULL. /// - /// @param triple Triplet instance from which to get the value. + /// @param triplet Triplet instance from which to get the value. void addMin(const isc::util::Triplet& triplet); /// @brief Adds an integer Triplet's maximum value to the bind array @@ -347,7 +345,7 @@ struct PsqlBindArray { /// Stores the maximum value of a triplet to the bind array. /// If it is unspecified it stores a NULL. /// - /// @param triple Triplet instance from which to get the value. + /// @param triplet Triplet instance from which to get the value. void addMax(const isc::util::Triplet& triplet); /// @brief Adds an @c Optional string to the bind array. @@ -721,6 +719,7 @@ public: /// @param r the result set containing the query results /// @param row the row number within the result set /// @param col the column number within the row + /// @param[out] value ElementPtr to receive the column data /// /// @throw DbOperationError if the value cannot be fetched or is /// invalid. -- GitLab From 7da642078661a1080f70f63840b0cdf675ad64e7 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Mon, 7 Feb 2022 14:21:27 -0500 Subject: [PATCH 3/4] [#2285] Addressed further comments src/lib/pgsql/pgsql_exchange.* PsqlBindArray::addTempBinary() - new function src/lib/pgsql/tests/pgsql_exchange_unittest.cc TEST(PsqlBindArray, addDataTest) - added use of addTempBinary() --- src/lib/pgsql/pgsql_exchange.cc | 9 +++++++++ src/lib/pgsql/pgsql_exchange.h | 9 +++++++++ src/lib/pgsql/tests/pgsql_exchange_unittest.cc | 7 ++++++- 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/lib/pgsql/pgsql_exchange.cc b/src/lib/pgsql/pgsql_exchange.cc index ed2ae76e96..f2c25b4b65 100644 --- a/src/lib/pgsql/pgsql_exchange.cc +++ b/src/lib/pgsql/pgsql_exchange.cc @@ -82,6 +82,15 @@ void PsqlBindArray::add(const std::vector& data) { formats_.push_back(BINARY_FMT); } +void PsqlBindArray::addTempBinary(const std::vector& data) { + bound_strs_.push_back(ConstStringPtr(new std::string( + (reinterpret_cast(data.data())), data.size()))); + + values_.push_back(reinterpret_cast(bound_strs_.back()->data())); + lengths_.push_back(data.size()); + formats_.push_back(BINARY_FMT); +} + void PsqlBindArray::add(const uint8_t* data, const size_t len) { if (!data) { isc_throw(BadValue, "PsqlBindArray::add - uint8_t data cannot be NULL"); diff --git a/src/lib/pgsql/pgsql_exchange.h b/src/lib/pgsql/pgsql_exchange.h index 9c40c30756..37c7479044 100644 --- a/src/lib/pgsql/pgsql_exchange.h +++ b/src/lib/pgsql/pgsql_exchange.h @@ -255,6 +255,15 @@ struct PsqlBindArray { /// @param data vector of binary bytes. void add(const std::vector& data); + /// @brief Adds a vector of binary data to the bind array. + /// + /// Adds a BINARY_FMT value to the end of the bind array using the + /// given vector as the data source. This creates an internally scoped + /// copy of the vector. + /// + /// @param data vector of binary bytes. + void addTempBinary(const std::vector& data); + /// @brief Adds a buffer of binary data to the bind array. /// /// Adds a BINARY_FMT value to the end of the bind array using the diff --git a/src/lib/pgsql/tests/pgsql_exchange_unittest.cc b/src/lib/pgsql/tests/pgsql_exchange_unittest.cc index 3e0b6d092c..d8e8eb7fa4 100644 --- a/src/lib/pgsql/tests/pgsql_exchange_unittest.cc +++ b/src/lib/pgsql/tests/pgsql_exchange_unittest.cc @@ -98,6 +98,10 @@ TEST(PsqlBindArray, addDataTest) { // Add a JSON. ElementPtr elems = Element::fromJSON("{ \"foo\": \"bar\" }"); b.add(elems); + + // Add a Temporary blob + std::vector blob({0x0a,0x0b,0x0,0xc,0xd}); + b.addTempBinary(blob); } // We've left bind scope, everything should be intact. @@ -117,7 +121,8 @@ TEST(PsqlBindArray, addDataTest) { "12 : \"192.168.1.1\"\n" "13 : \"3001::1\"\n" "14 : \"2\"\n" - "15 : \"{ \"foo\": \"bar\" }\"\n"; + "15 : \"{ \"foo\": \"bar\" }\"\n" + "16 : 0x0a0b000c0d\n"; EXPECT_EQ(expected, b.toText()); } -- GitLab From e43c624d629190711883e1148ea48ecbe0e8cdf6 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Mon, 7 Feb 2022 14:29:21 -0500 Subject: [PATCH 4/4] [#2285] Use addTempBinary in PgSQL CB addOptionValueBinding() modified: src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc --- src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc b/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc index 1fdbb9fd80..734204d87d 100644 --- a/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc +++ b/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc @@ -647,7 +647,7 @@ PgSqlConfigBackendImpl::addOptionValueBinding(PsqlBindArray& bindings, const char* buf_ptr = static_cast(buf.getData()); std::vector blob(buf_ptr + opt->getHeaderLen(), buf_ptr + buf.getLength()); - bindings.add(blob); + bindings.addTempBinary(blob); } else { bindings.addNull(); } -- GitLab